mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-07 12:37:41 +01:00
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 <noreply@anthropic.com> --------- Co-authored-by: Stefan Scharlott <stefans@Stefans-MacBook-Pro.local> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2 files changed
+71
-16
No files matched your search
@@ -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 {
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
Reference in new issue
Block a user