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>
This commit is contained in:
Chris LuandDevin authored and GitHub committed 2026-10-16 19:50:36 -07:00
1 parent 446e6f3444
commit 2d06e2d6c1
8 files changed
+301 -22

No files matched your search

+18 -13
View File
@@ -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 <method> /<bucket>/<objectKey> 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)