Browse Source

fix: honor SSE-C chunk offsets in decryption for large chunked uploads (#8216)

* fix: honor SSE-C chunk offsets in decryption for large chunked uploads

Fixes issue #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 #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.
pull/8207/head
Chris Lu 3 weeks ago
committed by GitHub
parent
commit
c2bfd7b524
No known key found for this signature in database GPG Key ID: B5690EEEBB952194
  1. 19
      weed/s3api/s3_sse_c.go
  2. 52
      weed/s3api/s3_sse_multipart_test.go
  3. 57
      weed/s3api/s3api_object_handlers.go

19
weed/s3api/s3_sse_c.go

@ -263,18 +263,13 @@ func CreateSSECDecryptedReaderWithOffset(r io.Reader, customerKey *SSECustomerKe
// createCTRStreamWithOffset creates a CTR stream positioned at a specific counter offset
func createCTRStreamWithOffset(block cipher.Block, iv []byte, counterOffset uint64) cipher.Stream {
// Create a copy of the IV to avoid modifying the original
offsetIV := make([]byte, len(iv))
copy(offsetIV, iv)
// Calculate the counter offset in blocks (AES block size is 16 bytes)
blockOffset := counterOffset / 16
// Add the block offset to the counter portion of the IV
// In AES-CTR, the last 8 bytes of the IV are typically used as the counter
addCounterToIV(offsetIV, blockOffset)
return cipher.NewCTR(block, offsetIV)
adjustedIV, skip := calculateIVWithOffset(iv, int64(counterOffset))
stream := cipher.NewCTR(block, adjustedIV)
if skip > 0 {
dummy := make([]byte, skip)
stream.XORKeyStream(dummy, dummy)
}
return stream
}
// addCounterToIV adds a counter value to the IV (treating last 8 bytes as big-endian counter)

52
weed/s3api/s3_sse_multipart_test.go

@ -422,6 +422,58 @@ func TestMultipartSSEMixedScenarios(t *testing.T) {
})
}
func TestSSECLargeObjectChunkReassembly(t *testing.T) {
keyPair := GenerateTestSSECKey(1)
customerKey := &SSECustomerKey{
Algorithm: "AES256",
Key: keyPair.Key,
KeyMD5: keyPair.KeyMD5,
}
const chunkSize = 8 * 1024 * 1024 // matches putToFiler chunk size
totalSize := chunkSize*2 + 3*1024*1024
plaintext := make([]byte, totalSize)
for i := range plaintext {
plaintext[i] = byte(i % 251)
}
encryptedReader, iv, err := CreateSSECEncryptedReader(bytes.NewReader(plaintext), customerKey)
if err != nil {
t.Fatalf("Failed to create encrypted reader: %v", err)
}
encryptedData, err := io.ReadAll(encryptedReader)
if err != nil {
t.Fatalf("Failed to read encrypted data: %v", err)
}
var reconstructed bytes.Buffer
offset := int64(0)
for offset < int64(len(encryptedData)) {
end := offset + chunkSize
if end > int64(len(encryptedData)) {
end = int64(len(encryptedData))
}
chunkIV := make([]byte, len(iv))
copy(chunkIV, iv)
chunkReader := bytes.NewReader(encryptedData[offset:end])
decryptedReader, decErr := CreateSSECDecryptedReaderWithOffset(chunkReader, customerKey, chunkIV, uint64(offset))
if decErr != nil {
t.Fatalf("Failed to create decrypted reader for offset %d: %v", offset, decErr)
}
decryptedChunk, decErr := io.ReadAll(decryptedReader)
if decErr != nil {
t.Fatalf("Failed to read decrypted chunk at offset %d: %v", offset, decErr)
}
reconstructed.Write(decryptedChunk)
offset = end
}
if !bytes.Equal(reconstructed.Bytes(), plaintext) {
t.Fatalf("Reconstructed data mismatch: expected %d bytes, got %d", len(plaintext), reconstructed.Len())
}
}
// TestMultipartSSEPerformance tests performance characteristics of SSE with multipart
func TestMultipartSSEPerformance(t *testing.T) {
if testing.Short() {

57
weed/s3api/s3api_object_handlers.go

@ -1512,17 +1512,18 @@ func writeZeroBytes(w io.Writer, n int64) error {
//
// IV Handling for SSE-C:
// ----------------------
// SSE-C multipart encryption (see lines 2772-2781) differs fundamentally from SSE-KMS/SSE-S3:
// SSE-C multipart encryption differs from SSE-KMS/SSE-S3:
//
// 1. Encryption: CreateSSECEncryptedReader generates a RANDOM IV per part/chunk
// - Each part starts with a fresh random IV
// 1. Encryption: CreateSSECEncryptedReader generates a RANDOM IV per part
// - Each part starts with a fresh random IV (NOT derived from a base IV)
// - CTR counter starts from 0 for each part: counter₀, counter₁, counter₂, ...
// - PartOffset is stored in metadata but NOT applied during encryption
// - PartOffset is stored in metadata to describe where this chunk sits in that encrypted stream
//
// 2. Decryption: Use the stored IV directly WITHOUT offset adjustment
// - The stored IV already represents the start of this part's encryption
// - Applying calculateIVWithOffset would shift to counterₙ, misaligning the keystream
// - Result: XOR with wrong keystream = corrupted plaintext
// 2. Decryption: Use the stored per-part IV and advance the CTR by PartOffset
// - CreateSSECDecryptedReaderWithOffset internally uses calculateIVWithOffset to advance
// the CTR counter to reach PartOffset within the per-part encrypted stream
// - calculateIVWithOffset is applied to the per-part IV, NOT to derive a global base IV
// - Do NOT compute a single base IV for all parts (unlike SSE-KMS/SSE-S3)
//
// This contrasts with SSE-KMS/SSE-S3 which use: base IV + calculateIVWithOffset(ChunkOffset)
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
return nil, fmt.Errorf("failed to fetch full chunk: %w", err)
}
// CRITICAL: Use stored IV directly WITHOUT offset adjustment
// The stored IV is the random IV used at encryption time for this specific part
// SSE-C does NOT apply calculateIVWithOffset during encryption, so we must not apply it during decryption
// (See documentation above and at lines 2772-2781 for detailed explanation)
decryptedReader, decryptErr := CreateSSECDecryptedReader(fullChunkReader, customerKey, chunkIV)
partOffset := ssecMetadata.PartOffset
if partOffset < 0 {
fullChunkReader.Close()
return nil, fmt.Errorf("invalid SSE-C part offset %d for chunk %s", partOffset, chunkView.FileId)
}
// Use stored IV and advance CTR stream by PartOffset within the encrypted stream
decryptedReader, decryptErr := CreateSSECDecryptedReaderWithOffset(fullChunkReader, customerKey, chunkIV, uint64(partOffset))
if decryptErr != nil {
fullChunkReader.Close()
return nil, fmt.Errorf("failed to create decrypted reader: %w", decryptErr)
@ -2844,15 +2848,20 @@ func (s3a *S3ApiServer) createMultipartSSECDecryptedReaderDirect(ctx context.Con
// Note: SSE-C multipart behavior (differs from SSE-KMS/SSE-S3):
// - Upload: CreateSSECEncryptedReader generates RANDOM IV per part (no base IV + offset)
// - Metadata: PartOffset is stored but not used during encryption
// - Decryption: Use stored random IV directly (no offset adjustment needed)
// - Metadata: PartOffset tracks position within the encrypted stream
// - Decryption: Use stored IV and advance CTR stream by PartOffset
//
// This differs from:
// - SSE-KMS/SSE-S3: Use base IV + calculateIVWithOffset(partOffset) during encryption
// - CopyObject: Applies calculateIVWithOffset to SSE-C (which may be incorrect)
//
// TODO: Investigate CopyObject SSE-C PartOffset handling for consistency
decryptedChunkReader, decErr := CreateSSECDecryptedReader(chunkReader, customerKey, chunkIV)
partOffset := ssecMetadata.PartOffset
if partOffset < 0 {
chunkReader.Close()
return nil, fmt.Errorf("invalid SSE-C part offset %d for chunk %s", partOffset, chunk.GetFileIdString())
}
decryptedChunkReader, decErr := CreateSSECDecryptedReaderWithOffset(chunkReader, customerKey, chunkIV, uint64(partOffset))
if decErr != nil {
chunkReader.Close()
return nil, fmt.Errorf("failed to decrypt chunk: %v", decErr)
@ -3235,26 +3244,32 @@ func (s3a *S3ApiServer) createMultipartSSECDecryptedReader(r *http.Request, prox
// Deserialize the SSE-C metadata stored in the unified metadata field
ssecMetadata, decErr := DeserializeSSECMetadata(chunk.GetSseMetadata())
if decErr != nil {
chunkReader.Close()
return nil, fmt.Errorf("failed to deserialize SSE-C metadata for chunk %s: %v", chunk.GetFileIdString(), decErr)
}
// Decode the IV from the metadata
iv, ivErr := base64.StdEncoding.DecodeString(ssecMetadata.IV)
if ivErr != nil {
chunkReader.Close()
return nil, fmt.Errorf("failed to decode IV for SSE-C chunk %s: %v", chunk.GetFileIdString(), ivErr)
}
// Note: For multipart SSE-C, each part was encrypted with offset=0
// So we use the stored IV directly without offset adjustment
// PartOffset is stored for informational purposes, but encryption uses offset=0
chunkIV := iv
partOffset := ssecMetadata.PartOffset
if partOffset < 0 {
chunkReader.Close()
return nil, fmt.Errorf("invalid SSE-C part offset %d for chunk %s", partOffset, chunk.GetFileIdString())
}
decryptedReader, decErr := CreateSSECDecryptedReader(chunkReader, customerKey, chunkIV)
// Use stored IV and advance CTR stream by PartOffset within the encrypted stream
decryptedReader, decErr := CreateSSECDecryptedReaderWithOffset(chunkReader, customerKey, iv, uint64(partOffset))
if decErr != nil {
chunkReader.Close()
return nil, fmt.Errorf("failed to create SSE-C decrypted reader for chunk %s: %v", chunk.GetFileIdString(), decErr)
}
readers = append(readers, decryptedReader)
} else {
chunkReader.Close()
return nil, fmt.Errorf("SSE-C chunk %s missing required metadata", chunk.GetFileIdString())
}
} else {

Loading…
Cancel
Save