Skip to content

Commit c2bfd7b

Browse files
authored
fix: honor SSE-C chunk offsets in decryption for large chunked uploads (seaweedfs#8216)
* fix: honor SSE-C chunk offsets in decryption for large chunked uploads Fixes issue seaweedfs#8215 where SSE-C decryption for large objects could corrupt data by ignoring per-chunk PartOffset values. Changes: - Add TestSSECLargeObjectChunkReassembly unit test to verify correct decryption of 19MB object split into 8MB chunks using PartOffset - Update decryptSSECChunkView and createMultipartSSECDecryptedReaderDirect to extract PartOffset from SSE-C metadata and pass to CreateSSECDecryptedReaderWithOffset for offset-aware decryption - Fix createCTRStreamWithOffset to use calculateIVWithOffset for proper block-aligned counter advancement, matching SSE-KMS/S3 behavior - Update comments to clarify SSE-C IV handling uses per-chunk offsets (unlike base IV approach used by KMS/S3) All tests pass: go test ./weed/s3api ✓ * fix: close chunkReader on error paths in createMultipartSSECDecryptedReader Address resource leak issue reported in PR seaweedfs#8216: ensure chunkReader is properly closed before returning on all error paths, including: - DeserializeSSECMetadata failures - IV decoding errors - Invalid PartOffset values - SSE-C reader creation failures - Missing per-chunk metadata This prevents leaking network connections and file handles during SSE-C multipart decryption error scenarios. * docs: clarify SSE-C IV handling in decryptSSECChunkView comment Replace misleading warning 'Do NOT call calculateIVWithOffset' with accurate explanation that: - CreateSSECDecryptedReaderWithOffset internally uses calculateIVWithOffset to advance the CTR counter to reach PartOffset - calculateIVWithOffset is applied only to the per-part IV, NOT to derive a global base IV for all parts - This differs fundamentally from SSE-KMS/SSE-S3 which use base IV + calculateIVWithOffset(ChunkOffset) This clarifies the IV advancement mechanism while contrasting it with the base IV approach used by other encryption schemes.
1 parent 19c18d8 commit c2bfd7b

File tree

3 files changed

+95
-33
lines changed

3 files changed

+95
-33
lines changed

weed/s3api/s3_sse_c.go

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -263,18 +263,13 @@ func CreateSSECDecryptedReaderWithOffset(r io.Reader, customerKey *SSECustomerKe
263263

264264
// createCTRStreamWithOffset creates a CTR stream positioned at a specific counter offset
265265
func createCTRStreamWithOffset(block cipher.Block, iv []byte, counterOffset uint64) cipher.Stream {
266-
// Create a copy of the IV to avoid modifying the original
267-
offsetIV := make([]byte, len(iv))
268-
copy(offsetIV, iv)
269-
270-
// Calculate the counter offset in blocks (AES block size is 16 bytes)
271-
blockOffset := counterOffset / 16
272-
273-
// Add the block offset to the counter portion of the IV
274-
// In AES-CTR, the last 8 bytes of the IV are typically used as the counter
275-
addCounterToIV(offsetIV, blockOffset)
276-
277-
return cipher.NewCTR(block, offsetIV)
266+
adjustedIV, skip := calculateIVWithOffset(iv, int64(counterOffset))
267+
stream := cipher.NewCTR(block, adjustedIV)
268+
if skip > 0 {
269+
dummy := make([]byte, skip)
270+
stream.XORKeyStream(dummy, dummy)
271+
}
272+
return stream
278273
}
279274

280275
// addCounterToIV adds a counter value to the IV (treating last 8 bytes as big-endian counter)

weed/s3api/s3_sse_multipart_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,58 @@ func TestMultipartSSEMixedScenarios(t *testing.T) {
422422
})
423423
}
424424

425+
func TestSSECLargeObjectChunkReassembly(t *testing.T) {
426+
keyPair := GenerateTestSSECKey(1)
427+
customerKey := &SSECustomerKey{
428+
Algorithm: "AES256",
429+
Key: keyPair.Key,
430+
KeyMD5: keyPair.KeyMD5,
431+
}
432+
433+
const chunkSize = 8 * 1024 * 1024 // matches putToFiler chunk size
434+
totalSize := chunkSize*2 + 3*1024*1024
435+
plaintext := make([]byte, totalSize)
436+
for i := range plaintext {
437+
plaintext[i] = byte(i % 251)
438+
}
439+
440+
encryptedReader, iv, err := CreateSSECEncryptedReader(bytes.NewReader(plaintext), customerKey)
441+
if err != nil {
442+
t.Fatalf("Failed to create encrypted reader: %v", err)
443+
}
444+
encryptedData, err := io.ReadAll(encryptedReader)
445+
if err != nil {
446+
t.Fatalf("Failed to read encrypted data: %v", err)
447+
}
448+
449+
var reconstructed bytes.Buffer
450+
offset := int64(0)
451+
for offset < int64(len(encryptedData)) {
452+
end := offset + chunkSize
453+
if end > int64(len(encryptedData)) {
454+
end = int64(len(encryptedData))
455+
}
456+
457+
chunkIV := make([]byte, len(iv))
458+
copy(chunkIV, iv)
459+
chunkReader := bytes.NewReader(encryptedData[offset:end])
460+
decryptedReader, decErr := CreateSSECDecryptedReaderWithOffset(chunkReader, customerKey, chunkIV, uint64(offset))
461+
if decErr != nil {
462+
t.Fatalf("Failed to create decrypted reader for offset %d: %v", offset, decErr)
463+
}
464+
decryptedChunk, decErr := io.ReadAll(decryptedReader)
465+
if decErr != nil {
466+
t.Fatalf("Failed to read decrypted chunk at offset %d: %v", offset, decErr)
467+
}
468+
reconstructed.Write(decryptedChunk)
469+
offset = end
470+
}
471+
472+
if !bytes.Equal(reconstructed.Bytes(), plaintext) {
473+
t.Fatalf("Reconstructed data mismatch: expected %d bytes, got %d", len(plaintext), reconstructed.Len())
474+
}
475+
}
476+
425477
// TestMultipartSSEPerformance tests performance characteristics of SSE with multipart
426478
func TestMultipartSSEPerformance(t *testing.T) {
427479
if testing.Short() {

weed/s3api/s3api_object_handlers.go

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,17 +1512,18 @@ func writeZeroBytes(w io.Writer, n int64) error {
15121512
//
15131513
// IV Handling for SSE-C:
15141514
// ----------------------
1515-
// SSE-C multipart encryption (see lines 2772-2781) differs fundamentally from SSE-KMS/SSE-S3:
1515+
// SSE-C multipart encryption differs from SSE-KMS/SSE-S3:
15161516
//
1517-
// 1. Encryption: CreateSSECEncryptedReader generates a RANDOM IV per part/chunk
1518-
// - Each part starts with a fresh random IV
1517+
// 1. Encryption: CreateSSECEncryptedReader generates a RANDOM IV per part
1518+
// - Each part starts with a fresh random IV (NOT derived from a base IV)
15191519
// - CTR counter starts from 0 for each part: counter₀, counter₁, counter₂, ...
1520-
// - PartOffset is stored in metadata but NOT applied during encryption
1520+
// - PartOffset is stored in metadata to describe where this chunk sits in that encrypted stream
15211521
//
1522-
// 2. Decryption: Use the stored IV directly WITHOUT offset adjustment
1523-
// - The stored IV already represents the start of this part's encryption
1524-
// - Applying calculateIVWithOffset would shift to counterₙ, misaligning the keystream
1525-
// - Result: XOR with wrong keystream = corrupted plaintext
1522+
// 2. Decryption: Use the stored per-part IV and advance the CTR by PartOffset
1523+
// - CreateSSECDecryptedReaderWithOffset internally uses calculateIVWithOffset to advance
1524+
// the CTR counter to reach PartOffset within the per-part encrypted stream
1525+
// - calculateIVWithOffset is applied to the per-part IV, NOT to derive a global base IV
1526+
// - Do NOT compute a single base IV for all parts (unlike SSE-KMS/SSE-S3)
15261527
//
15271528
// This contrasts with SSE-KMS/SSE-S3 which use: base IV + calculateIVWithOffset(ChunkOffset)
15281529
func (s3a *S3ApiServer) decryptSSECChunkView(ctx context.Context, fileChunk *filer_pb.FileChunk, chunkView *filer.ChunkView, customerKey *SSECustomerKey) (io.Reader, error) {
@@ -1544,11 +1545,14 @@ func (s3a *S3ApiServer) decryptSSECChunkView(ctx context.Context, fileChunk *fil
15441545
return nil, fmt.Errorf("failed to fetch full chunk: %w", err)
15451546
}
15461547

1547-
// CRITICAL: Use stored IV directly WITHOUT offset adjustment
1548-
// The stored IV is the random IV used at encryption time for this specific part
1549-
// SSE-C does NOT apply calculateIVWithOffset during encryption, so we must not apply it during decryption
1550-
// (See documentation above and at lines 2772-2781 for detailed explanation)
1551-
decryptedReader, decryptErr := CreateSSECDecryptedReader(fullChunkReader, customerKey, chunkIV)
1548+
partOffset := ssecMetadata.PartOffset
1549+
if partOffset < 0 {
1550+
fullChunkReader.Close()
1551+
return nil, fmt.Errorf("invalid SSE-C part offset %d for chunk %s", partOffset, chunkView.FileId)
1552+
}
1553+
1554+
// Use stored IV and advance CTR stream by PartOffset within the encrypted stream
1555+
decryptedReader, decryptErr := CreateSSECDecryptedReaderWithOffset(fullChunkReader, customerKey, chunkIV, uint64(partOffset))
15521556
if decryptErr != nil {
15531557
fullChunkReader.Close()
15541558
return nil, fmt.Errorf("failed to create decrypted reader: %w", decryptErr)
@@ -2844,15 +2848,20 @@ func (s3a *S3ApiServer) createMultipartSSECDecryptedReaderDirect(ctx context.Con
28442848

28452849
// Note: SSE-C multipart behavior (differs from SSE-KMS/SSE-S3):
28462850
// - Upload: CreateSSECEncryptedReader generates RANDOM IV per part (no base IV + offset)
2847-
// - Metadata: PartOffset is stored but not used during encryption
2848-
// - Decryption: Use stored random IV directly (no offset adjustment needed)
2851+
// - Metadata: PartOffset tracks position within the encrypted stream
2852+
// - Decryption: Use stored IV and advance CTR stream by PartOffset
28492853
//
28502854
// This differs from:
28512855
// - SSE-KMS/SSE-S3: Use base IV + calculateIVWithOffset(partOffset) during encryption
28522856
// - CopyObject: Applies calculateIVWithOffset to SSE-C (which may be incorrect)
28532857
//
28542858
// TODO: Investigate CopyObject SSE-C PartOffset handling for consistency
2855-
decryptedChunkReader, decErr := CreateSSECDecryptedReader(chunkReader, customerKey, chunkIV)
2859+
partOffset := ssecMetadata.PartOffset
2860+
if partOffset < 0 {
2861+
chunkReader.Close()
2862+
return nil, fmt.Errorf("invalid SSE-C part offset %d for chunk %s", partOffset, chunk.GetFileIdString())
2863+
}
2864+
decryptedChunkReader, decErr := CreateSSECDecryptedReaderWithOffset(chunkReader, customerKey, chunkIV, uint64(partOffset))
28562865
if decErr != nil {
28572866
chunkReader.Close()
28582867
return nil, fmt.Errorf("failed to decrypt chunk: %v", decErr)
@@ -3235,26 +3244,32 @@ func (s3a *S3ApiServer) createMultipartSSECDecryptedReader(r *http.Request, prox
32353244
// Deserialize the SSE-C metadata stored in the unified metadata field
32363245
ssecMetadata, decErr := DeserializeSSECMetadata(chunk.GetSseMetadata())
32373246
if decErr != nil {
3247+
chunkReader.Close()
32383248
return nil, fmt.Errorf("failed to deserialize SSE-C metadata for chunk %s: %v", chunk.GetFileIdString(), decErr)
32393249
}
32403250

32413251
// Decode the IV from the metadata
32423252
iv, ivErr := base64.StdEncoding.DecodeString(ssecMetadata.IV)
32433253
if ivErr != nil {
3254+
chunkReader.Close()
32443255
return nil, fmt.Errorf("failed to decode IV for SSE-C chunk %s: %v", chunk.GetFileIdString(), ivErr)
32453256
}
32463257

3247-
// Note: For multipart SSE-C, each part was encrypted with offset=0
3248-
// So we use the stored IV directly without offset adjustment
3249-
// PartOffset is stored for informational purposes, but encryption uses offset=0
3250-
chunkIV := iv
3258+
partOffset := ssecMetadata.PartOffset
3259+
if partOffset < 0 {
3260+
chunkReader.Close()
3261+
return nil, fmt.Errorf("invalid SSE-C part offset %d for chunk %s", partOffset, chunk.GetFileIdString())
3262+
}
32513263

3252-
decryptedReader, decErr := CreateSSECDecryptedReader(chunkReader, customerKey, chunkIV)
3264+
// Use stored IV and advance CTR stream by PartOffset within the encrypted stream
3265+
decryptedReader, decErr := CreateSSECDecryptedReaderWithOffset(chunkReader, customerKey, iv, uint64(partOffset))
32533266
if decErr != nil {
3267+
chunkReader.Close()
32543268
return nil, fmt.Errorf("failed to create SSE-C decrypted reader for chunk %s: %v", chunk.GetFileIdString(), decErr)
32553269
}
32563270
readers = append(readers, decryptedReader)
32573271
} else {
3272+
chunkReader.Close()
32583273
return nil, fmt.Errorf("SSE-C chunk %s missing required metadata", chunk.GetFileIdString())
32593274
}
32603275
} else {

0 commit comments

Comments
 (0)