Commit Graph
2 Commits
Author SHA1 Message Date
Chris LuandDevin 2d06e2d6c1 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>
2026-10-16 19:50:36 -07:00
Chris Lu 7fa2f75f30 s3: bucket-policy Allow must not override an identity explicit Deny (#11256)
* s3: add isActionExplicitlyDeniedByApplicablePolicies helper

Add a helper that reports whether any applicable identity-side policy
(attached IAM policies, enabled-group policies, or the IAM-integration
session policy) explicitly denies an action. It reuses the existing
evaluateAttachedIAMPolicies, resolveS3AuthTarget, buildPrincipalARN, and
isActionExplicitlyDeniedByIAM helpers, and fails closed on evaluation
errors. A nil identity has no identity-side policy plane, so the helper
returns false to keep the bucket policy authoritative for anonymous
access. No behavior change yet; the next commits apply it to the two
bucket-policy Allow short-circuits.

* s3: enforce identity explicit Deny before bucket-policy Allow

authRequestWithAuthType short-circuits on a matching bucket-policy Allow
and skips VerifyActionPermission, so an explicit Deny in an authenticated
identity attached, group, or session policy is bypassed. A non-admin
principal with s3:PutBucketPolicy can install a bucket-policy Allow for
itself and read an object its identity policy explicitly denies.

Before honoring a bucket-policy Allow, check the applicable identity-side
policies for a matching explicit Deny via the new
isActionExplicitlyDeniedByApplicablePolicies helper, and fail closed.
The cross-account behavior is preserved: a bucket Allow still supplies the
Allow an identity policy omits (implicit denial), and a nil identity keeps
the bucket policy authoritative for anonymous access.

Regression tests cover the explicit-Deny override, the implicit-deny
Allow preservation, and the unmatched-key fall-through control.

* s3: enforce identity explicit Deny in secondary object-key auth

authorizeObjectKeyAction authorizes keys the request URL does not name
(CopySource, DeleteObjects body keys, POST Object form keys) and shares
the same bucket-policy Allow short-circuit as the primary path, so an
explicit Deny in the identity, group, or session policy is bypassed the
same way when a bucket policy allows the secondary key.

Apply isActionExplicitlyDeniedByApplicablePolicies before accepting the
bucket-policy Allow, mirroring the primary path. A regression test covers
AuthorizeCopySource for both the explicit-Deny override and the
implicit-deny Allow preservation.
2026-09-09 20:15:43 -07:00