diff --git a/test/s3/sse/s3_sse_integration_test.go b/test/s3/sse/s3_sse_integration_test.go index 4b7eb0ddc..1a4576691 100644 --- a/test/s3/sse/s3_sse_integration_test.go +++ b/test/s3/sse/s3_sse_integration_test.go @@ -773,6 +773,200 @@ func TestSSEMultipartUploadIntegration(t *testing.T) { assert.Equal(t, types.ServerSideEncryptionAwsKms, resp.ServerSideEncryption) assert.Equal(t, kmsKeyID, aws.ToString(resp.SSEKMSKeyId)) }) + + t.Run("Multipart Parts Larger Than Internal Chunks Across SSE Types", func(t *testing.T) { + largeParts := [][]byte{ + generateTestData(9*1024*1024 + 123), // crosses SeaweedFS 8MB internal chunk boundary + generateTestData(5*1024*1024 + 321), + } + + t.Run("SSE-C", func(t *testing.T) { + sseKey := generateSSECKey() + uploadAndVerifyMultipartSSEObject(t, ctx, client, bucketName, "large-internal-chunks-ssec", largeParts, multipartSSEOptions{ + configureCreate: func(input *s3.CreateMultipartUploadInput) { + input.SSECustomerAlgorithm = aws.String("AES256") + input.SSECustomerKey = aws.String(sseKey.KeyB64) + input.SSECustomerKeyMD5 = aws.String(sseKey.KeyMD5) + }, + configureUploadPart: func(input *s3.UploadPartInput) { + input.SSECustomerAlgorithm = aws.String("AES256") + input.SSECustomerKey = aws.String(sseKey.KeyB64) + input.SSECustomerKeyMD5 = aws.String(sseKey.KeyMD5) + }, + configureGet: func(input *s3.GetObjectInput) { + input.SSECustomerAlgorithm = aws.String("AES256") + input.SSECustomerKey = aws.String(sseKey.KeyB64) + input.SSECustomerKeyMD5 = aws.String(sseKey.KeyMD5) + }, + verifyGet: func(resp *s3.GetObjectOutput) { + assert.Equal(t, "AES256", aws.ToString(resp.SSECustomerAlgorithm)) + assert.Equal(t, sseKey.KeyMD5, aws.ToString(resp.SSECustomerKeyMD5)) + }, + }) + }) + + t.Run("SSE-KMS", func(t *testing.T) { + kmsKeyID := "test-large-internal-chunks-key" + uploadAndVerifyMultipartSSEObject(t, ctx, client, bucketName, "large-internal-chunks-ssekms", largeParts, multipartSSEOptions{ + configureCreate: func(input *s3.CreateMultipartUploadInput) { + input.ServerSideEncryption = types.ServerSideEncryptionAwsKms + input.SSEKMSKeyId = aws.String(kmsKeyID) + }, + verifyGet: func(resp *s3.GetObjectOutput) { + assert.Equal(t, types.ServerSideEncryptionAwsKms, resp.ServerSideEncryption) + assert.Equal(t, kmsKeyID, aws.ToString(resp.SSEKMSKeyId)) + }, + }) + }) + + t.Run("SSE-S3 Explicit", func(t *testing.T) { + uploadAndVerifyMultipartSSEObject(t, ctx, client, bucketName, "large-internal-chunks-sses3-explicit", largeParts, multipartSSEOptions{ + configureCreate: func(input *s3.CreateMultipartUploadInput) { + input.ServerSideEncryption = types.ServerSideEncryptionAes256 + }, + verifyGet: func(resp *s3.GetObjectOutput) { + assert.Equal(t, types.ServerSideEncryptionAes256, resp.ServerSideEncryption) + }, + }) + }) + + t.Run("SSE-S3 Bucket Default", func(t *testing.T) { + _, err := client.PutBucketEncryption(ctx, &s3.PutBucketEncryptionInput{ + Bucket: aws.String(bucketName), + ServerSideEncryptionConfiguration: &types.ServerSideEncryptionConfiguration{ + Rules: []types.ServerSideEncryptionRule{ + { + ApplyServerSideEncryptionByDefault: &types.ServerSideEncryptionByDefault{ + SSEAlgorithm: types.ServerSideEncryptionAes256, + }, + }, + }, + }, + }) + require.NoError(t, err, "Failed to set bucket default SSE-S3 encryption") + defer client.DeleteBucketEncryption(ctx, &s3.DeleteBucketEncryptionInput{Bucket: aws.String(bucketName)}) + + uploadAndVerifyMultipartSSEObject(t, ctx, client, bucketName, "large-internal-chunks-sses3-default", largeParts, multipartSSEOptions{ + verifyGet: func(resp *s3.GetObjectOutput) { + assert.Equal(t, types.ServerSideEncryptionAes256, resp.ServerSideEncryption) + }, + }) + }) + }) +} + +type multipartSSEOptions struct { + configureCreate func(*s3.CreateMultipartUploadInput) + configureUploadPart func(*s3.UploadPartInput) + configureGet func(*s3.GetObjectInput) + verifyGet func(*s3.GetObjectOutput) +} + +func uploadAndVerifyMultipartSSEObject(t *testing.T, ctx context.Context, client *s3.Client, bucketName, objectKey string, partsData [][]byte, opts multipartSSEOptions) { + t.Helper() + + createInput := &s3.CreateMultipartUploadInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + } + if opts.configureCreate != nil { + opts.configureCreate(createInput) + } + createResp, err := client.CreateMultipartUpload(ctx, createInput) + require.NoError(t, err, "Failed to create multipart upload") + + uploadID := aws.ToString(createResp.UploadId) + completedParts := make([]types.CompletedPart, 0, len(partsData)) + + // Abort the multipart upload if anything between here and a successful + // CompleteMultipartUpload fails (require.NoError calls t.Fatal, which + // triggers t.Cleanup but skips inline defers). We use context.Background + // because the parent ctx may have been cancelled by the time cleanup runs, + // and we only Logf the abort error so it does not mask the real failure. + completed := false + t.Cleanup(func() { + if completed { + return + } + if _, abortErr := client.AbortMultipartUpload(context.Background(), &s3.AbortMultipartUploadInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + UploadId: aws.String(uploadID), + }); abortErr != nil { + t.Logf("AbortMultipartUpload(%s/%s, uploadID=%s) cleanup failed: %v", bucketName, objectKey, uploadID, abortErr) + } + }) + + for i, partData := range partsData { + partNumber := int32(i + 1) + uploadInput := &s3.UploadPartInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + PartNumber: aws.Int32(partNumber), + UploadId: aws.String(uploadID), + Body: bytes.NewReader(partData), + } + if opts.configureUploadPart != nil { + opts.configureUploadPart(uploadInput) + } + partResp, err := client.UploadPart(ctx, uploadInput) + require.NoError(t, err, "Failed to upload part %d", partNumber) + + completedParts = append(completedParts, types.CompletedPart{ + ETag: partResp.ETag, + PartNumber: aws.Int32(partNumber), + }) + } + + _, err = client.CompleteMultipartUpload(ctx, &s3.CompleteMultipartUploadInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + UploadId: aws.String(uploadID), + MultipartUpload: &types.CompletedMultipartUpload{ + Parts: completedParts, + }, + }) + require.NoError(t, err, "Failed to complete multipart upload") + completed = true + + expectedData := bytes.Join(partsData, nil) + getInput := &s3.GetObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + } + if opts.configureGet != nil { + opts.configureGet(getInput) + } + resp, err := client.GetObject(ctx, getInput) + require.NoError(t, err, "Failed to retrieve completed multipart object") + downloadedData, err := io.ReadAll(resp.Body) + resp.Body.Close() + require.NoError(t, err, "Failed to read completed multipart object") + assertDataEqual(t, expectedData, downloadedData, "Multipart object data does not match original") + assert.Equal(t, int64(len(expectedData)), aws.ToInt64(resp.ContentLength), "Multipart content length mismatch") + if opts.verifyGet != nil { + opts.verifyGet(resp) + } + + rangeStart := int64(8*1024*1024 - 64) + rangeEnd := int64(8*1024*1024 + 64) + rangeInput := &s3.GetObjectInput{ + Bucket: aws.String(bucketName), + Key: aws.String(objectKey), + Range: aws.String(fmt.Sprintf("bytes=%d-%d", rangeStart, rangeEnd)), + } + if opts.configureGet != nil { + opts.configureGet(rangeInput) + } + rangeResp, err := client.GetObject(ctx, rangeInput) + require.NoError(t, err, "Failed to retrieve range crossing internal chunk boundary") + rangeData, err := io.ReadAll(rangeResp.Body) + rangeResp.Body.Close() + require.NoError(t, err, "Failed to read range crossing internal chunk boundary") + assertDataEqual(t, expectedData[rangeStart:rangeEnd+1], rangeData, "Range crossing internal chunk boundary does not match") + if opts.verifyGet != nil { + opts.verifyGet(rangeResp) + } } // TestDebugSSEMultipart helps debug the multipart SSE-KMS data mismatch diff --git a/weed/s3api/filer_multipart.go b/weed/s3api/filer_multipart.go index 7e16770c3..6a113f9e4 100644 --- a/weed/s3api/filer_multipart.go +++ b/weed/s3api/filer_multipart.go @@ -189,10 +189,17 @@ type multipartPartBoundary struct { ETag string `json:"etag"` } +type multipartSSES3Info struct { + keyData []byte + key *SSES3Key + baseIV []byte +} + type multipartCompletionState struct { deleteEntries []*filer_pb.Entry partEntries map[int][]*filer_pb.Entry pentry *filer_pb.Entry + sses3Info *multipartSSES3Info mime string finalParts []*filer_pb.FileChunk offset int64 @@ -221,6 +228,125 @@ func completeMultipartResult(r *http.Request, input *s3.CompleteMultipartUploadI return result } +func extractMultipartSSES3Info(entry *filer_pb.Entry) (*multipartSSES3Info, error) { + if entry == nil || entry.Extended == nil { + return nil, nil + } + if encryptionType := string(entry.Extended[s3_constants.SeaweedFSSSES3Encryption]); encryptionType != s3_constants.SSEAlgorithmAES256 { + return nil, nil + } + + baseIVEncoded := entry.Extended[s3_constants.SeaweedFSSSES3BaseIV] + if len(baseIVEncoded) == 0 { + return nil, fmt.Errorf("missing SSE-S3 multipart base IV") + } + baseIV, err := base64.StdEncoding.DecodeString(string(baseIVEncoded)) + if err != nil { + return nil, fmt.Errorf("decode SSE-S3 multipart base IV: %w", err) + } + if len(baseIV) != s3_constants.AESBlockSize { + return nil, fmt.Errorf("invalid SSE-S3 multipart base IV length %d", len(baseIV)) + } + + keyDataEncoded := entry.Extended[s3_constants.SeaweedFSSSES3KeyData] + if len(keyDataEncoded) == 0 { + return nil, fmt.Errorf("missing SSE-S3 multipart key data") + } + keyData, err := base64.StdEncoding.DecodeString(string(keyDataEncoded)) + if err != nil { + return nil, fmt.Errorf("decode SSE-S3 multipart key data: %w", err) + } + + key, err := DeserializeSSES3Metadata(keyData, GetSSES3KeyManager()) + if err != nil { + return nil, fmt.Errorf("deserialize SSE-S3 multipart key data: %w", err) + } + + return &multipartSSES3Info{ + keyData: keyData, + key: key, + baseIV: baseIV, + }, nil +} + +func completedMultipartChunk(chunk *filer_pb.FileChunk, offset int64, sses3Info *multipartSSES3Info) (*filer_pb.FileChunk, error) { + finalChunk := &filer_pb.FileChunk{ + FileId: chunk.GetFileIdString(), + Offset: offset, + Size: chunk.Size, + ModifiedTsNs: chunk.ModifiedTsNs, + CipherKey: chunk.CipherKey, + ETag: chunk.ETag, + IsCompressed: chunk.IsCompressed, + SseType: chunk.SseType, + SseMetadata: chunk.SseMetadata, + } + + if sses3Info == nil { + return finalChunk, nil + } + if finalChunk.GetSseType() != filer_pb.SSEType_NONE && finalChunk.GetSseType() != filer_pb.SSEType_SSE_S3 { + return finalChunk, nil + } + // Trust existing per-chunk SSE-S3 metadata: if the part went through + // putToFiler's chunk loop with a non-nil sseS3Key it carries an offset + // derived IV that we cannot recover from the upload-entry alone. We do + // not validate that the embedded IV equals calculateIVWithOffset(baseIV, + // chunk.Offset); only completely missing metadata is backfilled. This + // keeps healthy parts untouched and limits backfill to the cases that + // caused #8908. + if finalChunk.GetSseType() == filer_pb.SSEType_SSE_S3 && len(finalChunk.GetSseMetadata()) > 0 { + return finalChunk, nil + } + + // IV uses the chunk's offset within its part, with no partNumber term. + // This mirrors the encryption side: putToFiler hardcodes partOffset=0 when + // it calls handleSSES3MultipartEncryption for every part, so each part's + // encrypted stream begins at IV = calculateIVWithOffset(baseIV, 0) = baseIV. + // Each chunk within that stream is then at IV = + // calculateIVWithOffset(baseIV, chunk.Offset_part_local). Any deviation + // (e.g. adding (partNumber-1)*PartOffsetMultiplier) would produce IVs that + // do not match the actual encryption and would corrupt decryption. + chunkIV, _ := calculateIVWithOffset(sses3Info.baseIV, chunk.GetOffset()) + chunkKey := &SSES3Key{ + Key: sses3Info.key.Key, + KeyID: sses3Info.key.KeyID, + Algorithm: sses3Info.key.Algorithm, + IV: chunkIV, + } + chunkMetadata, err := SerializeSSES3Metadata(chunkKey) + if err != nil { + return nil, fmt.Errorf("serialize SSE-S3 chunk metadata for %s: %w", chunk.GetFileIdString(), err) + } + + finalChunk.SseType = filer_pb.SSEType_SSE_S3 + finalChunk.SseMetadata = chunkMetadata + return finalChunk, nil +} + +// applyMultipartSSES3HeadersFromUploadEntry writes the canonical object-level +// SSE-S3 attributes (SeaweedFSSSES3Key / X-Amz-Server-Side-Encryption) onto a +// completed multipart entry when they are missing. detectPrimarySSEType uses +// the object-level X-Amz-Server-Side-Encryption header to recognize SSE-S3 on +// inline / small-object reads, and IsSSES3EncryptedInternal requires both keys +// to consider an object encrypted at all. Without this, an object whose first +// part lacked the canonical attributes (e.g. a part written through a path +// that did not set them) would be served as unencrypted on GET. +func applyMultipartSSES3HeadersFromUploadEntry(dst *filer_pb.Entry, sses3Info *multipartSSES3Info) { + if dst == nil || sses3Info == nil { + return + } + if dst.Extended == nil { + dst.Extended = make(map[string][]byte) + } + if _, exists := dst.Extended[s3_constants.SeaweedFSSSES3Key]; !exists { + dst.Extended[s3_constants.SeaweedFSSSES3Key] = sses3Info.keyData + } + if _, exists := dst.Extended[s3_constants.AmzServerSideEncryption]; !exists { + dst.Extended[s3_constants.AmzServerSideEncryption] = []byte(s3_constants.SSEAlgorithmAES256) + } +} + func (s3a *S3ApiServer) prepareMultipartCompletionState(r *http.Request, input *s3.CompleteMultipartUploadInput, uploadDirectory, entryName, dirName string, completedPartNumbers []int, completedPartMap map[int][]string, maxPartNo int) (*multipartCompletionState, *CompleteMultipartUploadResult, s3err.ErrorCode) { if entry, err := s3a.resolveObjectEntry(*input.Bucket, *input.Key); err == nil && entry != nil && entry.Extended != nil { if uploadId, ok := entry.Extended[s3_constants.SeaweedFSUploadId]; ok && *input.UploadId == string(uploadId) { @@ -312,6 +438,11 @@ func (s3a *S3ApiServer) prepareMultipartCompletionState(r *http.Request, input * if pentry.Attributes != nil { mime = pentry.Attributes.Mime } + sses3Info, sses3Err := extractMultipartSSES3Info(pentry) + if sses3Err != nil { + glog.Errorf("completeMultipartUpload %s %s SSE-S3 metadata error: %v", *input.Bucket, *input.UploadId, sses3Err) + return nil, nil, s3err.ErrInternalError + } finalParts := make([]*filer_pb.FileChunk, 0) partBoundaries := make([]multipartPartBoundary, 0, len(completedPartNumbers)) var offset int64 @@ -339,17 +470,12 @@ func (s3a *S3ApiServer) prepareMultipartCompletionState(r *http.Request, input * partETag := filer.ETag(entry) for _, chunk := range entry.GetChunks() { - finalParts = append(finalParts, &filer_pb.FileChunk{ - FileId: chunk.GetFileIdString(), - Offset: offset, - Size: chunk.Size, - ModifiedTsNs: chunk.ModifiedTsNs, - CipherKey: chunk.CipherKey, - ETag: chunk.ETag, - IsCompressed: chunk.IsCompressed, - SseType: chunk.SseType, - SseMetadata: chunk.SseMetadata, - }) + finalChunk, chunkErr := completedMultipartChunk(chunk, offset, sses3Info) + if chunkErr != nil { + glog.Errorf("completeMultipartUpload %s %s SSE-S3 chunk metadata error: %v", *input.Bucket, *input.UploadId, chunkErr) + return nil, nil, s3err.ErrInternalError + } + finalParts = append(finalParts, finalChunk) offset += int64(chunk.Size) } @@ -387,6 +513,7 @@ func (s3a *S3ApiServer) prepareMultipartCompletionState(r *http.Request, input * deleteEntries: deleteEntries, partEntries: partEntries, pentry: pentry, + sses3Info: sses3Info, mime: mime, finalParts: finalParts, offset: offset, @@ -493,6 +620,7 @@ func (s3a *S3ApiServer) completeMultipartUpload(r *http.Request, input *s3.Compl firstPartEntry := completionState.partEntries[completedPartNumbers[0]][0] copySSEHeadersFromFirstPart(versionEntry, firstPartEntry, "versioned") } + applyMultipartSSES3HeadersFromUploadEntry(versionEntry, completionState.sses3Info) if completionState.pentry.Attributes != nil && completionState.pentry.Attributes.Mime != "" { versionEntry.Attributes.Mime = completionState.pentry.Attributes.Mime } else if completionState.mime != "" { @@ -578,6 +706,7 @@ func (s3a *S3ApiServer) completeMultipartUpload(r *http.Request, input *s3.Compl firstPartEntry := completionState.partEntries[completedPartNumbers[0]][0] copySSEHeadersFromFirstPart(entry, firstPartEntry, "suspended versioning") } + applyMultipartSSES3HeadersFromUploadEntry(entry, completionState.sses3Info) // Persist ETag to ensure subsequent HEAD/GET uses the same value entry.Extended[s3_constants.ExtETagKey] = []byte(completionState.multipartETag) // Store composite checksum if computed from per-part checksums @@ -640,6 +769,7 @@ func (s3a *S3ApiServer) completeMultipartUpload(r *http.Request, input *s3.Compl firstPartEntry := completionState.partEntries[completedPartNumbers[0]][0] copySSEHeadersFromFirstPart(entry, firstPartEntry, "non-versioned") } + applyMultipartSSES3HeadersFromUploadEntry(entry, completionState.sses3Info) // Persist ETag to ensure subsequent HEAD/GET uses the same value entry.Extended[s3_constants.ExtETagKey] = []byte(completionState.multipartETag) // Store composite checksum if computed from per-part checksums diff --git a/weed/s3api/filer_multipart_sse_s3_test.go b/weed/s3api/filer_multipart_sse_s3_test.go new file mode 100644 index 000000000..38f6f07f5 --- /dev/null +++ b/weed/s3api/filer_multipart_sse_s3_test.go @@ -0,0 +1,346 @@ +package s3api + +import ( + "bytes" + "crypto/rand" + "encoding/base64" + "io" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" +) + +func TestCompletedMultipartChunkBackfillsSSES3MetadataFromUploadEntry(t *testing.T) { + keyManager := initSSES3KeyManagerForTest(t) + + key, err := GenerateSSES3Key() + if err != nil { + t.Fatalf("GenerateSSES3Key: %v", err) + } + keyData, err := SerializeSSES3Metadata(key) + if err != nil { + t.Fatalf("SerializeSSES3Metadata: %v", err) + } + + baseIV := []byte("1234567890abcdef") + uploadEntry := &filer_pb.Entry{ + Extended: map[string][]byte{ + s3_constants.SeaweedFSSSES3Encryption: []byte(s3_constants.SSEAlgorithmAES256), + s3_constants.SeaweedFSSSES3BaseIV: []byte(base64.StdEncoding.EncodeToString(baseIV)), + s3_constants.SeaweedFSSSES3KeyData: []byte(base64.StdEncoding.EncodeToString(keyData)), + }, + } + + sses3Info, err := extractMultipartSSES3Info(uploadEntry) + if err != nil { + t.Fatalf("extractMultipartSSES3Info: %v", err) + } + + partLocalOffset := int64(8 * 1024 * 1024) + finalObjectOffset := int64(40 * 1024 * 1024) + chunk := &filer_pb.FileChunk{ + FileId: "1,abc", + Offset: partLocalOffset, + Size: 1024, + } + + finalChunk, err := completedMultipartChunk(chunk, finalObjectOffset, sses3Info) + if err != nil { + t.Fatalf("completedMultipartChunk: %v", err) + } + if finalChunk.GetOffset() != finalObjectOffset { + t.Fatalf("final chunk offset = %d, want %d", finalChunk.GetOffset(), finalObjectOffset) + } + if finalChunk.GetSseType() != filer_pb.SSEType_SSE_S3 { + t.Fatalf("final chunk SSE type = %s, want SSE_S3", finalChunk.GetSseType()) + } + if len(finalChunk.GetSseMetadata()) == 0 { + t.Fatal("expected final chunk SSE metadata") + } + + chunkKey, err := DeserializeSSES3Metadata(finalChunk.GetSseMetadata(), keyManager) + if err != nil { + t.Fatalf("DeserializeSSES3Metadata: %v", err) + } + if !bytes.Equal(chunkKey.Key, key.Key) { + t.Fatal("chunk metadata key does not match upload key") + } + expectedIV, _ := calculateIVWithOffset(baseIV, partLocalOffset) + if !bytes.Equal(chunkKey.IV, expectedIV) { + t.Fatalf("chunk metadata IV = %x, want %x", chunkKey.IV, expectedIV) + } +} + +func TestCompletedMultipartChunkPreservesExistingSSES3Metadata(t *testing.T) { + existingMetadata := []byte("already-present") + chunk := &filer_pb.FileChunk{ + FileId: "2,abc", + Offset: 1024, + Size: 2048, + SseType: filer_pb.SSEType_SSE_S3, + SseMetadata: existingMetadata, + } + + finalChunk, err := completedMultipartChunk(chunk, 4096, &multipartSSES3Info{}) + if err != nil { + t.Fatalf("completedMultipartChunk: %v", err) + } + if !bytes.Equal(finalChunk.GetSseMetadata(), existingMetadata) { + t.Fatal("existing SSE-S3 metadata should be preserved") + } +} + +// TestApplyMultipartSSES3HeadersFromUploadEntry pins the canonical object-level +// SSE-S3 attributes onto the completed entry: SeaweedFSSSES3Key (used by +// IsSSES3EncryptedInternal and the read-side decryption-key lookup) and +// X-Amz-Server-Side-Encryption (used by detectPrimarySSEType for non-chunked +// reads). Pre-existing values must not be clobbered. +func TestApplyMultipartSSES3HeadersFromUploadEntry(t *testing.T) { + initSSES3KeyManagerForTest(t) + + key, err := GenerateSSES3Key() + if err != nil { + t.Fatalf("GenerateSSES3Key: %v", err) + } + keyData, err := SerializeSSES3Metadata(key) + if err != nil { + t.Fatalf("SerializeSSES3Metadata: %v", err) + } + + baseIV := []byte("1234567890abcdef") + uploadEntry := &filer_pb.Entry{ + Extended: map[string][]byte{ + s3_constants.SeaweedFSSSES3Encryption: []byte(s3_constants.SSEAlgorithmAES256), + s3_constants.SeaweedFSSSES3BaseIV: []byte(base64.StdEncoding.EncodeToString(baseIV)), + s3_constants.SeaweedFSSSES3KeyData: []byte(base64.StdEncoding.EncodeToString(keyData)), + }, + } + sses3Info, err := extractMultipartSSES3Info(uploadEntry) + if err != nil { + t.Fatalf("extractMultipartSSES3Info: %v", err) + } + + t.Run("backfills missing canonical attributes", func(t *testing.T) { + finalEntry := &filer_pb.Entry{Extended: map[string][]byte{}} + applyMultipartSSES3HeadersFromUploadEntry(finalEntry, sses3Info) + if !bytes.Equal(finalEntry.Extended[s3_constants.SeaweedFSSSES3Key], keyData) { + t.Fatal("final entry did not receive object-level SSE-S3 key metadata") + } + if got := string(finalEntry.Extended[s3_constants.AmzServerSideEncryption]); got != s3_constants.SSEAlgorithmAES256 { + t.Fatalf("final entry SSE header = %q, want %q", got, s3_constants.SSEAlgorithmAES256) + } + }) + + t.Run("does not clobber existing canonical attributes", func(t *testing.T) { + existingKey := []byte("existing-key-bytes") + existingHeader := []byte("aws:kms") + finalEntry := &filer_pb.Entry{Extended: map[string][]byte{ + s3_constants.SeaweedFSSSES3Key: existingKey, + s3_constants.AmzServerSideEncryption: existingHeader, + }} + applyMultipartSSES3HeadersFromUploadEntry(finalEntry, sses3Info) + if !bytes.Equal(finalEntry.Extended[s3_constants.SeaweedFSSSES3Key], existingKey) { + t.Fatal("existing SSE-S3 key was overwritten") + } + if !bytes.Equal(finalEntry.Extended[s3_constants.AmzServerSideEncryption], existingHeader) { + t.Fatal("existing X-Amz-Server-Side-Encryption was overwritten") + } + }) + + t.Run("nil info is a no-op", func(t *testing.T) { + finalEntry := &filer_pb.Entry{Extended: map[string][]byte{}} + applyMultipartSSES3HeadersFromUploadEntry(finalEntry, nil) + if len(finalEntry.Extended) != 0 { + t.Fatalf("nil sses3Info should not write any keys, got %d", len(finalEntry.Extended)) + } + }) +} + +// TestCompletedMultipartChunkBackfilledIVDecryptsActualCiphertext is a round +// trip across the encryption boundary: it encrypts simulated multipart parts +// using the same call path putToFiler uses (CreateSSES3EncryptedReaderWithBaseIV +// with partOffset=0, then chunked at fixed boundaries), throws away the chunk +// level SSE metadata to simulate the bug from #8908, runs completedMultipartChunk +// to backfill it, and decrypts each chunk's slice of ciphertext with the +// backfilled IV. The plaintext must come back intact. +// +// Regression coverage for the gemini review on #9224 which suggested using +// (partNumber-1)*PartOffsetMultiplier + chunk.Offset for the backfill IV. That +// formula does not match what the encryption side actually does (PartOffsetMultiplier +// is defined but not used in the write path), so adopting it would silently +// produce backfilled IVs that fail to decrypt the bytes on disk. +func TestCompletedMultipartChunkBackfilledIVDecryptsActualCiphertext(t *testing.T) { + keyManager := initSSES3KeyManagerForTest(t) + + key, err := GenerateSSES3Key() + if err != nil { + t.Fatalf("GenerateSSES3Key: %v", err) + } + keyData, err := SerializeSSES3Metadata(key) + if err != nil { + t.Fatalf("SerializeSSES3Metadata: %v", err) + } + + baseIV := make([]byte, s3_constants.AESBlockSize) + if _, err := rand.Read(baseIV); err != nil { + t.Fatalf("rand.Read baseIV: %v", err) + } + + uploadEntry := &filer_pb.Entry{ + Extended: map[string][]byte{ + s3_constants.SeaweedFSSSES3Encryption: []byte(s3_constants.SSEAlgorithmAES256), + s3_constants.SeaweedFSSSES3BaseIV: []byte(base64.StdEncoding.EncodeToString(baseIV)), + s3_constants.SeaweedFSSSES3KeyData: []byte(base64.StdEncoding.EncodeToString(keyData)), + }, + } + sses3Info, err := extractMultipartSSES3Info(uploadEntry) + if err != nil { + t.Fatalf("extractMultipartSSES3Info: %v", err) + } + + const chunkSize = int64(8 * 1024 * 1024) + + // Two parts: part 1 spans three internal chunks (offsets 0, 8MB, 16MB), + // part 2 spans two (offsets 0, 8MB). Both parts are encrypted independently + // with partOffset=0, the way putToFiler invokes the encryption path today. + parts := []struct { + name string + plaintext []byte + }{ + {"part1", makeRandomPlaintext(t, int(chunkSize)*3-1234)}, + {"part2", makeRandomPlaintext(t, int(chunkSize)*2+5678)}, + } + + for _, p := range parts { + t.Run(p.name, func(t *testing.T) { + // Encrypt the part stream the same way handleSSES3MultipartEncryption does. + encReader, _, err := CreateSSES3EncryptedReaderWithBaseIV(bytes.NewReader(p.plaintext), key, baseIV, 0) + if err != nil { + t.Fatalf("CreateSSES3EncryptedReaderWithBaseIV: %v", err) + } + ciphertext, err := io.ReadAll(encReader) + if err != nil { + t.Fatalf("read encrypted stream: %v", err) + } + if len(ciphertext) != len(p.plaintext) { + t.Fatalf("ciphertext length %d != plaintext length %d", len(ciphertext), len(p.plaintext)) + } + + // Slice the encrypted stream into chunks the same way UploadReaderInChunks + // would, and build FileChunks tagged SseType=NONE to simulate the bug. + var bugChunks []*filer_pb.FileChunk + for off := int64(0); off < int64(len(ciphertext)); off += chunkSize { + end := off + chunkSize + if end > int64(len(ciphertext)) { + end = int64(len(ciphertext)) + } + bugChunks = append(bugChunks, &filer_pb.FileChunk{ + FileId: "1,deadbeef", + Offset: off, + Size: uint64(end - off), + // SseType deliberately left at default NONE. + }) + } + + // finalOffset doesn't influence the IV calculation, only the chunk's + // position in the assembled object. Use a fake non-zero value to + // confirm the function does not accidentally use it for the IV. + const finalOffset int64 = 1 << 40 + + for _, bugChunk := range bugChunks { + finalChunk, err := completedMultipartChunk(bugChunk, finalOffset+bugChunk.Offset, sses3Info) + if err != nil { + t.Fatalf("completedMultipartChunk(offset=%d): %v", bugChunk.Offset, err) + } + if finalChunk.GetSseType() != filer_pb.SSEType_SSE_S3 { + t.Fatalf("chunk@%d not retagged SSE_S3", bugChunk.Offset) + } + meta, err := DeserializeSSES3Metadata(finalChunk.GetSseMetadata(), keyManager) + if err != nil { + t.Fatalf("DeserializeSSES3Metadata(chunk@%d): %v", bugChunk.Offset, err) + } + + cipherSlice := ciphertext[bugChunk.Offset : bugChunk.Offset+int64(bugChunk.Size)] + wantPlain := p.plaintext[bugChunk.Offset : bugChunk.Offset+int64(bugChunk.Size)] + + decReader, err := CreateSSES3DecryptedReader(bytes.NewReader(cipherSlice), key, meta.IV) + if err != nil { + t.Fatalf("CreateSSES3DecryptedReader(chunk@%d): %v", bugChunk.Offset, err) + } + gotPlain, err := io.ReadAll(decReader) + if err != nil { + t.Fatalf("read decrypted chunk@%d: %v", bugChunk.Offset, err) + } + if !bytes.Equal(gotPlain, wantPlain) { + t.Fatalf("chunk@%d: backfilled IV produced wrong plaintext (first mismatch byte at %d)", + bugChunk.Offset, firstMismatch(gotPlain, wantPlain)) + } + } + }) + } +} + +// TestCompletedMultipartChunkRejectsPartNumberMultiplierFormula is a guard +// against re-introducing the gemini review's suggested formula, which would +// produce IVs incompatible with the encryption side. The test simulates what +// would happen if the backfill used (partNumber-1)*PartOffsetMultiplier as an +// extra offset term: the resulting IV does not decrypt the actual ciphertext. +func TestCompletedMultipartChunkRejectsPartNumberMultiplierFormula(t *testing.T) { + key, err := GenerateSSES3Key() + if err != nil { + t.Fatalf("GenerateSSES3Key: %v", err) + } + baseIV := make([]byte, s3_constants.AESBlockSize) + if _, err := rand.Read(baseIV); err != nil { + t.Fatalf("rand.Read baseIV: %v", err) + } + + plaintext := makeRandomPlaintext(t, 4096) + + encReader, _, err := CreateSSES3EncryptedReaderWithBaseIV(bytes.NewReader(plaintext), key, baseIV, 0) + if err != nil { + t.Fatalf("CreateSSES3EncryptedReaderWithBaseIV: %v", err) + } + ciphertext, err := io.ReadAll(encReader) + if err != nil { + t.Fatalf("read encrypted stream: %v", err) + } + + // "partNumber=2" with PartOffsetMultiplier added would shift the IV by 8GB. + wrongIV, _ := calculateIVWithOffset(baseIV, s3_constants.PartOffsetMultiplier+0) + + decReader, err := CreateSSES3DecryptedReader(bytes.NewReader(ciphertext), key, wrongIV) + if err != nil { + t.Fatalf("CreateSSES3DecryptedReader: %v", err) + } + got, err := io.ReadAll(decReader) + if err != nil { + t.Fatalf("read decrypted: %v", err) + } + if bytes.Equal(got, plaintext) { + t.Fatal("partNumber-multiplier IV decrypted plaintext correctly; the backfill formula must NOT include this term") + } +} + +func makeRandomPlaintext(t *testing.T, n int) []byte { + t.Helper() + b := make([]byte, n) + if _, err := rand.Read(b); err != nil { + t.Fatalf("rand.Read: %v", err) + } + return b +} + +func firstMismatch(a, b []byte) int { + n := len(a) + if len(b) < n { + n = len(b) + } + for i := 0; i < n; i++ { + if a[i] != b[i] { + return i + } + } + return n +} diff --git a/weed/s3api/s3api_object_handlers.go b/weed/s3api/s3api_object_handlers.go index 3c350a8fe..40ea157fa 100644 --- a/weed/s3api/s3api_object_handlers.go +++ b/weed/s3api/s3api_object_handlers.go @@ -1648,44 +1648,15 @@ func (s3a *S3ApiServer) decryptSSEKMSChunkView(ctx context.Context, fileChunk *f return nil, fmt.Errorf("failed to fetch full chunk: %w", err) } - // IMPORTANT: Calculate adjusted IV using ChunkOffset - // SSE-KMS uses base IV + offset calculation (unlike SSE-C which uses random IVs) - // This reconstructs the same IV that was used during encryption - var adjustedIV []byte - var ivSkip int - if sseKMSKey.ChunkOffset > 0 { - adjustedIV, ivSkip = calculateIVWithOffset(sseKMSKey.IV, sseKMSKey.ChunkOffset) - } else { - adjustedIV = sseKMSKey.IV - ivSkip = 0 - } - - adjustedKey := &SSEKMSKey{ - KeyID: sseKMSKey.KeyID, - EncryptedDataKey: sseKMSKey.EncryptedDataKey, - EncryptionContext: sseKMSKey.EncryptionContext, - BucketKeyEnabled: sseKMSKey.BucketKeyEnabled, - IV: adjustedIV, - ChunkOffset: sseKMSKey.ChunkOffset, - } - - decryptedReader, decryptErr := CreateSSEKMSDecryptedReader(fullChunkReader, adjustedKey) + // CreateSSEKMSDecryptedReader applies ChunkOffset to the stored base IV. + // Passing a pre-adjusted IV here would apply the offset twice and corrupt + // range reads that cross multipart chunk boundaries. + decryptedReader, decryptErr := CreateSSEKMSDecryptedReader(fullChunkReader, sseKMSKey) if decryptErr != nil { fullChunkReader.Close() return nil, fmt.Errorf("failed to create KMS decrypted reader: %w", decryptErr) } - // CRITICAL: Skip intra-block bytes from CTR decryption (non-block-aligned offset handling) - if ivSkip > 0 { - _, err = io.CopyN(io.Discard, decryptedReader, int64(ivSkip)) - if err != nil { - if closer, ok := decryptedReader.(io.Closer); ok { - closer.Close() - } - return nil, fmt.Errorf("failed to skip intra-block bytes (%d): %w", ivSkip, err) - } - } - // Skip to position and limit to ViewSize if chunkView.OffsetInChunk > 0 { _, err = io.CopyN(io.Discard, decryptedReader, chunkView.OffsetInChunk) diff --git a/weed/s3api/s3api_sse_decrypt_test.go b/weed/s3api/s3api_sse_decrypt_test.go index f66a89ebd..8feb3988b 100644 --- a/weed/s3api/s3api_sse_decrypt_test.go +++ b/weed/s3api/s3api_sse_decrypt_test.go @@ -161,6 +161,27 @@ func TestSSEKMSDecryptChunkView_RequiresOffsetAdjustment(t *testing.T) { t.Logf("✓ SSE-KMS decryption with offset adjustment successful") } + // ANTI-TEST: Decrypt after applying the offset twice (incorrect range path behavior) + doubleAdjustedIV, doubleSkip := calculateIVWithOffset(adjustedIV, chunkOffset) + blockDoubleWrong, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("Failed to create cipher for double-adjusted decryption: %v", err) + } + + decryptedDoubleWrong := make([]byte, len(ciphertext)) + streamDoubleWrong := cipher.NewCTR(blockDoubleWrong, doubleAdjustedIV) + if doubleSkip > 0 { + dummy := make([]byte, doubleSkip) + streamDoubleWrong.XORKeyStream(dummy, dummy) + } + streamDoubleWrong.XORKeyStream(decryptedDoubleWrong, ciphertext) + + if bytes.Equal(decryptedDoubleWrong, plaintext) { + t.Errorf("CRITICAL: Double-adjusted IV produced correct plaintext! SSE-KMS chunk offset must be applied exactly once.") + } else { + t.Logf("✓ Verified: double-adjusted IV produces corrupted data (range path must not pre-adjust)") + } + // ANTI-TEST: Decrypt using base IV directly (incorrect for SSE-KMS) blockWrong, err := aes.NewCipher(key) if err != nil {