From 2d06e2d6c12480cc74bbf7da9399b67b7d3a58e8 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 16 Oct 2026 19:50:36 -0700 Subject: [PATCH] s3: evaluate tag conditions on copy-source and tagging subresources (#11771) * s3: evaluate tag conditions on copy-source and tagging subresources Policy evaluation ran without the object entry on several paths, so a Deny conditioned on s3:ExistingObjectTag never matched: - CopyObject, UploadPartCopy and RenameObject checked the source key before its entry was fetched; repeat the check on a synthesized GET once the entry resolves. - GetObjectAttributes and Get/Put/DeleteObjectTagging never supplied the entry; recheck with the resolved entry. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: fail closed on tag-policy lookups and honor tag-conditioned grants - recheckTagConditionedPolicyWithLookup returned success when the entry lookup errored, letting a tag-gated Deny slip past a transient miss; only ErrNotFound still falls through to the handler's own NoSuchKey. - A read or delete denied in the tagless first phase now gets one more evaluation with the resolved entry's tags in hand, so an s3:ExistingObjectTag-conditioned Allow can still grant a copy-source read or rename. The deferred recheck requires an explicit Allow; indeterminate results keep the original verdict, and the existence of the key never leaks past a deny. - RenameObject now applies the same tag-aware recheck to the source removal, so a tag-gated Deny on s3:DeleteObject blocks a rename the way it blocks a direct DELETE. - GetObjectAttributes evaluates the object-entry policy recheck before conditional headers, so a 304/412 response can no longer bypass a tag-based Deny. * s3: honor identity denies in the tag-aware source recheck The deferred recheck re-evaluates only the bucket policy, so a bucket Allow could lift an explicit Deny in the caller's attached or session policies on a copy/rename source. Apply the same applicable-policy Deny check the first phase uses. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/s3api/auth_credentials.go | 31 +++-- weed/s3api/s3_existing_object_tag_test.go | 112 ++++++++++++++++++ .../s3api_bucket_policy_identity_deny_test.go | 31 +++++ .../s3api/s3api_object_handlers_attributes.go | 5 + weed/s3api/s3api_object_handlers_copy.go | 26 +++- weed/s3api/s3api_object_handlers_rename.go | 18 ++- weed/s3api/s3api_object_handlers_tagging.go | 26 +++- weed/s3api/s3api_server.go | 74 ++++++++++++ 8 files changed, 301 insertions(+), 22 deletions(-) diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 11c4a105e..2884542eb 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -2916,16 +2916,12 @@ func (iam *IdentityAccessManagement) bucketsNamedByAttachedPolicies(r *http.Requ // its query describe. versionId, when set, and the STS session token ride along // for policy conditions. Returns ErrNone when auth is disabled (checked by the // callers) or the identity is an admin. -func (iam *IdentityAccessManagement) authorizeObjectKeyAction(r *http.Request, identity *Identity, method string, action Action, bucket, objectKey, versionId string) s3err.ErrorCode { - if identity == nil { - return s3err.ErrAccessDenied - } - if identity.isAdmin() { - return s3err.ErrNone - } - - // Shallow copy: authorization only reads headers, so sharing the header map - // with the original request is safe, and this can run once per key. +// objectKeyPolicyRequest synthesizes a // request +// for policy evaluation against a key the real URL does not name. The query is +// built from scratch so a param on the real request (delete, uploadId, +// partNumber, ...) cannot steer ResolveS3Action off the intended action; the +// STS token and versionId ride along for policy conditions. +func objectKeyPolicyRequest(r *http.Request, method, bucket, objectKey, versionId string) *http.Request { keyReq := new(http.Request) *keyReq = *r keyURL := &url.URL{ @@ -2933,9 +2929,6 @@ func (iam *IdentityAccessManagement) authorizeObjectKeyAction(r *http.Request, i Host: r.URL.Host, Path: "/" + bucket + "/" + objectKey, } - // Build the query from scratch so a param on the real request (delete, - // uploadId, partNumber, ...) cannot steer ResolveS3Action off the intended - // action; keep the STS token and per-key versionId for policy conditions. keyQuery := make(url.Values) if versionId != "" { keyQuery.Set("versionId", versionId) @@ -2954,6 +2947,18 @@ func (iam *IdentityAccessManagement) authorizeObjectKeyAction(r *http.Request, i keyReq.Body = nil keyReq.GetBody = nil keyReq.ContentLength = 0 + return keyReq +} + +func (iam *IdentityAccessManagement) authorizeObjectKeyAction(r *http.Request, identity *Identity, method string, action Action, bucket, objectKey, versionId string) s3err.ErrorCode { + if identity == nil { + return s3err.ErrAccessDenied + } + if identity.isAdmin() { + return s3err.ErrNone + } + + keyReq := objectKeyPolicyRequest(r, method, bucket, objectKey, versionId) if iam.policyEngine != nil { principal := buildPrincipalARN(identity, keyReq) diff --git a/weed/s3api/s3_existing_object_tag_test.go b/weed/s3api/s3_existing_object_tag_test.go index 0adea6976..9a986eb78 100644 --- a/weed/s3api/s3_existing_object_tag_test.go +++ b/weed/s3api/s3_existing_object_tag_test.go @@ -4,6 +4,9 @@ import ( "net/http" "net/http/httptest" "testing" + "time" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" @@ -257,3 +260,112 @@ func TestHasPolicyForBucket(t *testing.T) { // No longer has policy assert.False(t, engine.HasPolicyForBucket("test-bucket")) } + +// TestRecheckCopySourcePolicy verifies that a tag-conditioned Deny on +// s3:GetObject also blocks CopyObject-style reads of the source object. The +// destination request keeps its PUT method while the policy is evaluated +// against a synthesized GET on the source key. +func TestRecheckCopySourcePolicy(t *testing.T) { + s3a := &S3ApiServer{ + policyEngine: NewBucketPolicyEngine(), + } + + policyJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:*", + "Resource": ["arn:aws:s3:::src-bucket", "arn:aws:s3:::src-bucket/*", "arn:aws:s3:::dst-bucket", "arn:aws:s3:::dst-bucket/*"] + }, + { + "Effect": "Deny", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::src-bucket/*", + "Condition": { + "StringEquals": { + "s3:ExistingObjectTag/status": ["private"] + } + } + } + ] + }` + require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("src-bucket", policyJSON)) + + identity := &Identity{Name: "denied-user", PrincipalArn: "arn:aws:iam::123456789012:user/denied"} + + req := httptest.NewRequest(http.MethodPut, "/dst-bucket/dst-key", nil) + req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), identity)) + + privateEntry := map[string][]byte{ + s3_constants.AmzObjectTaggingPrefix + "status": []byte("private"), + } + assert.Equal(t, s3err.ErrAccessDenied, + s3a.recheckCopySourcePolicy(req, "src-bucket", "src-key", "", privateEntry, "test")) + + publicEntry := map[string][]byte{ + s3_constants.AmzObjectTaggingPrefix + "status": []byte("public"), + } + assert.Equal(t, s3err.ErrNone, + s3a.recheckCopySourcePolicy(req, "src-bucket", "src-key", "", publicEntry, "test")) +} + +// A source read whose only grant is conditioned on s3:ExistingObjectTag denies +// in the tagless first phase; the deferred lookup recheck must still grant it +// when the resolved entry's tags satisfy the Allow, and keep denying when they +// do not. The same recheck with a synthesized DELETE is what a rename applies +// to the source removal. +func TestRecheckPolicyForKeyWithLookup(t *testing.T) { + filerImpl := &ambiguousPutFiler{ + entries: map[string]*filer_pb.Entry{ + "/buckets/src-bucket/src-key": { + Name: "src-key", + Extended: map[string][]byte{ + s3_constants.AmzObjectTaggingPrefix + "status": []byte("public"), + }, + }, + }, + } + s3a := newPutTestServer(t, startFakeFiler(t, filerImpl)) + s3a.policyEngine = NewBucketPolicyEngine() + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.bucketConfigCache.Set("src-bucket", &BucketConfig{Name: "src-bucket"}) + + policyJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": "arn:aws:s3:::src-bucket/*", + "Condition": { + "StringEquals": { + "s3:ExistingObjectTag/status": ["public"] + } + } + } + ] + }` + require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("src-bucket", policyJSON)) + + identity := &Identity{Name: "tagged-user", PrincipalArn: "arn:aws:iam::123456789012:user/tagged"} + req := httptest.NewRequest(http.MethodPut, "/dst-bucket/dst-key", nil) + req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), identity)) + + assert.Equal(t, s3err.ErrNone, + s3a.recheckPolicyForKeyWithLookup(req, http.MethodGet, "src-bucket", "src-key", "", string(s3_constants.ACTION_READ), "test"), + "a tag-conditioned Allow on the resolved entry must still grant the read") + + filerImpl.entries["/buckets/src-bucket/src-key"].Extended[s3_constants.AmzObjectTaggingPrefix+"status"] = []byte("private") + assert.Equal(t, s3err.ErrAccessDenied, + s3a.recheckPolicyForKeyWithLookup(req, http.MethodGet, "src-bucket", "src-key", "", string(s3_constants.ACTION_READ), "test"), + "non-matching tags must keep the first-phase deny") + + // Without a policy on the bucket the recheck must not widen anything. + require.NoError(t, s3a.policyEngine.engine.DeleteBucketPolicy("src-bucket")) + assert.Equal(t, s3err.ErrAccessDenied, + s3a.recheckPolicyForKeyWithLookup(req, http.MethodGet, "src-bucket", "src-key", "", string(s3_constants.ACTION_READ), "test")) +} diff --git a/weed/s3api/s3api_bucket_policy_identity_deny_test.go b/weed/s3api/s3api_bucket_policy_identity_deny_test.go index 67cb08f45..8ea9008fa 100644 --- a/weed/s3api/s3api_bucket_policy_identity_deny_test.go +++ b/weed/s3api/s3api_bucket_policy_identity_deny_test.go @@ -3,10 +3,13 @@ package s3api import ( "fmt" "net/http" + "net/http/httptest" "sync" "testing" + "time" "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" @@ -180,3 +183,31 @@ func TestAuthorizeObjectKeyActionBucketAllowDoesNotOverrideIdentityDeny(t *testi iam.AuthorizeCopySource(req, reader, bpdBucket, "other/data.txt", ""), "AuthorizeCopySource must preserve the bucket Allow over an implicit identity denial") } + +// A deferred tag-aware recheck (copy/rename source) re-runs only the bucket +// policy; it must still honor an explicit Deny in the caller's attached or +// session policies, or the phase-1 veto on secret/* could be bypassed. +func TestRecheckPolicyForKeyWithLookupHonorsIdentityDeny(t *testing.T) { + filerImpl := &ambiguousPutFiler{ + entries: map[string]*filer_pb.Entry{ + "/buckets/" + bpdBucket + "/secret/payroll.txt": {Name: "payroll.txt"}, + "/buckets/" + bpdBucket + "/other/data.txt": {Name: "data.txt"}, + }, + } + s3a := newPutTestServer(t, startFakeFiler(t, filerImpl)) + iam := newBucketPolicyDenyIAM(t, bpdBucketPolicyDoc(t, "secret/*", "other/*")) + s3a.iam = iam + s3a.policyEngine = iam.policyEngine + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.bucketConfigCache.Set(bpdBucket, &BucketConfig{Name: bpdBucket}) + + req := httptest.NewRequest(http.MethodPut, "/victim-bucket/dst-key", nil) + req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), bpdReader(t, iam))) + + assert.Equal(t, s3err.ErrAccessDenied, + s3a.recheckPolicyForKeyWithLookup(req, http.MethodGet, bpdBucket, "secret/payroll.txt", "", s3_constants.ACTION_READ, "test"), + "an explicit identity-policy Deny must survive the bucket-policy recheck") + assert.Equal(t, s3err.ErrNone, + s3a.recheckPolicyForKeyWithLookup(req, http.MethodGet, bpdBucket, "other/data.txt", "", s3_constants.ACTION_READ, "test"), + "a bucket-policy Allow on a prefix the identity does not deny must still grant") +} diff --git a/weed/s3api/s3api_object_handlers_attributes.go b/weed/s3api/s3api_object_handlers_attributes.go index 901409a26..f483c13a2 100644 --- a/weed/s3api/s3api_object_handlers_attributes.go +++ b/weed/s3api/s3api_object_handlers_attributes.go @@ -219,6 +219,11 @@ func (s3a *S3ApiServer) GetObjectAttributesHandler(w http.ResponseWriter, r *htt } } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES, entry.Extended, "GetObjectAttributesHandler"); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + // Evaluate conditional headers against the resolved entry (after version resolution) // This ensures conditions are checked against the correct version, not always the latest if s3a.hasConditionalHeaders(r) { diff --git a/weed/s3api/s3api_object_handlers_copy.go b/weed/s3api/s3api_object_handlers_copy.go index 3d85058f8..29858368d 100644 --- a/weed/s3api/s3api_object_handlers_copy.go +++ b/weed/s3api/s3api_object_handlers_copy.go @@ -202,10 +202,14 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request // CopyObject requires s3:GetObject on the source in addition to s3:PutObject // on the destination. The Auth middleware only checked the destination, so - // verify the source explicitly here. + // verify the source explicitly here. A first-phase deny decided without + // object tags can still flip on a tag-conditioned Allow, so when a bucket + // policy exists the tag-aware recheck gets the last word. if errCode := s3a.authorizeCopySource(r, srcBucket, srcObject, srcVersionId); errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) - return + if s3a.recheckPolicyForKeyWithLookup(r, http.MethodGet, srcBucket, srcObject, srcVersionId, string(s3_constants.ACTION_READ), "CopyObjectHandler") != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } } // A copy creates destination ownership and grants independently of the @@ -238,6 +242,11 @@ func (s3a *S3ApiServer) CopyObjectHandler(w http.ResponseWriter, r *http.Request return } + if errCode := s3a.recheckCopySourcePolicy(r, srcBucket, srcObject, srcVersionId, entry.Extended, "CopyObjectHandler"); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + // Cache remote-only sources before copying; otherwise the copy path below // writes a destination with FileSize > 0 but no chunks/content. if entry.IsInRemoteOnly() { @@ -931,8 +940,10 @@ func (s3a *S3ApiServer) CopyObjectPartHandler(w http.ResponseWriter, r *http.Req // UploadPartCopy requires s3:GetObject on the source. The Auth middleware // only verified s3:PutObject (s3:UploadPart) on the destination. if errCode := s3a.authorizeCopySource(r, srcBucket, srcObject, srcVersionId); errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) - return + if s3a.recheckPolicyForKeyWithLookup(r, http.MethodGet, srcBucket, srcObject, srcVersionId, string(s3_constants.ACTION_READ), "CopyObjectPartHandler") != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } } partIDString := r.URL.Query().Get("partNumber") @@ -1001,6 +1012,11 @@ func (s3a *S3ApiServer) CopyObjectPartHandler(w http.ResponseWriter, r *http.Req return } + if errCode := s3a.recheckCopySourcePolicy(r, srcBucket, srcObject, srcVersionId, entry.Extended, "CopyObjectPartHandler"); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + // Cache remote-only sources before copying; the part-copy paths below // iterate entry.GetChunks() and would otherwise produce an empty part. if entry.IsInRemoteOnly() { diff --git a/weed/s3api/s3api_object_handlers_rename.go b/weed/s3api/s3api_object_handlers_rename.go index 08b189d75..fc1ba4436 100644 --- a/weed/s3api/s3api_object_handlers_rename.go +++ b/weed/s3api/s3api_object_handlers_rename.go @@ -77,8 +77,14 @@ func (s3a *S3ApiServer) RenameObjectHandler(w http.ResponseWriter, r *http.Reque // is what the request URL names. The source arrives in a header and loses // its key, so it needs both read and delete permission checked here. if errCode := s3a.authorizeRenameSource(r, bucket, srcObject); errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) - return + // Same tag-conditioned-Allow deferral as the copy path: the first + // phase ran without the source's tags, so a tag-gated Allow can still + // grant both the read and the delete the rename performs. + if s3a.recheckPolicyForKeyWithLookup(r, http.MethodGet, bucket, srcObject, "", string(s3_constants.ACTION_READ), "RenameObjectHandler") != s3err.ErrNone || + s3a.recheckPolicyForKeyWithLookup(r, http.MethodDelete, bucket, srcObject, "", string(s3_constants.ACTION_WRITE), "RenameObjectHandler") != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } } versioningState, err := s3a.getVersioningState(bucket) @@ -115,6 +121,14 @@ func (s3a *S3ApiServer) RenameObjectHandler(w http.ResponseWriter, r *http.Reque if srcErrCode != s3err.ErrNone { return srcErrCode } + if errCode := s3a.recheckCopySourcePolicy(r, bucket, srcObject, "", entry.Extended, "RenameObjectHandler"); errCode != s3err.ErrNone { + return errCode + } + // The rename also deletes the source; a tag-gated Deny on the delete + // applies the same way it does to a direct DELETE. + if errCode := s3a.recheckPolicyWithObjectEntry(objectKeyPolicyRequest(r, http.MethodDelete, bucket, srcObject, ""), bucket, srcObject, string(s3_constants.ACTION_WRITE), entry.Extended, "RenameObjectHandler"); errCode != s3err.ErrNone { + return errCode + } if errCode := validateSourceConditionalHeaders(r, entry, renameSourceConditionalHeaders); errCode != s3err.ErrNone { return errCode } diff --git a/weed/s3api/s3api_object_handlers_tagging.go b/weed/s3api/s3api_object_handlers_tagging.go index 976721888..9cb867603 100644 --- a/weed/s3api/s3api_object_handlers_tagging.go +++ b/weed/s3api/s3api_object_handlers_tagging.go @@ -74,7 +74,7 @@ func (s3a *S3ApiServer) GetObjectTaggingHandler(w http.ResponseWriter, r *http.R target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object)) dir, name := target.DirAndName() - tags, err := s3a.getTags(dir, name) + entry, err = s3a.getEntry(dir, name) if err != nil { if err == filer_pb.ErrNotFound { glog.Errorf("GetObjectTaggingHandler %s: %v", r.URL, err) @@ -85,8 +85,10 @@ func (s3a *S3ApiServer) GetObjectTaggingHandler(w http.ResponseWriter, r *http.R } return } + } - writeSuccessResponseXML(w, r, FromTags(tags)) + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_GET_OBJECT_TAGGING, entry.Extended, "GetObjectTaggingHandler"); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) return } @@ -183,6 +185,11 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object)) dir, name := target.DirAndName() + if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, s3_constants.S3_ACTION_PUT_OBJECT_TAGGING, "PutObjectTaggingHandler"); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + if err = s3a.setTags(dir, name, tags); err != nil { if err == filer_pb.ErrNotFound { glog.Errorf("PutObjectTaggingHandler setTags %s: %v", r.URL, err) @@ -199,6 +206,11 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R return } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_PUT_OBJECT_TAGGING, entry.Extended, "PutObjectTaggingHandler"); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + // For versioned objects, resolve the directory holding the entry to update. A null // version lives under its full key's parent directory, so a nested key must not fall // back to the bucket root and rewrite a different object sharing the same basename. @@ -301,6 +313,11 @@ func (s3a *S3ApiServer) DeleteObjectTaggingHandler(w http.ResponseWriter, r *htt target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object)) dir, name := target.DirAndName() + if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING, "DeleteObjectTaggingHandler"); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + err := s3a.rmTags(dir, name) if err != nil { if err == filer_pb.ErrNotFound { @@ -318,6 +335,11 @@ func (s3a *S3ApiServer) DeleteObjectTaggingHandler(w http.ResponseWriter, r *htt return } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING, entry.Extended, "DeleteObjectTaggingHandler"); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + // For versioned objects, resolve the directory holding the entry to update. A null // version lives under its full key's parent directory, so a nested key must not fall // back to the bucket root and rewrite a different object sharing the same basename. diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 920d8fb47..9b01528bf 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -28,6 +28,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/iam/policy" "github.com/seaweedfs/seaweedfs/weed/iam/sts" "github.com/seaweedfs/seaweedfs/weed/pb" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" "github.com/seaweedfs/seaweedfs/weed/pb/s3_lifecycle_pb" "github.com/seaweedfs/seaweedfs/weed/pb/s3_pb" @@ -675,6 +676,79 @@ func (s3a *S3ApiServer) recheckPolicyWithObjectEntry(r *http.Request, bucket, ob return errCode } +// recheckCopySourcePolicy repeats the source object's policy check once its +// entry is fetched, so conditions on the object's existing tags apply to a +// CopyObject / UploadPartCopy / RenameObject source the same way they do to a +// plain GET. The synthesized GET keeps the destination's request shape (PUT, +// partNumber, ...) from resolving a different action. +func (s3a *S3ApiServer) recheckCopySourcePolicy(r *http.Request, bucket, object, versionId string, objectEntry map[string][]byte, handlerName string) s3err.ErrorCode { + keyReq := objectKeyPolicyRequest(r, http.MethodGet, bucket, object, versionId) + return s3a.recheckPolicyWithObjectEntry(keyReq, bucket, object, string(ACTION_READ), objectEntry, handlerName) +} + +// recheckTagConditionedPolicyWithLookup repeats policy evaluation with the +// object's entry in hand so s3:ExistingObjectTag conditions apply to tagging +// writes that did not otherwise fetch the entry. It only does the extra +// lookup when the bucket has a policy; entry misses return ErrNone because +// the handler reports its own NoSuchKey. +func (s3a *S3ApiServer) recheckTagConditionedPolicyWithLookup(r *http.Request, bucket, object, action, handlerName string) s3err.ErrorCode { + if s3a.policyEngine == nil || !s3a.policyEngine.HasPolicyForBucket(bucket) { + return s3err.ErrNone + } + target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object)) + dir, name := target.DirAndName() + entry, err := s3a.getEntry(dir, name) + if err != nil { + // The tag conditions cannot be evaluated without the entry; failing + // open would let a tag-gated Deny slip through a transient miss. + if errors.Is(err, filer_pb.ErrNotFound) { + return s3err.ErrNone + } + glog.Errorf("%s: recheck lookup for %s/%s failed: %v", handlerName, bucket, object, err) + return s3err.ErrInternalError + } + if entry == nil { + return s3err.ErrNone + } + return s3a.recheckPolicyWithObjectEntry(r, bucket, object, action, entry.Extended, handlerName) +} + +// recheckPolicyForKeyWithLookup re-decides a first-phase deny on one key for a +// synthesized method, resolving the entry the way the read path does (versions +// included) so s3:ExistingObjectTag conditions see the object's real tags. +// Unlike the deny-side rechecks, an indeterminate result does not grant: the +// caller reports the original verdict either way, so only a statement that +// matches with tags in hand can turn a deny into an allow, and a missed lookup +// never widens access or reveals whether the key exists. +func (s3a *S3ApiServer) recheckPolicyForKeyWithLookup(r *http.Request, method, bucket, object, versionId, action, handlerName string) s3err.ErrorCode { + if s3a.policyEngine == nil || !s3a.policyEngine.HasPolicyForBucket(bucket) { + return s3err.ErrAccessDenied + } + versioningState, err := s3a.getVersioningState(bucket) + if err != nil { + glog.Errorf("%s: recheck versioning state for %s: %v", handlerName, bucket, err) + return s3err.ErrAccessDenied + } + entry, err := s3a.resolveCopySourceEntry(bucket, object, versionId, versioningState) + entry = prefixObjectSource(entry) + if err != nil || entry == nil { + return s3err.ErrAccessDenied + } + keyReq := objectKeyPolicyRequest(r, method, bucket, object, versionId) + errCode, checked := s3a.checkPolicyWithEntry(keyReq, bucket, object, action, "", entry.Extended) + if errCode != s3err.ErrNone || !checked { + return s3err.ErrAccessDenied + } + // A bucket-policy Allow never overrides an explicit Deny in the caller's + // own attached or session policies — the first phase applied that veto, + // so the tag-aware recheck must too. + identity, _ := GetIdentityFromContext(r).(*Identity) + if s3a.iam != nil && s3a.iam.isActionExplicitlyDeniedByApplicablePolicies(keyReq, identity, Action(action), bucket, object) { + return s3err.ErrAccessDenied + } + return s3err.ErrNone +} + // classifyDomainNames classifies domains into path-style and virtual-host style domains. // A domain is considered path-style if: // 1. It contains a dot (has subdomains)