mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-05-21 17:21:34 +00:00
* 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.
211 lines
7.4 KiB
Go
211 lines
7.4 KiB
Go
package s3api
|
|
|
|
import (
|
|
"bytes"
|
|
"crypto/aes"
|
|
"crypto/cipher"
|
|
"crypto/rand"
|
|
"io"
|
|
"testing"
|
|
)
|
|
|
|
// TestSSECDecryptChunkView_NoOffsetAdjustment verifies that SSE-C decryption
|
|
// does NOT apply calculateIVWithOffset, preventing the critical bug where
|
|
// offset adjustment would cause CTR stream misalignment and data corruption.
|
|
func TestSSECDecryptChunkView_NoOffsetAdjustment(t *testing.T) {
|
|
// Setup: Create test data
|
|
plaintext := []byte("This is a test message for SSE-C decryption without offset adjustment")
|
|
customerKey := &SSECustomerKey{
|
|
Key: make([]byte, 32), // 256-bit key
|
|
KeyMD5: "test-key-md5",
|
|
}
|
|
// Generate random AES key
|
|
if _, err := rand.Read(customerKey.Key); err != nil {
|
|
t.Fatalf("Failed to generate random key: %v", err)
|
|
}
|
|
|
|
// Generate random IV for this "part"
|
|
randomIV := make([]byte, aes.BlockSize)
|
|
if _, err := rand.Read(randomIV); err != nil {
|
|
t.Fatalf("Failed to generate random IV: %v", err)
|
|
}
|
|
|
|
// Encrypt the plaintext using the random IV (simulating SSE-C multipart upload)
|
|
// This is what CreateSSECEncryptedReader does - uses the IV directly without offset
|
|
block, err := aes.NewCipher(customerKey.Key)
|
|
if err != nil {
|
|
t.Fatalf("Failed to create cipher: %v", err)
|
|
}
|
|
ciphertext := make([]byte, len(plaintext))
|
|
stream := cipher.NewCTR(block, randomIV)
|
|
stream.XORKeyStream(ciphertext, plaintext)
|
|
|
|
partOffset := int64(1024) // Non-zero offset that should NOT be applied during SSE-C decryption
|
|
|
|
// TEST: Decrypt using stored IV directly (correct behavior)
|
|
decryptedReaderCorrect, err := CreateSSECDecryptedReader(
|
|
io.NopCloser(bytes.NewReader(ciphertext)),
|
|
customerKey,
|
|
randomIV, // Use stored IV directly - CORRECT
|
|
)
|
|
if err != nil {
|
|
t.Fatalf("Failed to create decrypted reader (correct): %v", err)
|
|
}
|
|
decryptedCorrect, err := io.ReadAll(decryptedReaderCorrect)
|
|
if err != nil {
|
|
t.Fatalf("Failed to read decrypted data (correct): %v", err)
|
|
}
|
|
|
|
// Verify correct decryption
|
|
if !bytes.Equal(decryptedCorrect, plaintext) {
|
|
t.Errorf("Correct decryption failed:\nExpected: %s\nGot: %s", plaintext, decryptedCorrect)
|
|
} else {
|
|
t.Logf("✓ Correct decryption (using stored IV directly) successful")
|
|
}
|
|
|
|
// ANTI-TEST: Decrypt using offset-adjusted IV (incorrect behavior - the bug)
|
|
adjustedIV, ivSkip := calculateIVWithOffset(randomIV, partOffset)
|
|
decryptedReaderWrong, err := CreateSSECDecryptedReader(
|
|
io.NopCloser(bytes.NewReader(ciphertext)),
|
|
customerKey,
|
|
adjustedIV, // Use adjusted IV - WRONG
|
|
)
|
|
if err != nil {
|
|
t.Fatalf("Failed to create decrypted reader (wrong): %v", err)
|
|
}
|
|
|
|
// Skip ivSkip bytes (as the buggy code would do)
|
|
if ivSkip > 0 {
|
|
io.CopyN(io.Discard, decryptedReaderWrong, int64(ivSkip))
|
|
}
|
|
|
|
decryptedWrong, err := io.ReadAll(decryptedReaderWrong)
|
|
if err != nil {
|
|
t.Fatalf("Failed to read decrypted data (wrong): %v", err)
|
|
}
|
|
|
|
// Verify that offset adjustment produces DIFFERENT (corrupted) output
|
|
if bytes.Equal(decryptedWrong, plaintext) {
|
|
t.Errorf("CRITICAL: Offset-adjusted IV produced correct plaintext! This shouldn't happen for SSE-C.")
|
|
} else {
|
|
t.Logf("✓ Verified: Offset-adjusted IV produces corrupted data (as expected for SSE-C)")
|
|
maxLen := 20
|
|
if len(plaintext) < maxLen {
|
|
maxLen = len(plaintext)
|
|
}
|
|
t.Logf(" Plaintext: %q", plaintext[:maxLen])
|
|
maxLen2 := 20
|
|
if len(decryptedWrong) < maxLen2 {
|
|
maxLen2 = len(decryptedWrong)
|
|
}
|
|
t.Logf(" Corrupted: %q", decryptedWrong[:maxLen2])
|
|
}
|
|
}
|
|
|
|
// TestSSEKMSDecryptChunkView_RequiresOffsetAdjustment verifies that SSE-KMS
|
|
// decryption DOES require calculateIVWithOffset, unlike SSE-C.
|
|
func TestSSEKMSDecryptChunkView_RequiresOffsetAdjustment(t *testing.T) {
|
|
// Setup: Create test data
|
|
plaintext := []byte("This is a test message for SSE-KMS decryption with offset adjustment")
|
|
|
|
// Generate base IV and key
|
|
baseIV := make([]byte, aes.BlockSize)
|
|
key := make([]byte, 32)
|
|
if _, err := rand.Read(baseIV); err != nil {
|
|
t.Fatalf("Failed to generate base IV: %v", err)
|
|
}
|
|
if _, err := rand.Read(key); err != nil {
|
|
t.Fatalf("Failed to generate key: %v", err)
|
|
}
|
|
|
|
chunkOffset := int64(2048) // Simulate chunk at offset 2048
|
|
|
|
// Encrypt using base IV + offset (simulating SSE-KMS multipart upload)
|
|
adjustedIV, ivSkip := calculateIVWithOffset(baseIV, chunkOffset)
|
|
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, adjustedIV)
|
|
|
|
// Skip ivSkip bytes in the encryption stream if needed
|
|
if ivSkip > 0 {
|
|
dummy := make([]byte, ivSkip)
|
|
stream.XORKeyStream(dummy, dummy)
|
|
}
|
|
stream.XORKeyStream(ciphertext, plaintext)
|
|
|
|
// TEST: Decrypt using base IV + offset adjustment (correct for SSE-KMS)
|
|
adjustedIVDecrypt, ivSkipDecrypt := calculateIVWithOffset(baseIV, chunkOffset)
|
|
blockDecrypt, err := aes.NewCipher(key)
|
|
if err != nil {
|
|
t.Fatalf("Failed to create cipher for decryption: %v", err)
|
|
}
|
|
|
|
decrypted := make([]byte, len(ciphertext))
|
|
streamDecrypt := cipher.NewCTR(blockDecrypt, adjustedIVDecrypt)
|
|
|
|
// Skip ivSkip bytes in the decryption stream
|
|
if ivSkipDecrypt > 0 {
|
|
dummy := make([]byte, ivSkipDecrypt)
|
|
streamDecrypt.XORKeyStream(dummy, dummy)
|
|
}
|
|
streamDecrypt.XORKeyStream(decrypted, ciphertext)
|
|
|
|
// Verify correct decryption with offset adjustment
|
|
if !bytes.Equal(decrypted, plaintext) {
|
|
t.Errorf("SSE-KMS decryption with offset adjustment failed:\nExpected: %s\nGot: %s", plaintext, decrypted)
|
|
} else {
|
|
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 {
|
|
t.Fatalf("Failed to create cipher for wrong decryption: %v", err)
|
|
}
|
|
|
|
decryptedWrong := make([]byte, len(ciphertext))
|
|
streamWrong := cipher.NewCTR(blockWrong, baseIV) // Use base IV directly - WRONG for SSE-KMS
|
|
streamWrong.XORKeyStream(decryptedWrong, ciphertext)
|
|
|
|
// Verify that NOT using offset adjustment produces corrupted output
|
|
if bytes.Equal(decryptedWrong, plaintext) {
|
|
t.Errorf("CRITICAL: Base IV without offset produced correct plaintext! SSE-KMS requires offset adjustment.")
|
|
} else {
|
|
t.Logf("✓ Verified: Base IV without offset produces corrupted data (as expected for SSE-KMS)")
|
|
}
|
|
}
|
|
|
|
// TestSSEDecryptionDifferences documents the key differences between SSE types
|
|
func TestSSEDecryptionDifferences(t *testing.T) {
|
|
t.Log("SSE-C: Random IV per part → Use stored IV DIRECTLY (no offset)")
|
|
t.Log("SSE-KMS: Base IV + offset → MUST call calculateIVWithOffset(baseIV, offset)")
|
|
t.Log("SSE-S3: Base IV + offset → Stores ADJUSTED IV, use directly")
|
|
|
|
// This test documents the critical differences and serves as executable documentation
|
|
}
|