Browse Source

fix: copy to bucket with default SSE-S3 encryption fails (#7562)

When copying an object from an encrypted bucket to a temporary unencrypted
bucket, then to another bucket with default SSE-S3 encryption, the operation
fails with 'invalid SSE-S3 source key type' error.

Root cause:
When objects are copied from an SSE-S3 encrypted bucket to an unencrypted
bucket, the 'X-Amz-Server-Side-Encryption: AES256' header is preserved but
the actual encryption key (SeaweedFSSSES3Key) is stripped. This creates an
'orphaned' SSE-S3 header that causes IsSSES3EncryptedInternal() to return
true, triggering decryption logic with a nil key.

Fix:
1. Modified IsSSES3EncryptedInternal() to require BOTH the AES256 header
   AND the SeaweedFSSSES3Key to be present before returning true
2. Added isOrphanedSSES3Header() to detect orphaned SSE-S3 headers
3. Updated copy handler to strip orphaned headers during copy operations

Fixes #7562
pull/7568/head
Chris Lu 5 days ago
parent
commit
16b822bf08
  1. 228
      test/s3/sse/s3_sse_integration_test.go
  2. 16
      weed/s3api/s3_sse_s3.go
  3. 17
      weed/s3api/s3_sse_s3_test.go
  4. 30
      weed/s3api/s3api_object_handlers_copy.go
  5. 65
      weed/s3api/s3api_object_handlers_copy_test.go

228
test/s3/sse/s3_sse_integration_test.go

@ -1856,6 +1856,234 @@ func TestCrossSSECopy(t *testing.T) {
})
}
// TestCopyToBucketDefaultEncryptedRegression tests copying objects to buckets with default
// encryption enabled. This is a regression test for GitHub issue #7562 where copying from
// an unencrypted bucket to a bucket with SSE-S3 default encryption fails with error
// "invalid SSE-S3 source key type".
//
// The scenario is:
// 1. Create source bucket with SSE-S3 encryption
// 2. Upload encrypted object
// 3. Copy to temp bucket (unencrypted) - data is decrypted
// 4. Copy from temp bucket to dest bucket with SSE-S3 default encryption - this should re-encrypt
//
// The bug occurs because the source detection incorrectly identifies the temp object as
// SSE-S3 encrypted (based on leftover metadata) when it's actually unencrypted.
func TestCopyToBucketDefaultEncryptedRegression(t *testing.T) {
ctx := context.Background()
client, err := createS3Client(ctx, defaultConfig)
require.NoError(t, err, "Failed to create S3 client")
// Create three buckets for the test scenario
srcBucket, err := createTestBucket(ctx, client, defaultConfig.BucketPrefix+"copy-src-")
require.NoError(t, err, "Failed to create source bucket")
defer cleanupTestBucket(ctx, client, srcBucket)
tempBucket, err := createTestBucket(ctx, client, defaultConfig.BucketPrefix+"copy-temp-")
require.NoError(t, err, "Failed to create temp bucket")
defer cleanupTestBucket(ctx, client, tempBucket)
dstBucket, err := createTestBucket(ctx, client, defaultConfig.BucketPrefix+"copy-dst-")
require.NoError(t, err, "Failed to create destination bucket")
defer cleanupTestBucket(ctx, client, dstBucket)
// Enable SSE-S3 default encryption on source bucket
_, err = client.PutBucketEncryption(ctx, &s3.PutBucketEncryptionInput{
Bucket: aws.String(srcBucket),
ServerSideEncryptionConfiguration: &types.ServerSideEncryptionConfiguration{
Rules: []types.ServerSideEncryptionRule{
{
ApplyServerSideEncryptionByDefault: &types.ServerSideEncryptionByDefault{
SSEAlgorithm: types.ServerSideEncryptionAes256,
},
},
},
},
})
require.NoError(t, err, "Failed to set source bucket encryption")
// Enable SSE-S3 default encryption on destination bucket
_, err = client.PutBucketEncryption(ctx, &s3.PutBucketEncryptionInput{
Bucket: aws.String(dstBucket),
ServerSideEncryptionConfiguration: &types.ServerSideEncryptionConfiguration{
Rules: []types.ServerSideEncryptionRule{
{
ApplyServerSideEncryptionByDefault: &types.ServerSideEncryptionByDefault{
SSEAlgorithm: types.ServerSideEncryptionAes256,
},
},
},
},
})
require.NoError(t, err, "Failed to set destination bucket encryption")
// Test data
testData := []byte("Test data for copy-to-default-encrypted bucket regression test - GitHub issue #7562")
objectKey := "test-object.txt"
t.Run("CopyEncrypted_ToTemp_ToEncrypted", func(t *testing.T) {
// Step 1: Upload object to source bucket (will be automatically encrypted)
_, err = client.PutObject(ctx, &s3.PutObjectInput{
Bucket: aws.String(srcBucket),
Key: aws.String(objectKey),
Body: bytes.NewReader(testData),
// No encryption header - bucket default applies
})
require.NoError(t, err, "Failed to upload to source bucket")
// Verify source object is encrypted
srcHead, err := client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: aws.String(srcBucket),
Key: aws.String(objectKey),
})
require.NoError(t, err, "Failed to HEAD source object")
assert.Equal(t, types.ServerSideEncryptionAes256, srcHead.ServerSideEncryption,
"Source object should be SSE-S3 encrypted")
// Step 2: Copy to temp bucket (unencrypted) - this should decrypt
_, err = client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: aws.String(tempBucket),
Key: aws.String(objectKey),
CopySource: aws.String(fmt.Sprintf("%s/%s", srcBucket, objectKey)),
// No encryption - data should be stored unencrypted
})
require.NoError(t, err, "Failed to copy to temp bucket")
// Verify temp object is unencrypted
tempHead, err := client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: aws.String(tempBucket),
Key: aws.String(objectKey),
})
require.NoError(t, err, "Failed to HEAD temp object")
assert.Empty(t, tempHead.ServerSideEncryption,
"Temp object should be unencrypted")
// Verify temp object content is correct
tempGet, err := client.GetObject(ctx, &s3.GetObjectInput{
Bucket: aws.String(tempBucket),
Key: aws.String(objectKey),
})
require.NoError(t, err, "Failed to GET temp object")
tempData, err := io.ReadAll(tempGet.Body)
tempGet.Body.Close()
require.NoError(t, err, "Failed to read temp object")
assertDataEqual(t, testData, tempData, "Temp object data mismatch")
// Step 3: Copy from temp bucket to dest bucket (with default encryption)
// THIS IS THE BUG: This copy fails with "invalid SSE-S3 source key type"
_, err = client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: aws.String(dstBucket),
Key: aws.String(objectKey),
CopySource: aws.String(fmt.Sprintf("%s/%s", tempBucket, objectKey)),
// No encryption header - bucket default should apply
})
require.NoError(t, err, "Failed to copy to destination bucket - GitHub issue #7562")
// Verify destination object is encrypted
dstHead, err := client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: aws.String(dstBucket),
Key: aws.String(objectKey),
})
require.NoError(t, err, "Failed to HEAD destination object")
assert.Equal(t, types.ServerSideEncryptionAes256, dstHead.ServerSideEncryption,
"Destination object should be SSE-S3 encrypted via bucket default")
// Verify destination object content is correct
dstGet, err := client.GetObject(ctx, &s3.GetObjectInput{
Bucket: aws.String(dstBucket),
Key: aws.String(objectKey),
})
require.NoError(t, err, "Failed to GET destination object")
dstData, err := io.ReadAll(dstGet.Body)
dstGet.Body.Close()
require.NoError(t, err, "Failed to read destination object")
assertDataEqual(t, testData, dstData, "Destination object data mismatch after re-encryption")
})
t.Run("DirectCopyUnencrypted_ToEncrypted", func(t *testing.T) {
// Simpler test case: copy from unencrypted bucket directly to encrypted bucket
objectKey := "direct-copy-test.txt"
// Upload to temp bucket (no default encryption)
_, err = client.PutObject(ctx, &s3.PutObjectInput{
Bucket: aws.String(tempBucket),
Key: aws.String(objectKey),
Body: bytes.NewReader(testData),
})
require.NoError(t, err, "Failed to upload to temp bucket")
// Copy to destination bucket with default encryption
_, err = client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: aws.String(dstBucket),
Key: aws.String(objectKey),
CopySource: aws.String(fmt.Sprintf("%s/%s", tempBucket, objectKey)),
})
require.NoError(t, err, "Failed direct copy unencrypted to default-encrypted bucket")
// Verify destination is encrypted
dstHead, err := client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: aws.String(dstBucket),
Key: aws.String(objectKey),
})
require.NoError(t, err, "Failed to HEAD destination object")
assert.Equal(t, types.ServerSideEncryptionAes256, dstHead.ServerSideEncryption,
"Object should be encrypted via bucket default")
// Verify content
dstGet, err := client.GetObject(ctx, &s3.GetObjectInput{
Bucket: aws.String(dstBucket),
Key: aws.String(objectKey),
})
require.NoError(t, err, "Failed to GET destination object")
dstData, err := io.ReadAll(dstGet.Body)
dstGet.Body.Close()
require.NoError(t, err, "Failed to read destination object")
assertDataEqual(t, testData, dstData, "Data mismatch after encryption")
})
t.Run("CopyWithExplicitSSES3Header", func(t *testing.T) {
// Test explicit SSE-S3 header during copy (should work even without bucket default)
objectKey := "explicit-sse-copy-test.txt"
// Upload to temp bucket (unencrypted)
_, err = client.PutObject(ctx, &s3.PutObjectInput{
Bucket: aws.String(tempBucket),
Key: aws.String(objectKey),
Body: bytes.NewReader(testData),
})
require.NoError(t, err, "Failed to upload to temp bucket")
// Copy with explicit SSE-S3 header
_, err = client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: aws.String(tempBucket), // Same bucket, but with encryption
Key: aws.String(objectKey + "-encrypted"),
CopySource: aws.String(fmt.Sprintf("%s/%s", tempBucket, objectKey)),
ServerSideEncryption: types.ServerSideEncryptionAes256,
})
require.NoError(t, err, "Failed copy with explicit SSE-S3 header")
// Verify encrypted
head, err := client.HeadObject(ctx, &s3.HeadObjectInput{
Bucket: aws.String(tempBucket),
Key: aws.String(objectKey + "-encrypted"),
})
require.NoError(t, err, "Failed to HEAD object")
assert.Equal(t, types.ServerSideEncryptionAes256, head.ServerSideEncryption,
"Object should be SSE-S3 encrypted")
// Verify content
getResp, err := client.GetObject(ctx, &s3.GetObjectInput{
Bucket: aws.String(tempBucket),
Key: aws.String(objectKey + "-encrypted"),
})
require.NoError(t, err, "Failed to GET object")
data, err := io.ReadAll(getResp.Body)
getResp.Body.Close()
require.NoError(t, err, "Failed to read object")
assertDataEqual(t, testData, data, "Data mismatch")
})
}
// REGRESSION TESTS FOR CRITICAL BUGS FIXED
// These tests specifically target the IV storage bugs that were fixed

16
weed/s3api/s3_sse_s3.go

@ -51,11 +51,21 @@ func IsSSES3RequestInternal(r *http.Request) bool {
}
// IsSSES3EncryptedInternal checks if the object metadata indicates SSE-S3 encryption
// An object is considered SSE-S3 encrypted only if it has BOTH the encryption header
// AND the actual encryption key metadata. This prevents false positives when an object
// has leftover headers from a previous encryption state (e.g., after being decrypted
// during a copy operation). Fixes GitHub issue #7562.
func IsSSES3EncryptedInternal(metadata map[string][]byte) bool {
if sseAlgorithm, exists := metadata[s3_constants.AmzServerSideEncryption]; exists {
return string(sseAlgorithm) == SSES3Algorithm
// Check for SSE-S3 algorithm header
sseAlgorithm, hasHeader := metadata[s3_constants.AmzServerSideEncryption]
if !hasHeader || string(sseAlgorithm) != SSES3Algorithm {
return false
}
return false
// Must also have the actual encryption key to be considered encrypted
// Without the key, the object cannot be decrypted and should be treated as unencrypted
_, hasKey := metadata[s3_constants.SeaweedFSSSES3Key]
return hasKey
}
// GenerateSSES3Key generates a new SSE-S3 encryption key

17
weed/s3api/s3_sse_s3_test.go

@ -630,12 +630,20 @@ func TestSSES3IsEncryptedInternal(t *testing.T) {
expected: false,
},
{
name: "Valid SSE-S3 metadata",
name: "Valid SSE-S3 metadata with key",
metadata: map[string][]byte{
s3_constants.AmzServerSideEncryption: []byte("AES256"),
s3_constants.SeaweedFSSSES3Key: []byte("test-key-data"),
},
expected: true,
},
{
name: "SSE-S3 header without key (orphaned header - GitHub #7562)",
metadata: map[string][]byte{
s3_constants.AmzServerSideEncryption: []byte("AES256"),
},
expected: false, // Should not be considered encrypted without the key
},
{
name: "SSE-KMS metadata",
metadata: map[string][]byte{
@ -650,6 +658,13 @@ func TestSSES3IsEncryptedInternal(t *testing.T) {
},
expected: false,
},
{
name: "Key without header",
metadata: map[string][]byte{
s3_constants.SeaweedFSSSES3Key: []byte("test-key-data"),
},
expected: false, // Need both header and key
},
}
for _, tc := range testCases {

30
weed/s3api/s3api_object_handlers_copy.go

@ -171,8 +171,14 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request
// Skip encryption-specific headers that might conflict with destination encryption type
skipHeader := false
// Skip orphaned SSE-S3 headers (header exists but key is missing)
// This prevents confusion about the object's actual encryption state
if isOrphanedSSES3Header(k, entry.Extended) {
skipHeader = true
}
// If we're doing cross-encryption, skip conflicting headers
if len(entry.GetChunks()) > 0 {
if !skipHeader && len(entry.GetChunks()) > 0 {
// Detect source and destination encryption types
srcHasSSEC := IsSSECEncrypted(entry.Extended)
srcHasSSEKMS := IsSSEKMSEncrypted(entry.Extended)
@ -2350,6 +2356,28 @@ func shouldCreateVersionForCopy(versioningState string) bool {
return versioningState == s3_constants.VersioningEnabled
}
// isOrphanedSSES3Header checks if a header is an orphaned SSE-S3 encryption header.
// An orphaned header is one where the encryption indicator exists but the actual key is missing.
// This can happen when an object was previously encrypted but then copied without encryption,
// leaving behind the header but removing the key. These orphaned headers should be stripped
// during copy operations to prevent confusion about the object's actual encryption state.
// Fixes GitHub issue #7562.
func isOrphanedSSES3Header(headerKey string, metadata map[string][]byte) bool {
if headerKey != s3_constants.AmzServerSideEncryption {
return false
}
// Check if this is an AES256 (SSE-S3) header
if val, exists := metadata[s3_constants.AmzServerSideEncryption]; exists {
if string(val) == "AES256" {
// It's orphaned if the actual key is missing
_, hasKey := metadata[s3_constants.SeaweedFSSSES3Key]
return !hasKey
}
}
return false
}
// shouldSkipEncryptionHeader determines if a header should be skipped when copying extended attributes
// based on the source and destination encryption types. This consolidates the repetitive logic for
// filtering encryption-related headers during copy operations.

65
weed/s3api/s3api_object_handlers_copy_test.go

@ -639,3 +639,68 @@ func TestCopyVersioningIntegration(t *testing.T) {
})
}
}
// TestIsOrphanedSSES3Header tests detection of orphaned SSE-S3 headers.
// This is a regression test for GitHub issue #7562 where copying from an
// encrypted bucket to an unencrypted bucket left behind the encryption header
// without the actual key, causing subsequent copy operations to fail.
func TestIsOrphanedSSES3Header(t *testing.T) {
testCases := []struct {
name string
headerKey string
metadata map[string][]byte
expected bool
}{
{
name: "Not an encryption header",
headerKey: "X-Amz-Meta-Custom",
metadata: map[string][]byte{
"X-Amz-Meta-Custom": []byte("value"),
},
expected: false,
},
{
name: "SSE-S3 header with key present (valid)",
headerKey: s3_constants.AmzServerSideEncryption,
metadata: map[string][]byte{
s3_constants.AmzServerSideEncryption: []byte("AES256"),
s3_constants.SeaweedFSSSES3Key: []byte("key-data"),
},
expected: false,
},
{
name: "SSE-S3 header without key (orphaned - GitHub #7562)",
headerKey: s3_constants.AmzServerSideEncryption,
metadata: map[string][]byte{
s3_constants.AmzServerSideEncryption: []byte("AES256"),
},
expected: true,
},
{
name: "SSE-KMS header (not SSE-S3)",
headerKey: s3_constants.AmzServerSideEncryption,
metadata: map[string][]byte{
s3_constants.AmzServerSideEncryption: []byte("aws:kms"),
},
expected: false,
},
{
name: "Different header key entirely",
headerKey: s3_constants.SeaweedFSSSES3Key,
metadata: map[string][]byte{
s3_constants.AmzServerSideEncryption: []byte("AES256"),
},
expected: false,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
result := isOrphanedSSES3Header(tc.headerKey, tc.metadata)
if result != tc.expected {
t.Errorf("isOrphanedSSES3Header(%q, metadata) = %v, expected %v",
tc.headerKey, result, tc.expected)
}
})
}
}
Loading…
Cancel
Save