aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChris Lu <chrislusf@users.noreply.github.com>2025-11-28 13:28:17 -0800
committerGitHub <noreply@github.com>2025-11-28 13:28:17 -0800
commitbd419fda5119ae7f7225cfca0fcb98bce04f4980 (patch)
tree16e1f6d020daf38663c44c423f935d3a0546f7f8
parent7e4bab80326866b1ade17bf3ebddb96999507a25 (diff)
downloadseaweedfs-bd419fda5119ae7f7225cfca0fcb98bce04f4980.tar.xz
seaweedfs-bd419fda5119ae7f7225cfca0fcb98bce04f4980.zip
fix: copy to bucket with default SSE-S3 encryption fails (#7562) (#7568)
* filer use context without cancellation * pass along context * 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 * fmt * refactor: simplify isOrphanedSSES3Header function logic Remove redundant existence check since the caller iterates through metadata map, making the check unnecessary. Improves readability while maintaining the same functionality.
-rw-r--r--test/s3/sse/s3_sse_integration_test.go228
-rw-r--r--weed/s3api/s3_sse_s3.go16
-rw-r--r--weed/s3api/s3_sse_s3_test.go17
-rw-r--r--weed/s3api/s3api_object_handlers_copy.go31
-rw-r--r--weed/s3api/s3api_object_handlers_copy_test.go88
5 files changed, 363 insertions, 17 deletions
diff --git a/test/s3/sse/s3_sse_integration_test.go b/test/s3/sse/s3_sse_integration_test.go
index 0b3ff8f04..7b939ea76 100644
--- a/test/s3/sse/s3_sse_integration_test.go
+++ b/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
diff --git a/weed/s3api/s3_sse_s3.go b/weed/s3api/s3_sse_s3.go
index 22292bb9b..f7cbfa991 100644
--- a/weed/s3api/s3_sse_s3.go
+++ b/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
diff --git a/weed/s3api/s3_sse_s3_test.go b/weed/s3api/s3_sse_s3_test.go
index 391692921..dc3378ae3 100644
--- a/weed/s3api/s3_sse_s3_test.go
+++ b/weed/s3api/s3_sse_s3_test.go
@@ -630,13 +630,21 @@ 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{
s3_constants.AmzServerSideEncryption: []byte("aws:kms"),
@@ -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 {
diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go
index 4dd31c8ce..0c465d3db 100644
--- a/weed/s3api/s3api_object_handlers_copy.go
+++ b/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)
@@ -297,7 +303,7 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request
// For non-versioned destination, use regular copy
// Remove any versioning-related metadata from source that shouldn't carry over
cleanupVersioningMetadata(dstEntry.Extended)
-
+
dstPath := util.FullPath(fmt.Sprintf("%s/%s%s", s3a.option.BucketsPath, dstBucket, dstObject))
dstDir, dstName := dstPath.DirAndName()
@@ -2350,6 +2356,27 @@ 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
+ }
+
+ // The header is AmzServerSideEncryption. Check if its value indicates SSE-S3.
+ if string(metadata[headerKey]) == "AES256" {
+ // It's an SSE-S3 header. It's orphaned if the actual encryption 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.
diff --git a/weed/s3api/s3api_object_handlers_copy_test.go b/weed/s3api/s3api_object_handlers_copy_test.go
index 018c9f270..369d2aac9 100644
--- a/weed/s3api/s3api_object_handlers_copy_test.go
+++ b/weed/s3api/s3api_object_handlers_copy_test.go
@@ -2,12 +2,13 @@ package s3api
import (
"fmt"
- "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
"net/http"
"reflect"
"sort"
"strings"
"testing"
+
+ "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
type H map[string]string
@@ -442,10 +443,10 @@ func transferHeaderToH(data map[string][]string) H {
// This addresses issue #7505 where copies were incorrectly creating versions for non-versioned buckets.
func TestShouldCreateVersionForCopy(t *testing.T) {
testCases := []struct {
- name string
- versioningState string
- expectedResult bool
- description string
+ name string
+ versioningState string
+ expectedResult bool
+ description string
}{
{
name: "VersioningEnabled",
@@ -503,7 +504,7 @@ func TestCleanupVersioningMetadata(t *testing.T) {
removedKeys: []string{s3_constants.ExtVersionIdKey, s3_constants.ExtDeleteMarkerKey, s3_constants.ExtIsLatestKey, s3_constants.ExtETagKey},
},
{
- name: "HandlesEmptyMetadata",
+ name: "HandlesEmptyMetadata",
sourceMetadata: map[string][]byte{},
expectedKeys: []string{},
removedKeys: []string{s3_constants.ExtVersionIdKey, s3_constants.ExtDeleteMarkerKey, s3_constants.ExtIsLatestKey, s3_constants.ExtETagKey},
@@ -511,11 +512,11 @@ func TestCleanupVersioningMetadata(t *testing.T) {
{
name: "PreservesNonVersioningMetadata",
sourceMetadata: map[string][]byte{
- s3_constants.ExtVersionIdKey: []byte("version-456"),
- s3_constants.ExtETagKey: []byte("\"def456\""),
- "X-Amz-Meta-Custom": []byte("value1"),
- "X-Amz-Meta-Another": []byte("value2"),
- s3_constants.ExtIsLatestKey: []byte("true"),
+ s3_constants.ExtVersionIdKey: []byte("version-456"),
+ s3_constants.ExtETagKey: []byte("\"def456\""),
+ "X-Amz-Meta-Custom": []byte("value1"),
+ "X-Amz-Meta-Another": []byte("value2"),
+ s3_constants.ExtIsLatestKey: []byte("true"),
},
expectedKeys: []string{"X-Amz-Meta-Custom", "X-Amz-Meta-Another"},
removedKeys: []string{s3_constants.ExtVersionIdKey, s3_constants.ExtETagKey, s3_constants.ExtIsLatestKey},
@@ -639,3 +640,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)
+ }
+ })
+ }
+}