mirrors/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-08-19 13:46:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(s3api): preserve requested AES256 copy encryption (#10049)

Browse Source 
* fix(s3api): preserve requested AES256 copy encryption

Problem
CopyObject metadata processing ignored an explicit x-amz-server-side-encryption: AES256 request header. A destination copy could lose the requested SSE-S3 metadata even though KMS requests were handled.

Root cause
processMetadataBytes only wrote the destination SSE header when the requested algorithm was aws:kms. Any other explicit SSE algorithm fell through to the source-preservation branch.

Fix
Write the requested SSE algorithm whenever x-amz-server-side-encryption is present, and keep KMS-specific metadata handling limited to aws:kms.

Co-authored-by: Codex <noreply@openai.com>

* fix(s3api): reject unsupported copy encryption algorithms

A mistyped or unsupported x-amz-server-side-encryption value on a copy
request slipped past validation and got persisted as the destination's
algorithm header, advertising encryption that was never applied. Reject
anything other than AES256 or aws:kms up front.

* fix(s3api): write SSE key metadata for empty encrypted copies

A zero-byte source copied with an explicit SSE request took the
no-content branch and never ran the encryption path, leaving the object
with a bare algorithm header but no key. HEAD then advertised SSE while
the encryption-state machine saw the header as orphaned. Run the inline
encryption path when the destination requests encryption so the key
metadata is written too.

* s3api: use SSEAlgorithmKMS constant in copy metadata handling

* test(s3api): cover source SSE preservation on copy

* test(iam): allow the local client's real source IP in SourceIp tests

The aws:SourceIp allow policies hardcoded the loopback CIDRs, but a CI
runner reaching the server over localhost can be observed with one of the
host's RFC1918 addresses (the S3 endpoint is advertised on a 10.x
interface), so the positive-condition PutObject was denied and the allow
assertion flaked while the deny path passed trivially. Broaden the allow
list to loopback plus private ranges via a shared helper, and log the
denial on each failed attempt so any residual failure is diagnosable.

---------

Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
This commit is contained in:
7y-9
2026-06-22 22:19:24 -07:00
committed by GitHub
co-authored by Codex Chris Lu
parent 42ccfc0763
commit 44d575100a
4 changed files with 113 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/s3/iam/s3_iam_inline_policy_condition_test.go
+52 -52
View File
@@ -37,10 +37,43 @@ func isAccessDenied(err error) bool {
return ok && awsErr.Code() == "AccessDenied"
}
// localSourceAllowCIDRs lists every address a loopback-targeted test client may
// present as aws:SourceIp. The SDK reaches the server over localhost, but
// depending on resolver order and host routing the source the server observes
// can be IPv4 loopback, IPv6 loopback, or one of the host's RFC1918 addresses
// (CI runners advertise the S3 endpoint on a 10.x interface). An allow policy
// meant to match "this local client" must cover all of them or the
// positive-condition assertion flakes.
var localSourceAllowCIDRs = []string{
"127.0.0.0/8",
"::1/128",
"10.0.0.0/8",
"172.16.0.0/12",
"192.168.0.0/16",
}
// sourceIpAllowPolicy builds an Allow policy for s3:* on bucketName gated by an
// aws:SourceIp IpAddress condition over the given CIDRs.
func sourceIpAllowPolicy(bucketName string, cidrs ...string) string {
quoted := make([]string, len(cidrs))
for i, c := range cidrs {
quoted[i] = `"` + c + `"`
}
return `{
"Version":"2012-10-17",
"Statement":[{
"Effect":"Allow",
"Action":"s3:*",
"Resource":["arn:aws:s3:::` + bucketName + `","arn:aws:s3:::` + bucketName + `/*"],
"Condition":{"IpAddress":{"aws:SourceIp":[` + strings.Join(quoted, ",") + `]}}
}]
}`
}
// TestIAMUserInlinePolicySourceIpCondition verifies that an aws:SourceIp condition
// on a user inline policy is honored. Tests run from localhost (127.0.0.1), so a
// policy that only allows access from a non-loopback CIDR must deny the request,
// and a policy that allows access from 127.0.0.0/8 must allow it.
// on a user inline policy is honored. Tests run against a local server, so a
// policy that only allows a non-local CIDR must deny the request, and a policy
// that allows the local client's address (see localSourceAllowCIDRs) must allow it.
func TestIAMUserInlinePolicySourceIpCondition(t *testing.T) {
framework := NewS3IAMTestFramework(t)
defer framework.Cleanup()
@@ -87,30 +120,13 @@ func TestIAMUserInlinePolicySourceIpCondition(t *testing.T) {
}
})
policyDoc := func(cidrs ...string) string {
quoted := make([]string, len(cidrs))
for i, c := range cidrs {
quoted[i] = `"` + c + `"`
}
return `{
"Version":"2012-10-17",
"Statement":[{
"Effect":"Allow",
"Action":"s3:*",
"Resource":["arn:aws:s3:::` + bucketName + `","arn:aws:s3:::` + bucketName + `/*"],
"Condition":{"IpAddress":{"aws:SourceIp":[` + strings.Join(quoted, ",") + `]}}
}]
}`
}
t.Run("denies_when_source_ip_does_not_match", func(t *testing.T) {
// SourceIp 198.51.100.0/24 is RFC5737 TEST-NET-2; the test client is on
// loopback (127.0.0.1 or ::1 depending on resolver), so the condition
// must fail and the action must be denied.
// the local host, so the condition must fail and the action be denied.
_, err = iamClient.PutUserPolicy(&iam.PutUserPolicyInput{
UserName: aws.String(userName),
PolicyName: aws.String(policyName),
PolicyDocument: aws.String(policyDoc("198.51.100.0/24")),
PolicyDocument: aws.String(sourceIpAllowPolicy(bucketName, "198.51.100.0/24")),
})
require.NoError(t, err)
@@ -127,25 +143,25 @@ func TestIAMUserInlinePolicySourceIpCondition(t *testing.T) {
})
t.Run("allows_when_source_ip_matches", func(t *testing.T) {
// Cover both IPv4 and IPv6 loopback: on CI runners `localhost` may
// resolve to ::1 first, in which case a 127.0.0.0/8-only allow would
// silently never match and the test would hang.
_, err = iamClient.PutUserPolicy(&iam.PutUserPolicyInput{
UserName: aws.String(userName),
PolicyName: aws.String(policyName),
PolicyDocument: aws.String(policyDoc("127.0.0.0/8", "::1/128")),
PolicyDocument: aws.String(sourceIpAllowPolicy(bucketName, localSourceAllowCIDRs...)),
})
require.NoError(t, err)
require.Eventually(t, func() bool {
_, err := userS3.PutObject(&s3.PutObjectInput{
_, putErr := userS3.PutObject(&s3.PutObjectInput{
Bucket: aws.String(bucketName),
Key: aws.String("allowed.txt"),
Body: aws.ReadSeekCloser(strings.NewReader("ok")),
})
return err == nil
if putErr != nil {
t.Logf("allow attempt denied (source IP not in %v?): %v", localSourceAllowCIDRs, putErr)
}
return putErr == nil
}, 10*time.Second, 500*time.Millisecond,
"PutObject must succeed when aws:SourceIp condition matches the loopback range")
"PutObject must succeed when aws:SourceIp condition matches the local client address")
})
}
@@ -215,27 +231,8 @@ func TestIAMGroupInlinePolicyEnforcement(t *testing.T) {
}
})
// Cover both IPv4 and IPv6 loopback in the allow CIDR list: on CI runners
// `localhost` may resolve to ::1 first, in which case a 127.0.0.0/8-only
// allow would silently never match and the test would hang.
allowDoc := `{
"Version":"2012-10-17",
"Statement":[{
"Effect":"Allow",
"Action":"s3:*",
"Resource":["arn:aws:s3:::` + bucketName + `","arn:aws:s3:::` + bucketName + `/*"],
"Condition":{"IpAddress":{"aws:SourceIp":["127.0.0.0/8","::1/128"]}}
}]
}`
denyDoc := `{
"Version":"2012-10-17",
"Statement":[{
"Effect":"Allow",
"Action":"s3:*",
"Resource":["arn:aws:s3:::` + bucketName + `","arn:aws:s3:::` + bucketName + `/*"],
"Condition":{"IpAddress":{"aws:SourceIp":"198.51.100.0/24"}}
}]
}`
allowDoc := sourceIpAllowPolicy(bucketName, localSourceAllowCIDRs...)
denyDoc := sourceIpAllowPolicy(bucketName, "198.51.100.0/24")
t.Run("crud_round_trip", func(t *testing.T) {
_, err := iamClient.PutGroupPolicy(&iam.PutGroupPolicyInput{
@@ -277,12 +274,15 @@ func TestIAMGroupInlinePolicyEnforcement(t *testing.T) {
require.NoError(t, err)
require.Eventually(t, func() bool {
_, err := userS3.PutObject(&s3.PutObjectInput{
_, putErr := userS3.PutObject(&s3.PutObjectInput{
Bucket: aws.String(bucketName),
Key: aws.String("group-allowed.txt"),
Body: aws.ReadSeekCloser(strings.NewReader("ok")),
})
return err == nil
if putErr != nil {
t.Logf("allow attempt denied (source IP not in %v?): %v", localSourceAllowCIDRs, putErr)
}
return putErr == nil
}, 10*time.Second, 500*time.Millisecond,
"group member must be allowed when the group policy condition matches")
})
weed/s3api/s3api_copy_validation.go
+12 -2
View File
@@ -115,9 +115,19 @@ func validateSSEKMSCopyRequirements(srcMetadata map[string][]byte, headers http.
// validateEncryptionCompatibility validates that encryption methods are not conflicting
func validateEncryptionCompatibility(headers http.Header) error {
sseAlgorithm := headers.Get(s3_constants.AmzServerSideEncryption)
hasSSEC := hasSSECHeaders(headers)
hasSSEKMS := headers.Get(s3_constants.AmzServerSideEncryption) == "aws:kms"
hasSSES3 := headers.Get(s3_constants.AmzServerSideEncryption) == "AES256"
hasSSEKMS := sseAlgorithm == s3_constants.SSEAlgorithmKMS
hasSSES3 := sseAlgorithm == s3_constants.SSEAlgorithmAES256
// Reject unsupported algorithms so they are never persisted as a bogus
// destination header advertising encryption that was never applied.
if sseAlgorithm != "" && !hasSSEKMS && !hasSSES3 {
return &CopyValidationError{
Code: s3err.ErrInvalidEncryptionAlgorithm,
Message: fmt.Sprintf("Unsupported server-side encryption algorithm: %s", sseAlgorithm),
}
}
// Count how many encryption methods are specified
encryptionCount := 0
weed/s3api/s3api_object_handlers_copy.go
+20 -16
View File
@@ -350,8 +350,10 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request
if entry.Attributes.FileSize == 0 || len(entry.GetChunks()) == 0 {
dstEntry.Chunks = nil
// Handle inline encrypted content - fixes GitHub #7562
if len(entry.Content) > 0 {
// Handle inline encrypted content - fixes GitHub #7562.
// Also run when the destination requests encryption with no content so
// empty objects get real key metadata, not just a bare algorithm header.
if len(entry.Content) > 0 || dstWantsSSEC || dstWantsSSEKMS || dstWantsSSES3 {
inlineContent, inlineMetadata, inlineErr := s3a.processInlineContentForCopy(
entry, r, dstBucket, dstObject,
srcHasSSEC, srcHasSSEKMS, srcHasSSES3,
@@ -1029,26 +1031,28 @@ func processMetadataBytes(reqHeader http.Header, existing map[string][]byte, rep
metadata[s3_constants.AmzStorageClass] = []byte(sc)
}
// Handle SSE-KMS headers - these are always processed from request headers if present
if sseAlgorithm := reqHeader.Get(s3_constants.AmzServerSideEncryption); sseAlgorithm == "aws:kms" {
// Handle destination SSE headers from the request when present.
if sseAlgorithm := reqHeader.Get(s3_constants.AmzServerSideEncryption); sseAlgorithm != "" {
metadata[s3_constants.AmzServerSideEncryption] = []byte(sseAlgorithm)
// KMS Key ID (optional - can use default key)
if kmsKeyID := reqHeader.Get(s3_constants.AmzServerSideEncryptionAwsKmsKeyId); kmsKeyID != "" {
metadata[s3_constants.AmzServerSideEncryptionAwsKmsKeyId] = []byte(kmsKeyID)
}
if sseAlgorithm == s3_constants.SSEAlgorithmKMS {
// KMS Key ID (optional - can use default key)
if kmsKeyID := reqHeader.Get(s3_constants.AmzServerSideEncryptionAwsKmsKeyId); kmsKeyID != "" {
metadata[s3_constants.AmzServerSideEncryptionAwsKmsKeyId] = []byte(kmsKeyID)
}
// Encryption Context (optional)
if encryptionContext := reqHeader.Get(s3_constants.AmzServerSideEncryptionContext); encryptionContext != "" {
metadata[s3_constants.AmzServerSideEncryptionContext] = []byte(encryptionContext)
}
// Encryption Context (optional)
if encryptionContext := reqHeader.Get(s3_constants.AmzServerSideEncryptionContext); encryptionContext != "" {
metadata[s3_constants.AmzServerSideEncryptionContext] = []byte(encryptionContext)
}
// Bucket Key Enabled (optional)
if bucketKeyEnabled := reqHeader.Get(s3_constants.AmzServerSideEncryptionBucketKeyEnabled); bucketKeyEnabled != "" {
metadata[s3_constants.AmzServerSideEncryptionBucketKeyEnabled] = []byte(bucketKeyEnabled)
// Bucket Key Enabled (optional)
if bucketKeyEnabled := reqHeader.Get(s3_constants.AmzServerSideEncryptionBucketKeyEnabled); bucketKeyEnabled != "" {
metadata[s3_constants.AmzServerSideEncryptionBucketKeyEnabled] = []byte(bucketKeyEnabled)
}
}
} else {
// If not explicitly setting SSE-KMS, preserve existing SSE headers from source
// If not explicitly setting SSE, preserve existing SSE headers from source
for _, sseHeader := range []string{
s3_constants.AmzServerSideEncryption,
s3_constants.AmzServerSideEncryptionAwsKmsKeyId,
weed/s3api/s3api_object_handlers_copy_mime_test.go
+29
View File
@@ -3,6 +3,8 @@ package s3api
import (
"net/http"
"testing"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
)
func TestResolveDestinationMime(t *testing.T) {
@@ -279,3 +281,30 @@ func TestProcessMetadataBytes_CopyInheritsSystemHeaders(t *testing.T) {
t.Errorf("Content-Encoding = %q, want %q", got, "gzip")
}
}
func TestProcessMetadataBytes_CopyAppliesRequestedSSES3Header(t *testing.T) {
req := http.Header{}
req.Set(s3_constants.AmzServerSideEncryption, s3_constants.SSEAlgorithmAES256)
out, err := processMetadataBytes(req, nil, false, false)
if err != nil {
t.Fatalf("processMetadataBytes returned error: %v", err)
}
if got := string(out[s3_constants.AmzServerSideEncryption]); got != s3_constants.SSEAlgorithmAES256 {
t.Fatalf("%s = %q, want %q", s3_constants.AmzServerSideEncryption, got, s3_constants.SSEAlgorithmAES256)
}
}
func TestProcessMetadataBytes_CopyPreservesSourceSSEWhenRequestOmitsHeader(t *testing.T) {
existing := map[string][]byte{
s3_constants.AmzServerSideEncryption: []byte(s3_constants.SSEAlgorithmKMS),
}
out, err := processMetadataBytes(http.Header{}, existing, false, false)
if err != nil {
t.Fatalf("processMetadataBytes returned error: %v", err)
}
if got := string(out[s3_constants.AmzServerSideEncryption]); got != s3_constants.SSEAlgorithmKMS {
t.Fatalf("%s = %q, want %q", s3_constants.AmzServerSideEncryption, got, s3_constants.SSEAlgorithmKMS)
}
}