From 2c9f6bdae3a1a83c9ea6e3e2da8f9e6f1f44c756 Mon Sep 17 00:00:00 2001 From: chrislu Date: Mon, 17 Nov 2025 16:48:59 -0800 Subject: [PATCH] handle intra-block byte skipping --- weed/s3api/s3_sse_ctr_test.go | 308 +++++++++++++++++++++++ weed/s3api/s3_sse_kms.go | 7 +- weed/s3api/s3_sse_s3.go | 3 +- weed/s3api/s3_sse_utils.go | 15 +- weed/s3api/s3api_object_handlers.go | 30 ++- weed/s3api/s3api_object_handlers_copy.go | 24 +- 6 files changed, 374 insertions(+), 13 deletions(-) create mode 100644 weed/s3api/s3_sse_ctr_test.go diff --git a/weed/s3api/s3_sse_ctr_test.go b/weed/s3api/s3_sse_ctr_test.go new file mode 100644 index 000000000..299f837dc --- /dev/null +++ b/weed/s3api/s3_sse_ctr_test.go @@ -0,0 +1,308 @@ +package s3api + +import ( + "bytes" + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "io" + "testing" +) + +// TestCalculateIVWithOffset tests the calculateIVWithOffset function +func TestCalculateIVWithOffset(t *testing.T) { + baseIV := make([]byte, 16) + rand.Read(baseIV) + + tests := []struct { + name string + offset int64 + expectedSkip int + expectedBlock int64 + }{ + {"BlockAligned_0", 0, 0, 0}, + {"BlockAligned_16", 16, 0, 1}, + {"BlockAligned_32", 32, 0, 2}, + {"BlockAligned_48", 48, 0, 3}, + {"NonAligned_1", 1, 1, 0}, + {"NonAligned_5", 5, 5, 0}, + {"NonAligned_10", 10, 10, 0}, + {"NonAligned_15", 15, 15, 0}, + {"NonAligned_17", 17, 1, 1}, + {"NonAligned_21", 21, 5, 1}, + {"NonAligned_33", 33, 1, 2}, + {"NonAligned_47", 47, 15, 2}, + {"LargeOffset", 1000, 1000 % 16, 1000 / 16}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + adjustedIV, skip := calculateIVWithOffset(baseIV, tt.offset) + + // Verify skip is correct + if skip != tt.expectedSkip { + t.Errorf("calculateIVWithOffset(%d) skip = %d, want %d", tt.offset, skip, tt.expectedSkip) + } + + // Verify IV length is preserved + if len(adjustedIV) != 16 { + t.Errorf("calculateIVWithOffset(%d) IV length = %d, want 16", tt.offset, len(adjustedIV)) + } + + // Verify IV was adjusted correctly (last 8 bytes incremented by blockOffset) + if tt.expectedBlock == 0 { + if !bytes.Equal(adjustedIV, baseIV) { + t.Errorf("calculateIVWithOffset(%d) IV changed when blockOffset=0", tt.offset) + } + } else { + // IV should be different for non-zero block offsets + if bytes.Equal(adjustedIV, baseIV) { + t.Errorf("calculateIVWithOffset(%d) IV not changed when blockOffset=%d", tt.offset, tt.expectedBlock) + } + } + }) + } +} + +// TestCTRDecryptionWithNonBlockAlignedOffset tests that CTR decryption works correctly +// for non-block-aligned offsets (the critical bug fix) +func TestCTRDecryptionWithNonBlockAlignedOffset(t *testing.T) { + // Generate test data + plaintext := make([]byte, 1024) + for i := range plaintext { + plaintext[i] = byte(i % 256) + } + + // Generate random key and IV + key := make([]byte, 32) // AES-256 + iv := make([]byte, 16) + rand.Read(key) + rand.Read(iv) + + // Encrypt the entire plaintext + block, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("Failed to create cipher: %v", err) + } + + ciphertext := make([]byte, len(plaintext)) + stream := cipher.NewCTR(block, iv) + stream.XORKeyStream(ciphertext, plaintext) + + // Test various offsets (both block-aligned and non-block-aligned) + testOffsets := []int64{0, 1, 5, 10, 15, 16, 17, 21, 32, 33, 47, 48, 100, 500} + + for _, offset := range testOffsets { + t.Run(string(rune('A'+offset)), func(t *testing.T) { + // Calculate adjusted IV and skip + adjustedIV, skip := calculateIVWithOffset(iv, offset) + + // CRITICAL: Start from the block-aligned offset, not the user offset + // CTR mode works on 16-byte blocks, so we need to decrypt from the block start + blockAlignedOffset := offset - int64(skip) + + // Decrypt from the block-aligned offset + decryptBlock, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("Failed to create decrypt cipher: %v", err) + } + + decryptStream := cipher.NewCTR(decryptBlock, adjustedIV) + + // Create a reader for the ciphertext starting at block-aligned offset + ciphertextFromBlockStart := ciphertext[blockAlignedOffset:] + decryptedFromBlockStart := make([]byte, len(ciphertextFromBlockStart)) + decryptStream.XORKeyStream(decryptedFromBlockStart, ciphertextFromBlockStart) + + // CRITICAL: Skip the intra-block bytes to get to the user-requested offset + if skip > 0 { + if skip > len(decryptedFromBlockStart) { + t.Fatalf("Skip %d exceeds decrypted data length %d", skip, len(decryptedFromBlockStart)) + } + decryptedFromBlockStart = decryptedFromBlockStart[skip:] + } + + // Rename for consistency + decryptedFromOffset := decryptedFromBlockStart + + // Verify decrypted data matches original plaintext + expectedPlaintext := plaintext[offset:] + if !bytes.Equal(decryptedFromOffset, expectedPlaintext) { + t.Errorf("Decryption mismatch at offset %d (skip=%d)", offset, skip) + previewLen := 32 + if len(expectedPlaintext) < previewLen { + previewLen = len(expectedPlaintext) + } + t.Errorf(" Expected first 32 bytes: %x", expectedPlaintext[:previewLen]) + previewLen2 := 32 + if len(decryptedFromOffset) < previewLen2 { + previewLen2 = len(decryptedFromOffset) + } + t.Errorf(" Got first 32 bytes: %x", decryptedFromOffset[:previewLen2]) + + // Find first mismatch + for i := 0; i < len(expectedPlaintext) && i < len(decryptedFromOffset); i++ { + if expectedPlaintext[i] != decryptedFromOffset[i] { + t.Errorf(" First mismatch at byte %d: expected %02x, got %02x", i, expectedPlaintext[i], decryptedFromOffset[i]) + break + } + } + } + }) + } +} + +// TestCTRRangeRequestSimulation simulates a real-world S3 range request scenario +func TestCTRRangeRequestSimulation(t *testing.T) { + // Simulate uploading a 5MB object + objectSize := 5 * 1024 * 1024 + plaintext := make([]byte, objectSize) + for i := range plaintext { + plaintext[i] = byte(i % 256) + } + + // Encrypt the object + key := make([]byte, 32) + iv := make([]byte, 16) + rand.Read(key) + rand.Read(iv) + + block, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("Failed to create cipher: %v", err) + } + + ciphertext := make([]byte, len(plaintext)) + stream := cipher.NewCTR(block, iv) + stream.XORKeyStream(ciphertext, plaintext) + + // Simulate various S3 range requests + rangeTests := []struct { + name string + start int64 + end int64 + }{ + {"First byte", 0, 0}, + {"First 100 bytes", 0, 99}, + {"Mid-block range", 5, 100}, // Critical: starts at non-aligned offset + {"Single mid-block byte", 17, 17}, // Critical: single byte at offset 17 + {"Cross-block range", 10, 50}, // Spans multiple blocks + {"Large range", 1000, 10000}, + {"Tail range", int64(objectSize - 1000), int64(objectSize - 1)}, + } + + for _, rt := range rangeTests { + t.Run(rt.name, func(t *testing.T) { + rangeSize := rt.end - rt.start + 1 + + // Calculate adjusted IV and skip for the range start + adjustedIV, skip := calculateIVWithOffset(iv, rt.start) + + // CRITICAL: Start decryption from block-aligned offset + blockAlignedStart := rt.start - int64(skip) + + // Create decryption stream + decryptBlock, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("Failed to create decrypt cipher: %v", err) + } + + decryptStream := cipher.NewCTR(decryptBlock, adjustedIV) + + // Decrypt from block-aligned start through the end of range + ciphertextFromBlock := ciphertext[blockAlignedStart : rt.end+1] + decryptedFromBlock := make([]byte, len(ciphertextFromBlock)) + decryptStream.XORKeyStream(decryptedFromBlock, ciphertextFromBlock) + + // CRITICAL: Skip intra-block bytes to get to user-requested start + if skip > 0 { + decryptedFromBlock = decryptedFromBlock[skip:] + } + + decryptedRange := decryptedFromBlock + + // Verify decrypted range matches original plaintext + expectedPlaintext := plaintext[rt.start : rt.end+1] + if !bytes.Equal(decryptedRange, expectedPlaintext) { + t.Errorf("Range decryption mismatch for %s (offset=%d, size=%d, skip=%d)", + rt.name, rt.start, rangeSize, skip) + previewLen := 64 + if len(expectedPlaintext) < previewLen { + previewLen = len(expectedPlaintext) + } + t.Errorf(" Expected: %x", expectedPlaintext[:previewLen]) + previewLen2 := previewLen + if len(decryptedRange) < previewLen2 { + previewLen2 = len(decryptedRange) + } + t.Errorf(" Got: %x", decryptedRange[:previewLen2]) + } + }) + } +} + +// TestCTRDecryptionWithIOReader tests the integration with io.Reader +func TestCTRDecryptionWithIOReader(t *testing.T) { + plaintext := []byte("Hello, World! This is a test of CTR mode decryption with non-aligned offsets.") + + key := make([]byte, 32) + iv := make([]byte, 16) + rand.Read(key) + rand.Read(iv) + + // Encrypt + block, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("Failed to create cipher: %v", err) + } + + ciphertext := make([]byte, len(plaintext)) + stream := cipher.NewCTR(block, iv) + stream.XORKeyStream(ciphertext, plaintext) + + // Test reading from various offsets using io.Reader + testOffsets := []int64{0, 5, 10, 16, 17, 30} + + for _, offset := range testOffsets { + t.Run(string(rune('A'+offset)), func(t *testing.T) { + // Calculate adjusted IV and skip + adjustedIV, skip := calculateIVWithOffset(iv, offset) + + // CRITICAL: Start reading from block-aligned offset in ciphertext + blockAlignedOffset := offset - int64(skip) + + // Create decrypted reader + decryptBlock, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("Failed to create decrypt cipher: %v", err) + } + + decryptStream := cipher.NewCTR(decryptBlock, adjustedIV) + ciphertextReader := bytes.NewReader(ciphertext[blockAlignedOffset:]) + decryptedReader := &cipher.StreamReader{S: decryptStream, R: ciphertextReader} + + // Skip intra-block bytes to get to user-requested offset + if skip > 0 { + _, err := io.CopyN(io.Discard, decryptedReader, int64(skip)) + if err != nil { + t.Fatalf("Failed to skip %d bytes: %v", skip, err) + } + } + + // Read decrypted data + decryptedData, err := io.ReadAll(decryptedReader) + if err != nil { + t.Fatalf("Failed to read decrypted data: %v", err) + } + + // Verify + expectedPlaintext := plaintext[offset:] + if !bytes.Equal(decryptedData, expectedPlaintext) { + t.Errorf("Decryption mismatch at offset %d (skip=%d)", offset, skip) + t.Errorf(" Expected: %q", expectedPlaintext) + t.Errorf(" Got: %q", decryptedData) + } + }) + } +} + diff --git a/weed/s3api/s3_sse_kms.go b/weed/s3api/s3_sse_kms.go index ee21c129c..fa9451a8f 100644 --- a/weed/s3api/s3_sse_kms.go +++ b/weed/s3api/s3_sse_kms.go @@ -164,7 +164,8 @@ func CreateSSEKMSEncryptedReaderWithBaseIVAndOffset(r io.Reader, keyID string, e defer clearKMSDataKey(dataKeyResult) // Calculate unique IV using base IV and offset to prevent IV reuse in multipart uploads - iv := calculateIVWithOffset(baseIV, offset) + // Skip is not used here because we're encrypting from the start (not reading a range) + iv, _ := calculateIVWithOffset(baseIV, offset) // Create CTR mode cipher stream stream := cipher.NewCTR(dataKeyResult.Block, iv) @@ -420,9 +421,11 @@ func CreateSSEKMSDecryptedReader(r io.Reader, sseKey *SSEKMSKey) (io.Reader, err } // Calculate the correct IV for this chunk's offset within the original part + // Note: The skip bytes must be discarded by the caller before reading from the returned reader var iv []byte if sseKey.ChunkOffset > 0 { - iv = calculateIVWithOffset(sseKey.IV, sseKey.ChunkOffset) + iv, _ = calculateIVWithOffset(sseKey.IV, sseKey.ChunkOffset) + // Skip value is ignored here; caller must handle intra-block byte skipping } else { iv = sseKey.IV } diff --git a/weed/s3api/s3_sse_s3.go b/weed/s3api/s3_sse_s3.go index f02149088..22292bb9b 100644 --- a/weed/s3api/s3_sse_s3.go +++ b/weed/s3api/s3_sse_s3.go @@ -540,7 +540,8 @@ func CreateSSES3EncryptedReaderWithBaseIV(reader io.Reader, key *SSES3Key, baseI // Calculate the proper IV with offset to ensure unique IV per chunk/part // This prevents the severe security vulnerability of IV reuse in CTR mode - iv := calculateIVWithOffset(baseIV, offset) + // Skip is not used here because we're encrypting from the start (not reading a range) + iv, _ := calculateIVWithOffset(baseIV, offset) stream := cipher.NewCTR(block, iv) encryptedReader := &cipher.StreamReader{S: stream, R: reader} diff --git a/weed/s3api/s3_sse_utils.go b/weed/s3api/s3_sse_utils.go index 848bc61ea..c902dc423 100644 --- a/weed/s3api/s3_sse_utils.go +++ b/weed/s3api/s3_sse_utils.go @@ -4,19 +4,22 @@ import "github.com/seaweedfs/seaweedfs/weed/glog" // calculateIVWithOffset calculates a unique IV by combining a base IV with an offset. // This ensures each chunk/part uses a unique IV, preventing CTR mode IV reuse vulnerabilities. +// Returns the adjusted IV and the number of bytes to skip from the decrypted stream. +// The skip is needed because CTR mode operates on 16-byte blocks, but the offset may not be block-aligned. // This function is shared between SSE-KMS and SSE-S3 implementations for consistency. -func calculateIVWithOffset(baseIV []byte, offset int64) []byte { +func calculateIVWithOffset(baseIV []byte, offset int64) ([]byte, int) { if len(baseIV) != 16 { glog.Errorf("Invalid base IV length: expected 16, got %d", len(baseIV)) - return baseIV // Return original IV as fallback + return baseIV, 0 // Return original IV as fallback } // Create a copy of the base IV to avoid modifying the original iv := make([]byte, 16) copy(iv, baseIV) - // Calculate the block offset (AES block size is 16 bytes) + // Calculate the block offset (AES block size is 16 bytes) and intra-block skip blockOffset := offset / 16 + skip := int(offset % 16) originalBlockOffset := blockOffset // Add the block offset to the IV counter (last 8 bytes, big-endian) @@ -36,7 +39,7 @@ func calculateIVWithOffset(baseIV []byte, offset int64) []byte { } // Single consolidated debug log to avoid performance impact in high-throughput scenarios - glog.V(4).Infof("calculateIVWithOffset: baseIV=%x, offset=%d, blockOffset=%d, derivedIV=%x", - baseIV, offset, originalBlockOffset, iv) - return iv + glog.V(4).Infof("calculateIVWithOffset: baseIV=%x, offset=%d, blockOffset=%d, skip=%d, derivedIV=%x", + baseIV, offset, originalBlockOffset, skip, iv) + return iv, skip } diff --git a/weed/s3api/s3api_object_handlers.go b/weed/s3api/s3api_object_handlers.go index 91df78dfc..04e0f35d8 100644 --- a/weed/s3api/s3api_object_handlers.go +++ b/weed/s3api/s3api_object_handlers.go @@ -1297,10 +1297,12 @@ func (s3a *S3ApiServer) decryptSSECChunkView(ctx context.Context, fileChunk *fil // Calculate IV using PartOffset // PartOffset is the position of this chunk within its part's encrypted stream var adjustedIV []byte + var ivSkip int if ssecMetadata.PartOffset > 0 { - adjustedIV = calculateIVWithOffset(chunkIV, ssecMetadata.PartOffset) + adjustedIV, ivSkip = calculateIVWithOffset(chunkIV, ssecMetadata.PartOffset) } else { adjustedIV = chunkIV + ivSkip = 0 } // Decrypt the full chunk @@ -1310,6 +1312,17 @@ func (s3a *S3ApiServer) decryptSSECChunkView(ctx context.Context, fileChunk *fil return nil, fmt.Errorf("failed to create 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 the position we need in the decrypted stream if chunkView.OffsetInChunk > 0 { _, err = io.CopyN(io.Discard, decryptedReader, chunkView.OffsetInChunk) @@ -1351,10 +1364,12 @@ func (s3a *S3ApiServer) decryptSSEKMSChunkView(ctx context.Context, fileChunk *f // Calculate IV using ChunkOffset (same as PartOffset in SSE-C) var adjustedIV []byte + var ivSkip int if sseKMSKey.ChunkOffset > 0 { - adjustedIV = calculateIVWithOffset(sseKMSKey.IV, sseKMSKey.ChunkOffset) + adjustedIV, ivSkip = calculateIVWithOffset(sseKMSKey.IV, sseKMSKey.ChunkOffset) } else { adjustedIV = sseKMSKey.IV + ivSkip = 0 } adjustedKey := &SSEKMSKey{ @@ -1372,6 +1387,17 @@ func (s3a *S3ApiServer) decryptSSEKMSChunkView(ctx context.Context, fileChunk *f 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_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index 53fbf547c..48c3f429f 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/weed/s3api/s3api_object_handlers_copy.go @@ -1422,10 +1422,12 @@ func (s3a *S3ApiServer) copyMultipartSSECChunk(chunk *filer_pb.FileChunk, copySo // Calculate the correct IV for this chunk using within-part offset var chunkIV []byte + var ivSkip int if ssecMetadata.PartOffset > 0 { - chunkIV = calculateIVWithOffset(chunkBaseIV, ssecMetadata.PartOffset) + chunkIV, ivSkip = calculateIVWithOffset(chunkBaseIV, ssecMetadata.PartOffset) } else { chunkIV = chunkBaseIV + ivSkip = 0 } // Decrypt the chunk data @@ -1434,6 +1436,14 @@ func (s3a *S3ApiServer) copyMultipartSSECChunk(chunk *filer_pb.FileChunk, copySo return nil, nil, fmt.Errorf("create decrypted reader: %w", decErr) } + // CRITICAL: Skip intra-block bytes from CTR decryption (non-block-aligned offset handling) + if ivSkip > 0 { + _, skipErr := io.CopyN(io.Discard, decryptedReader, int64(ivSkip)) + if skipErr != nil { + return nil, nil, fmt.Errorf("failed to skip intra-block bytes (%d): %w", ivSkip, skipErr) + } + } + decryptedData, readErr := io.ReadAll(decryptedReader) if readErr != nil { return nil, nil, fmt.Errorf("decrypt chunk data: %w", readErr) @@ -1642,10 +1652,12 @@ func (s3a *S3ApiServer) copyCrossEncryptionChunk(chunk *filer_pb.FileChunk, sour // Calculate the correct IV for this chunk using within-part offset var chunkIV []byte + var ivSkip int if ssecMetadata.PartOffset > 0 { - chunkIV = calculateIVWithOffset(chunkBaseIV, ssecMetadata.PartOffset) + chunkIV, ivSkip = calculateIVWithOffset(chunkBaseIV, ssecMetadata.PartOffset) } else { chunkIV = chunkBaseIV + ivSkip = 0 } decryptedReader, decErr := CreateSSECDecryptedReader(bytes.NewReader(encryptedData), sourceSSECKey, chunkIV) @@ -1653,6 +1665,14 @@ func (s3a *S3ApiServer) copyCrossEncryptionChunk(chunk *filer_pb.FileChunk, sour return nil, fmt.Errorf("create SSE-C decrypted reader: %w", decErr) } + // CRITICAL: Skip intra-block bytes from CTR decryption (non-block-aligned offset handling) + if ivSkip > 0 { + _, skipErr := io.CopyN(io.Discard, decryptedReader, int64(ivSkip)) + if skipErr != nil { + return nil, fmt.Errorf("failed to skip intra-block bytes (%d): %w", ivSkip, skipErr) + } + } + decryptedData, readErr := io.ReadAll(decryptedReader) if readErr != nil { return nil, fmt.Errorf("decrypt SSE-C chunk data: %w", readErr)