Files
seaweedFS/weed/s3api/s3api_key_rotation.go
Chris Lu 0abf70061b S3 API: Fix SSE-S3 decryption on object download (#7366)
* S3 API: Fix SSE-S3 decryption on object download

Fixes #7363

This commit adds missing SSE-S3 decryption support when downloading
objects from SSE-S3 encrypted buckets. Previously, SSE-S3 encrypted
objects were returned in their encrypted form, causing data corruption
and hash mismatches.

Changes:
- Updated detectPrimarySSEType() to detect SSE-S3 encrypted objects
  by examining chunk metadata and distinguishing SSE-S3 from SSE-KMS
- Added SSE-S3 handling in handleSSEResponse() to route to new handler
- Implemented handleSSES3Response() for both single-part and multipart
  SSE-S3 encrypted objects with proper decryption
- Implemented createMultipartSSES3DecryptedReader() for multipart
  objects with per-chunk decryption using stored IVs
- Updated addSSEHeadersToResponse() to include SSE-S3 response headers

The fix follows the existing SSE-C and SSE-KMS patterns, using the
envelope encryption architecture where each object's DEK is encrypted
with the KEK stored in the filer.

* Add comprehensive tests for SSE-S3 decryption

- TestSSES3EncryptionDecryption: basic encryption/decryption
- TestSSES3IsRequestInternal: request detection
- TestSSES3MetadataSerialization: metadata serialization/deserialization
- TestDetectPrimarySSETypeS3: SSE type detection for various scenarios
- TestAddSSES3HeadersToResponse: response header validation
- TestSSES3EncryptionWithBaseIV: multipart encryption with base IV
- TestSSES3WrongKeyDecryption: wrong key error handling
- TestSSES3KeyGeneration: key generation and uniqueness
- TestSSES3VariousSizes: encryption/decryption with various data sizes
- TestSSES3ResponseHeaders: response header correctness
- TestSSES3IsEncryptedInternal: metadata-based encryption detection
- TestSSES3InvalidMetadataDeserialization: error handling for invalid metadata
- TestGetSSES3Headers: header generation
- TestProcessSSES3Request: request processing
- TestGetSSES3KeyFromMetadata: key extraction from metadata
- TestSSES3EnvelopeEncryption: envelope encryption correctness
- TestValidateSSES3Key: key validation

All tests pass successfully, providing comprehensive coverage for the
SSE-S3 decryption fix.

* Address PR review comments

1. Fix resource leak in createMultipartSSES3DecryptedReader:
   - Wrap decrypted reader with closer to properly release resources
   - Ensure underlying chunkReader is closed when done

2. Handle mixed-encryption objects correctly:
   - Check chunk encryption type before attempting decryption
   - Pass through non-SSE-S3 chunks unmodified
   - Log encryption type for debugging

3. Improve SSE type detection logic:
   - Add explicit case for aws:kms algorithm
   - Handle unknown algorithms gracefully
   - Better documentation for tie-breaking precedence

4. Document tie-breaking behavior:
   - Clarify that mixed encryption indicates potential corruption
   - Explicit precedence order: SSE-C > SSE-KMS > SSE-S3

These changes address high-severity resource management issues and
improve robustness when handling edge cases and mixed-encryption
scenarios.

* Fix IV retrieval for small/inline SSE-S3 encrypted files

Critical bug fix: The previous implementation only looked for the IV in
chunk metadata, which would fail for small files stored inline (without
chunks).

Changes:
- Check object-level metadata (sseS3Key.IV) first for inline files
- Fallback to first chunk metadata only if object-level IV not found
- Improved error message to indicate both locations were checked

This ensures small SSE-S3 encrypted files (stored inline in entry.Content)
can be properly decrypted, as their IV is stored in the object-level
SeaweedFSSSES3Key metadata rather than in chunk metadata.

Fixes the high-severity issue identified in PR review.

* Clean up unused SSE metadata helper functions

Remove legacy SSE metadata helper functions that were never fully
implemented or used:

Removed unused functions:
- StoreSSECMetadata() / GetSSECMetadata()
- StoreSSEKMSMetadata() / GetSSEKMSMetadata()
- StoreSSES3Metadata() / GetSSES3Metadata()
- IsSSEEncrypted()
- GetSSEAlgorithm()

Removed unused constants:
- MetaSSEAlgorithm
- MetaSSECKeyMD5
- MetaSSEKMSKeyID
- MetaSSEKMSEncryptedKey
- MetaSSEKMSContext
- MetaSSES3KeyID

These functions were from an earlier design where IV and other metadata
would be stored in common entry.Extended keys. The actual implementations
use type-specific serialization:

- SSE-C: Uses StoreIVInMetadata()/GetIVFromMetadata() directly for IV
- SSE-KMS: Serializes entire SSEKMSKey structure as JSON (includes IV)
- SSE-S3: Serializes entire SSES3Key structure as JSON (includes IV)

This follows Option A: SSE-S3 uses envelope encryption pattern like
SSE-KMS, where IV is stored within the serialized key metadata rather
than in a separate metadata field.

Kept functions still in use:
- StoreIVInMetadata() - Used by SSE-C
- GetIVFromMetadata() - Used by SSE-C and streaming copy
- MetaSSEIV constant - Used by SSE-C

All tests pass after cleanup.

* Rename SSE metadata functions to clarify SSE-C specific usage

Renamed functions and constants to explicitly indicate they are SSE-C
specific, improving code clarity:

Renamed:
- MetaSSEIV → MetaSSECIV
- StoreIVInMetadata() → StoreSSECIVInMetadata()
- GetIVFromMetadata() → GetSSECIVFromMetadata()

Updated all usages across:
- s3api_key_rotation.go
- s3api_streaming_copy.go
- s3api_object_handlers_copy.go
- s3_sse_copy_test.go
- s3_sse_test_utils_test.go

Rationale:
These functions are exclusively used by SSE-C for storing/retrieving
the IV in entry.Extended metadata. SSE-KMS and SSE-S3 use different
approaches (IV stored in serialized key structures), so the generic
names were misleading. The new names make it clear these are part of
the SSE-C implementation.

All tests pass.

* Add integration tests for SSE-S3 end-to-end encryption/decryption

These integration tests cover the complete encrypt->store->decrypt cycle
that was missing from the original test suite. They would have caught
the IV retrieval bug for inline files.

Tests added:
- TestSSES3EndToEndSmallFile: Tests inline files (10, 50, 256 bytes)
  * Specifically tests the critical IV retrieval path for inline files
  * This test explicitly checks the bug we fixed where inline files
    couldn't retrieve their IV from object-level metadata

- TestSSES3EndToEndChunkedFile: Tests multipart encrypted files
  * Verifies per-chunk metadata serialization/deserialization
  * Tests that each chunk can be independently decrypted with its own IV

- TestSSES3EndToEndWithDetectPrimaryType: Tests type detection
  * Verifies inline vs chunked SSE-S3 detection
  * Ensures SSE-S3 is distinguished from SSE-KMS

Note: Full HTTP handler tests (PUT -> GET through actual handlers) would
require a complete mock server with filer connections, which is complex.
These tests focus on the critical decrypt path and data flow.

Why these tests are important:
- Unit tests alone don't catch integration issues
- The IV retrieval bug existed because there was no end-to-end test
- These tests simulate the actual storage/retrieval flow
- They verify the complete encryption architecture works correctly

All tests pass.

* Fix TestValidateSSES3Key expectations to match actual implementation

The ValidateSSES3Key function only validates that the key struct is not
nil, but doesn't validate the Key field contents or size. The test was
expecting validation that doesn't exist.

Updated test cases:
- Nil key struct → should error (correct)
- Valid key → should not error (correct)
- Invalid key size → should not error (validation doesn't check this)
- Nil key bytes → should not error (validation doesn't check this)

Added comments to clarify what the current validation actually checks.
This matches the behavior of ValidateSSEKMSKey and ValidateSSECKey
which also only check for nil struct, not field contents.

All SSE tests now pass.

* Improve ValidateSSES3Key to properly validate key contents

Enhanced the validation function from only checking nil struct to
comprehensive validation of all key fields:

Validations added:
1. Key bytes not nil
2. Key size exactly 32 bytes (SSES3KeySize)
3. Algorithm must be "AES256" (SSES3Algorithm)
4. Key ID must not be empty
5. IV length must be 16 bytes if set (optional - set during encryption)

Test improvements (10 test cases):
- Nil key struct
- Valid key without IV
- Valid key with IV
- Invalid key size (too small)
- Invalid key size (too large)
- Nil key bytes
- Empty key ID
- Invalid algorithm
- Invalid IV length
- Empty IV (allowed - set during encryption)

This matches the robustness of SSE-C and SSE-KMS validation and will
catch configuration errors early rather than failing during
encryption/decryption.

All SSE tests pass.

* Replace custom string helper functions with strings.Contains

Address Gemini Code Assist review feedback:
- Remove custom contains() and findSubstring() helper functions
- Use standard library strings.Contains() instead
- Add strings import

This makes the code more idiomatic and easier to maintain by using
the standard library instead of reimplementing functionality.

Changes:
- Added "strings" to imports
- Replaced contains(err.Error(), tc.errorMsg) with strings.Contains(err.Error(), tc.errorMsg)
- Removed 15 lines of custom helper code

All tests pass.

* filer fix reading and writing SSE-S3 headers

* filter out seaweedfs internal headers

* Update weed/s3api/s3api_object_handlers.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update weed/s3api/s3_validation_utils.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update s3api_streaming_copy.go

* remove fallback

* remove redundant check

* refactor

* remove extra object fetching

* in case object is not found

* Correct Version Entry for SSE Routing

* Proper Error Handling for SSE Entry Fetching

* Eliminated All Redundant Lookups

* Removed brittle “exactly 5 successes/failures” assertions. Added invariant checks

total recorded attempts equals request count,
successes never exceed capacity,
failures cover remaining attempts,
final AvailableSpace matches capacity - successes.

* refactor

* fix test

* Fixed Broken Fallback Logic

* refactor

* Better Error for Encryption Type Mismatch

* refactor

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2025-10-23 20:10:12 -07:00

292 lines
9.7 KiB
Go

package s3api
import (
"bytes"
"crypto/rand"
"fmt"
"io"
"net/http"
"github.com/seaweedfs/seaweedfs/weed/glog"
"github.com/seaweedfs/seaweedfs/weed/pb/filer_pb"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
// rotateSSECKey handles SSE-C key rotation for same-object copies
func (s3a *S3ApiServer) rotateSSECKey(entry *filer_pb.Entry, r *http.Request) ([]*filer_pb.FileChunk, error) {
// Parse source and destination SSE-C keys
sourceKey, err := ParseSSECCopySourceHeaders(r)
if err != nil {
return nil, fmt.Errorf("parse SSE-C copy source headers: %w", err)
}
destKey, err := ParseSSECHeaders(r)
if err != nil {
return nil, fmt.Errorf("parse SSE-C destination headers: %w", err)
}
// Validate that we have both keys
if sourceKey == nil {
return nil, fmt.Errorf("source SSE-C key required for key rotation")
}
if destKey == nil {
return nil, fmt.Errorf("destination SSE-C key required for key rotation")
}
// Check if keys are actually different
if sourceKey.KeyMD5 == destKey.KeyMD5 {
glog.V(2).Infof("SSE-C key rotation: keys are identical, using direct copy")
return entry.GetChunks(), nil
}
glog.V(2).Infof("SSE-C key rotation: rotating from key %s to key %s",
sourceKey.KeyMD5[:8], destKey.KeyMD5[:8])
// For SSE-C key rotation, we need to re-encrypt all chunks
// This cannot be a metadata-only operation because the encryption key changes
return s3a.rotateSSECChunks(entry, sourceKey, destKey)
}
// rotateSSEKMSKey handles SSE-KMS key rotation for same-object copies
func (s3a *S3ApiServer) rotateSSEKMSKey(entry *filer_pb.Entry, r *http.Request) ([]*filer_pb.FileChunk, error) {
// Get source and destination key IDs
srcKeyID, srcEncrypted := GetSourceSSEKMSInfo(entry.Extended)
if !srcEncrypted {
return nil, fmt.Errorf("source object is not SSE-KMS encrypted")
}
dstKeyID := r.Header.Get(s3_constants.AmzServerSideEncryptionAwsKmsKeyId)
if dstKeyID == "" {
// Use default key if not specified
dstKeyID = "default"
}
// Check if keys are actually different
if srcKeyID == dstKeyID {
glog.V(2).Infof("SSE-KMS key rotation: keys are identical, using direct copy")
return entry.GetChunks(), nil
}
glog.V(2).Infof("SSE-KMS key rotation: rotating from key %s to key %s", srcKeyID, dstKeyID)
// For SSE-KMS, we can potentially do metadata-only rotation
// if the KMS service supports key aliasing and the data encryption key can be re-wrapped
if s3a.canDoMetadataOnlyKMSRotation(srcKeyID, dstKeyID) {
return s3a.rotateSSEKMSMetadataOnly(entry, srcKeyID, dstKeyID)
}
// Fallback to full re-encryption
return s3a.rotateSSEKMSChunks(entry, srcKeyID, dstKeyID, r)
}
// canDoMetadataOnlyKMSRotation determines if KMS key rotation can be done metadata-only
func (s3a *S3ApiServer) canDoMetadataOnlyKMSRotation(srcKeyID, dstKeyID string) bool {
// For now, we'll be conservative and always re-encrypt
// In a full implementation, this would check if:
// 1. Both keys are in the same KMS instance
// 2. The KMS supports key re-wrapping
// 3. The user has permissions for both keys
return false
}
// rotateSSEKMSMetadataOnly performs metadata-only SSE-KMS key rotation
func (s3a *S3ApiServer) rotateSSEKMSMetadataOnly(entry *filer_pb.Entry, srcKeyID, dstKeyID string) ([]*filer_pb.FileChunk, error) {
// This would re-wrap the data encryption key with the new KMS key
// For now, return an error since we don't support this yet
return nil, fmt.Errorf("metadata-only KMS key rotation not yet implemented")
}
// rotateSSECChunks re-encrypts all chunks with new SSE-C key
func (s3a *S3ApiServer) rotateSSECChunks(entry *filer_pb.Entry, sourceKey, destKey *SSECustomerKey) ([]*filer_pb.FileChunk, error) {
// Get IV from entry metadata
iv, err := GetSSECIVFromMetadata(entry.Extended)
if err != nil {
return nil, fmt.Errorf("get SSE-C IV from metadata: %w", err)
}
var rotatedChunks []*filer_pb.FileChunk
for _, chunk := range entry.GetChunks() {
rotatedChunk, err := s3a.rotateSSECChunk(chunk, sourceKey, destKey, iv)
if err != nil {
return nil, fmt.Errorf("rotate SSE-C chunk: %w", err)
}
rotatedChunks = append(rotatedChunks, rotatedChunk)
}
// Generate new IV for the destination and store it in entry metadata
newIV := make([]byte, s3_constants.AESBlockSize)
if _, err := io.ReadFull(rand.Reader, newIV); err != nil {
return nil, fmt.Errorf("generate new IV: %w", err)
}
// Update entry metadata with new IV and SSE-C headers
if entry.Extended == nil {
entry.Extended = make(map[string][]byte)
}
StoreSSECIVInMetadata(entry.Extended, newIV)
entry.Extended[s3_constants.AmzServerSideEncryptionCustomerAlgorithm] = []byte("AES256")
entry.Extended[s3_constants.AmzServerSideEncryptionCustomerKeyMD5] = []byte(destKey.KeyMD5)
return rotatedChunks, nil
}
// rotateSSEKMSChunks re-encrypts all chunks with new SSE-KMS key
func (s3a *S3ApiServer) rotateSSEKMSChunks(entry *filer_pb.Entry, srcKeyID, dstKeyID string, r *http.Request) ([]*filer_pb.FileChunk, error) {
var rotatedChunks []*filer_pb.FileChunk
// Parse encryption context and bucket key settings
_, encryptionContext, bucketKeyEnabled, err := ParseSSEKMSCopyHeaders(r)
if err != nil {
return nil, fmt.Errorf("parse SSE-KMS copy headers: %w", err)
}
for _, chunk := range entry.GetChunks() {
rotatedChunk, err := s3a.rotateSSEKMSChunk(chunk, srcKeyID, dstKeyID, encryptionContext, bucketKeyEnabled)
if err != nil {
return nil, fmt.Errorf("rotate SSE-KMS chunk: %w", err)
}
rotatedChunks = append(rotatedChunks, rotatedChunk)
}
return rotatedChunks, nil
}
// rotateSSECChunk rotates a single SSE-C encrypted chunk
func (s3a *S3ApiServer) rotateSSECChunk(chunk *filer_pb.FileChunk, sourceKey, destKey *SSECustomerKey, iv []byte) (*filer_pb.FileChunk, error) {
// Create new chunk with same properties
newChunk := &filer_pb.FileChunk{
Offset: chunk.Offset,
Size: chunk.Size,
ModifiedTsNs: chunk.ModifiedTsNs,
ETag: chunk.ETag,
}
// Assign new volume for the rotated chunk
assignResult, err := s3a.assignNewVolume("")
if err != nil {
return nil, fmt.Errorf("assign new volume: %w", err)
}
// Set file ID on new chunk
if err := s3a.setChunkFileId(newChunk, assignResult); err != nil {
return nil, err
}
// Get source chunk data
srcUrl, err := s3a.lookupVolumeUrl(chunk.GetFileIdString())
if err != nil {
return nil, fmt.Errorf("lookup source volume: %w", err)
}
// Download encrypted data
encryptedData, err := s3a.downloadChunkData(srcUrl, 0, int64(chunk.Size))
if err != nil {
return nil, fmt.Errorf("download chunk data: %w", err)
}
// Decrypt with source key using provided IV
decryptedReader, err := CreateSSECDecryptedReader(bytes.NewReader(encryptedData), sourceKey, iv)
if err != nil {
return nil, fmt.Errorf("create decrypted reader: %w", err)
}
decryptedData, err := io.ReadAll(decryptedReader)
if err != nil {
return nil, fmt.Errorf("decrypt data: %w", err)
}
// Re-encrypt with destination key
encryptedReader, _, err := CreateSSECEncryptedReader(bytes.NewReader(decryptedData), destKey)
if err != nil {
return nil, fmt.Errorf("create encrypted reader: %w", err)
}
// Note: IV will be handled at the entry level by the calling function
reencryptedData, err := io.ReadAll(encryptedReader)
if err != nil {
return nil, fmt.Errorf("re-encrypt data: %w", err)
}
// Update chunk size to include new IV
newChunk.Size = uint64(len(reencryptedData))
// Upload re-encrypted data
if err := s3a.uploadChunkData(reencryptedData, assignResult); err != nil {
return nil, fmt.Errorf("upload re-encrypted data: %w", err)
}
return newChunk, nil
}
// rotateSSEKMSChunk rotates a single SSE-KMS encrypted chunk
func (s3a *S3ApiServer) rotateSSEKMSChunk(chunk *filer_pb.FileChunk, srcKeyID, dstKeyID string, encryptionContext map[string]string, bucketKeyEnabled bool) (*filer_pb.FileChunk, error) {
// Create new chunk with same properties
newChunk := &filer_pb.FileChunk{
Offset: chunk.Offset,
Size: chunk.Size,
ModifiedTsNs: chunk.ModifiedTsNs,
ETag: chunk.ETag,
}
// Assign new volume for the rotated chunk
assignResult, err := s3a.assignNewVolume("")
if err != nil {
return nil, fmt.Errorf("assign new volume: %w", err)
}
// Set file ID on new chunk
if err := s3a.setChunkFileId(newChunk, assignResult); err != nil {
return nil, err
}
// Get source chunk data
srcUrl, err := s3a.lookupVolumeUrl(chunk.GetFileIdString())
if err != nil {
return nil, fmt.Errorf("lookup source volume: %w", err)
}
// Download data (this would be encrypted with the old KMS key)
chunkData, err := s3a.downloadChunkData(srcUrl, 0, int64(chunk.Size))
if err != nil {
return nil, fmt.Errorf("download chunk data: %w", err)
}
// For now, we'll just re-upload the data as-is
// In a full implementation, this would:
// 1. Decrypt with old KMS key
// 2. Re-encrypt with new KMS key
// 3. Update metadata accordingly
// Upload data with new key (placeholder implementation)
if err := s3a.uploadChunkData(chunkData, assignResult); err != nil {
return nil, fmt.Errorf("upload rotated data: %w", err)
}
return newChunk, nil
}
// IsSameObjectCopy determines if this is a same-object copy operation
func IsSameObjectCopy(r *http.Request, srcBucket, srcObject, dstBucket, dstObject string) bool {
return srcBucket == dstBucket && srcObject == dstObject
}
// NeedsKeyRotation determines if the copy operation requires key rotation
func NeedsKeyRotation(entry *filer_pb.Entry, r *http.Request) bool {
// Check for SSE-C key rotation
if IsSSECEncrypted(entry.Extended) && IsSSECRequest(r) {
return true // Assume different keys for safety
}
// Check for SSE-KMS key rotation
if IsSSEKMSEncrypted(entry.Extended) && IsSSEKMSRequest(r) {
srcKeyID, _ := GetSourceSSEKMSInfo(entry.Extended)
dstKeyID := r.Header.Get(s3_constants.AmzServerSideEncryptionAwsKmsKeyId)
return srcKeyID != dstKeyID
}
return false
}