fix(s3api): backfill multipart SSE-S3 metadata at completion (#9224)

* fix(s3api): backfill missing per-chunk SSE-S3 metadata at completion

When a part of an SSE-S3 multipart upload lands with SseType=NONE on
its chunks (e.g. a transient failure to apply SSE-S3 setup in
PutObjectPart), the completed object inherits NONE-tagged chunks and
detectPrimarySSEType then misses the chunked SSE-S3 encryption. The
read path falls through to the unencrypted serve and GET returns
ciphertext, producing the SHA mismatch reported in #8908.

Recover at completion using the base IV and key data the upload
directory recorded at CreateMultipartUpload:

  - extractMultipartSSES3Info validates upload-entry metadata up
    front and hard-fails completion if the base IV or key data are
    malformed; serializing chunk metadata we then could not decrypt
    is worse than rejecting the upload.
  - completedMultipartChunk re-derives a per-chunk IV from baseIV +
    chunk.Offset (matching what putToFiler would have written) and
    serializes per-chunk SSE-S3 metadata when the chunk has no tag.
    Existing per-chunk metadata is left alone; we cannot recover an
    already-derived IV from the upload-entry alone.

The IV formula intentionally has no partNumber term: putToFiler
hardcodes partOffset=0 when it calls handleSSES3MultipartEncryption
for every part, so each chunk's encryption IV is
calculateIVWithOffset(baseIV, chunk.Offset_part_local).
PartOffsetMultiplier is defined in s3_constants but is not consumed
by the encryption path. Adopting (partNumber-1)*PartOffsetMultiplier
+ chunk.Offset would produce IVs that fail to decrypt the bytes on
disk - a stronger failure mode than the bug being fixed. Tests pin
this:

  - TestCompletedMultipartChunkBackfilledIVDecryptsActualCiphertext
    runs the round trip across the encryption boundary: encrypt
    parts with CreateSSES3EncryptedReaderWithBaseIV (the call
    putToFiler uses), drop chunk metadata to reproduce #8908,
    backfill, decrypt with backfilled IV, assert plaintext intact.
  - TestCompletedMultipartChunkRejectsPartNumberMultiplierFormula
    constructs the IV the partNumber formula would produce and
    shows it does not decrypt the actual ciphertext.

This commit covers the chunk-level recovery only. The companion
fix for the object-level Extended attributes (SeaweedFSSSES3Key /
X-Amz-Server-Side-Encryption) follows separately.

* fix(s3api): backfill canonical SSE-S3 attributes onto multipart object

The previous commit ensures every chunk of an SSE-S3 multipart upload
carries SseType=SSE_S3 with a per-chunk IV, so the multipart-direct
read path can decrypt. The completed object's Extended map can still
miss the canonical pair detectPrimarySSEType and IsSSES3EncryptedInternal
look at:

  - X-Amz-Server-Side-Encryption (the AmzServerSideEncryption header
    detectPrimarySSEType reads on inline / small-object reads)
  - x-seaweedfs-sse-s3-key (SeaweedFSSSES3Key, required by
    IsSSES3EncryptedInternal and by the read-path key lookup)

When a part of the upload was written by a path that did not set
those (the same #8908 race that produced the NONE chunks),
copySSEHeadersFromFirstPart finds nothing to copy and the final entry
ends up with only the multipart-init keys (SeaweedFSSSES3Encryption /
BaseIV / KeyData). The read path then mis-detects the object as
unencrypted.

applyMultipartSSES3HeadersFromUploadEntry writes the canonical pair
from the multipart-init metadata in all three completion paths
(versioned, suspended, non-versioned), only when the keys are missing
so a healthy first part still wins. extractMultipartSSES3Info already
ran in prepareMultipartCompletionState, so the data is reused without
re-decoding.

Tests: TestApplyMultipartSSES3HeadersFromUploadEntry covers backfill,
do-not-clobber, and nil-info no-op cases.

* fix(s3api): drop double IV adjustment in SSE-KMS chunk view decrypt

decryptSSEKMSChunkView was pre-adjusting the SSE-KMS chunk IV
(calculateIVWithOffset(baseIV, ChunkOffset)) and then handing the
adjusted IV to CreateSSEKMSDecryptedReader, which itself runs
calculateIVWithOffset(IV, ChunkOffset) on whatever it receives. The
offset was being applied twice for any chunk with a non-zero
ChunkOffset, corrupting the keystream for range reads that cross
multipart chunk boundaries.

Pass the raw SSE-KMS key (with base IV and the original ChunkOffset
field) into CreateSSEKMSDecryptedReader so the offset is applied
exactly once, and remove the now-dead intra-block skip that was
compensating for the double adjustment.

Add an anti-test inside TestSSEKMSDecryptChunkView_RequiresOffsetAdjustment
that decrypts the same ciphertext with a deliberately double-adjusted
IV and asserts the output is corrupted, so any regression that
re-introduces the double application fails the unit test.

* test(s3): cover multipart SSE across chunk-spanning parts and ranges

Adds an integration subtest "Multipart Parts Larger Than Internal
Chunks Across SSE Types" to TestSSEMultipartUploadIntegration that
exercises the end-to-end S3 path for the bugs fixed in this branch:

  - Two-part multipart upload with each part larger than the 8MB
    internal SeaweedFS chunk, so each part itself spans multiple
    underlying chunks.
  - Subtests for SSE-C, SSE-KMS, explicit SSE-S3, and bucket-default
    SSE-S3 - the four paths multipart parts can take through the SSE
    pipeline.
  - Each subtest does a full GET (verifying every byte and the
    response Content-Length / SSE response headers) plus a 129-byte
    range read straddling the 8MB internal chunk boundary, which is
    the path that produced the SSE-KMS double-IV corruption (fix in
    the previous commit) and the SSE-S3 chunk-tag loss (fix in the
    earlier commits).

Factored the request shape behind multipartSSEOptions /
uploadAndVerifyMultipartSSEObject so all four SSE flavors share the
same upload+verify code; only the SSE-specific input/output
configuration differs per subtest.

* test(s3): abort orphan multipart uploads on test failure

Address coderabbit nitpick on uploadAndVerifyMultipartSSEObject. The
helper used require.NoError after CreateMultipartUpload, UploadPart
and CompleteMultipartUpload, so a failure in any of those (or in the
later GET / range read on a still-incomplete upload) called t.Fatal
without aborting the in-flight MPU, leaving an orphan upload in the
bucket. Harmless in CI where the data dir is wiped on shutdown, but a
real annoyance when iterating locally and a textbook AWS S3 caveat in
production.

Register a t.Cleanup that calls AbortMultipartUpload unless a
"completed" flag was set right after a successful
CompleteMultipartUpload. Use context.Background for the abort call
since the parent ctx may already be cancelled at cleanup time, and
t.Logf the abort error rather than failing the test so the original
failure remains visible in the run output.
This commit is contained in:
Chris Lu 2026-04-25 23:06:37 -07:00 committed by GitHub
parent 5eead9409a
commit 525900dfe4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 706 additions and 44 deletions

View File

@ -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

View File

@ -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

View File

@ -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
}

View File

@ -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)

View File

@ -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 {