From a8766f51771ce4a3123d71edb5547352e0cd5841 Mon Sep 17 00:00:00 2001 From: stefan-wisbric Date: Sat, 7 Nov 2026 06:08:44 +1300 Subject: [PATCH] s3api: apply copy bucket-default SSE after the ACL re-auth (#12436) * s3api: apply copy bucket-default SSE after the ACL re-auth CopyObject without its own x-amz-server-side-encryption header takes the destination bucket's default encryption, which CopyObjectHandler writes into the request headers. It did so before preparePutObjectACL, which re-authenticates a request that carries an ACL (x-amz-acl or a grant header) for s3:PutObjectAcl. verifySignedHeadersCoverage then found the synthesized x-amz-server-side-encryption* headers outside SignedHeaders and rejected the copy with SignatureDoesNotMatch. Move the bucket-default injection after the ACL check, so the re-authentication sees the request as the client signed it. The destination object still gets the bucket default encryption. * s3api: assert the copy's KMS key and stored ciphertext Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Stefan Scharlott Co-authored-by: Chris Lu Co-authored-by: Claude Opus 5.5 --- weed/s3api/s3api_object_handlers_copy.go | 34 ++++++------ .../s3api_object_handlers_copy_acl_test.go | 53 +++++++++++++++++++ 2 files changed, 71 insertions(+), 16 deletions(-) diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index 2990eff785..a7b5aa6d27 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/weed/s3api/s3api_object_handlers_copy.go @@ -180,22 +180,6 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request return } - // A copy request without SSE headers takes the destination bucket's - // default encryption, including its configured KMS key and bucket-key - // setting. Surface them as headers so every downstream parse sees them. - if !IsSSECRequest(r) && r.Header.Get(s3_constants.AmzServerSideEncryption) == "" { - encryptionConfig, encErr := s3a.GetBucketEncryptionConfig(dstBucket) - switch { - case encErr == nil: - applyCopyBucketDefaultEncryptionHeaders(r, encryptionConfig) - case errors.Is(encErr, ErrNoEncryptionConfig): - default: - glog.Errorf("CopyObjectHandler: read encryption config for bucket %s: %v", dstBucket, encErr) - s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) - return - } - } - if !isValidDirective(r.Header.Get(s3_constants.AmzUserMetaDirective)) { s3err.WriteErrorResponse(w, r, s3err.ErrInvalidMetadataDirective) return @@ -239,6 +223,24 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request return } + // A copy request without SSE headers takes the destination bucket's + // default encryption, including its configured KMS key and bucket-key + // setting. Surface them as headers so every downstream parse sees them. + // This runs after the ACL check above: it re-authenticates the request, and + // these headers are not in the client's SignedHeaders. + if !IsSSECRequest(r) && r.Header.Get(s3_constants.AmzServerSideEncryption) == "" { + encryptionConfig, encErr := s3a.GetBucketEncryptionConfig(dstBucket) + switch { + case encErr == nil: + applyCopyBucketDefaultEncryptionHeaders(r, encryptionConfig) + case errors.Is(encErr, ErrNoEncryptionConfig): + default: + glog.Errorf("CopyObjectHandler: read encryption config for bucket %s: %v", dstBucket, encErr) + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + return + } + } + // Get detailed versioning state for source bucket srcVersioningState, err := s3a.getVersioningState(srcBucket) if err != nil { diff --git a/weed/s3api/s3api_object_handlers_copy_acl_test.go b/weed/s3api/s3api_object_handlers_copy_acl_test.go index 583b27aa08..604e786947 100644 --- a/weed/s3api/s3api_object_handlers_copy_acl_test.go +++ b/weed/s3api/s3api_object_handlers_copy_acl_test.go @@ -22,6 +22,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/filer" "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/pb/s3_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/seaweedfs/seaweedfs/weed/wdclient" @@ -490,3 +491,55 @@ func TestCopyObjectDestinationACL(t *testing.T) { }) } } + +// A copy that sets an ACL is re-authenticated for s3:PutObjectAcl. The +// destination bucket's default encryption must not be written into the client +// request before that check: the synthesized x-amz-server-side-encryption +// headers are not in SignedHeaders and fail with SignatureDoesNotMatch. +func TestCopyObjectACLWithBucketDefaultEncryption(t *testing.T) { + const bucket, writer = "copy-acl-sse", "copy-writer" + kmsKey := SetupTestKMS(t) + t.Cleanup(kmsKey.Cleanup) + filer := ©ACLFiler{ambiguousPutFiler: &ambiguousPutFiler{apply: true, entries: map[string]*filer_pb.Entry{}}} + s3a := newPutTestServer(t, startFakeFiler(t, filer)) + s3a.iam = NewIdentityAccessManagementWithStore(s3a.option, nil, "memory") + t.Cleanup(s3a.iam.Shutdown) + s3a.iam.isAuthEnabled = true + account := &Account{Id: writer, DisplayName: writer} + identity := &Identity{Name: "copy-acl-sse-test", Account: account, IsStatic: true, + Credentials: []*Credential{{AccessKey: routingTestAccessKey, SecretKey: routingTestSecretKey}}, + Actions: []Action{"Read:" + bucket, "Write:" + bucket, "WriteAcp:" + bucket}} + s3a.iam.accessKeyIdent[routingTestAccessKey] = identity + s3a.iam.nameToIdentity[identity.Name] = identity + s3a.iam.accounts[writer] = account + bucketEntry := &filer_pb.Entry{Name: bucket, IsDirectory: true, Attributes: &filer_pb.FuseAttributes{}, Extended: map[string][]byte{ + s3_constants.ExtAmzOwnerKey: []byte(writer), s3_constants.ExtOwnershipKey: []byte(s3_constants.OwnershipObjectWriter), + }} + filer.entries["/buckets/"+bucket] = bucketEntry + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.bucketConfigCache.Set(bucket, &BucketConfig{Name: bucket, Owner: writer, Ownership: s3_constants.OwnershipObjectWriter, + Encryption: &s3_pb.EncryptionConfiguration{SseAlgorithm: "aws:kms", KmsKeyId: kmsKey.KeyID}}) + s3a.bucketRegistry = NewBucketRegistry(s3a) + s3a.bucketRegistry.LoadBucketMetadata(bucketEntry) + filer.entries["/buckets/"+bucket+"/source"] = &filer_pb.Entry{Name: "source", Content: []byte("data"), + Attributes: &filer_pb.FuseAttributes{FileMode: defaultFileMode, FileSize: 4, Mime: "text/plain"}} + + req := httptest.NewRequest(http.MethodPut, "http://s3/"+bucket+"/destination", nil) + req = mux.SetURLVars(req, map[string]string{"bucket": bucket, "object": "destination"}) + req.Header.Set("X-Amz-Copy-Source", "/"+bucket+"/source") + req.Header.Set(s3_constants.AmzCannedAcl, "private") + signRoutingTestRequest(t, req, "", "s3") + rr := httptest.NewRecorder() + s3a.iam.Auth(s3a.CopyObjectHandler, s3_constants.ACTION_WRITE)(rr, req) + + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + filer.mu.Lock() + defer filer.mu.Unlock() + stored := filer.entries["/buckets/"+bucket+"/destination"] + require.NotNil(t, stored) + sseKMSKey, err := DeserializeSSEKMSMetadata(stored.Extended[s3_constants.SeaweedFSSSEKMSKey]) + require.NoError(t, err, "the copy must still take the bucket default SSE-KMS") + require.Equal(t, kmsKey.KeyID, sseKMSKey.KeyID) + require.NotEmpty(t, stored.Content) + require.NotEqual(t, []byte("data"), stored.Content, "the copy must be stored encrypted") +}