Skip to content

fix(core,aws,s3): auth enforcement gaps (bug-hunt 5.1/5.3/5.4)#2288

Merged
vieiralucas merged 3 commits into
mainfrom
bh-auth
Jul 16, 2026
Merged

fix(core,aws,s3): auth enforcement gaps (bug-hunt 5.1/5.3/5.4)#2288
vieiralucas merged 3 commits into
mainfrom
bh-auth

Conversation

@vieiralucas

@vieiralucas vieiralucas commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary

Three authorization-correctness gaps from the 2026-07-15 bug hunt, each in a path CI's canonical happy-path never exercised. All three are opt-in-enforcement bugs (off-by-default is by design; these are wrong-allow / wrong-mapping when enforcement is on).

  • core/dispatch (5.1) — explicit Deny must override public ACL. The anonymous-access gate collapsed the resource-policy decision to a bool and ORed the public-read ACL onto it, so a public-read ACL could grant access the bucket owner had explicitly denied. Now an ExplicitDeny short-circuits the ACL grant (AWS Deny-overrides precedence).
  • s3 (5.3) — access-point management mapped to the wrong IAM actions. S3 Control access-point ops (host s3-control..., path /v20180820/accesspoint[/name]) were authorized under the generic method-based object actions (GetObject/PutObject/DeleteObject) instead of s3:CreateAccessPoint / GetAccessPoint / DeleteAccessPoint / ListAccessPoints, making access-point IAM policies ineffective. iam_action_for now detects the s3-control shape first.
  • aws/sigv4 (5.4) — payload hash not bound to the body. A concrete x-amz-content-sha256 is now verified against the actual request body during canonicalization; without it a captured valid signature could be replayed over a swapped body. UNSIGNED-PAYLOAD / STREAMING-* markers are signed literally and skip the rehash.

Dropped as verified false positives (documented, not deferred)

  • 5.5 (deny signed-but-unresolvable AKID under strict): fakecloud deliberately tolerates unverified signed creds when verify_sigv4 is off (the dummy-cred UX — the default AKIAIOSFODNN7EXAMPLE is itself unresolved), and the genuinely-unverifiable case (verify_sigv4 on) already returns InvalidClientTokenId. Enforcing it would break the documented UX and existing strict-mode tests.
  • 5.8 (map GetSecurityTokenServicePreferences to an IAM action): a fakecloud read-back extension absent from AWS's iam.json; an existing test pins it to no IAM action, and adding it would fail the handwritten-test audit (no Smithy model to checksum).

Test plan

  • cargo test -p fakecloud-aws --lib sigv4 — body-hash mismatch / match / unsigned-marker (new).
  • cargo test -p fakecloud-s3 --lib iam_action_for_maps_access_point_control_ops (new) — plus sanity that a plain object GET still maps to GetObject.
  • cargo test -p fakecloud-e2e --test s3_anonymous_access — new anonymous_get_explicit_deny_overrides_public_acl under --iam strict; all 6 pass (existing strict tests unregressed).
  • cargo clippy -p fakecloud-aws -p fakecloud-core -p fakecloud-s3 --tests -- -D warnings clean.

No API surface / SDK / docs change (internal authorization behavior only).


Summary by cubic

Fix auth enforcement for anonymous S3 access and access-point IAM mapping, and adjust SigV4 canonicalization to prevent false signature failures. No public API changes.

  • Bug Fixes
    • fakecloud-core: An ExplicitDeny in a bucket policy now overrides a public-read ACL for anonymous requests.
    • fakecloud-s3: Detect s3-control access-point routes and map to CreateAccessPoint/GetAccessPoint/DeleteAccessPoint/ListAccessPoints instead of object actions.
    • fakecloud-aws: Stop re-hashing the body in SigV4 canonicalization; use the signed x-amz-content-sha256 value verbatim. UNSIGNED-PAYLOAD and STREAMING-* markers are signed literally.

Written for commit aac6a56. Summary will update on new commits.

Review in cubic

Three real authorization-correctness gaps found in the 2026-07-15 bug hunt,
each in a path that CI's canonical happy-path never exercised.

- core/dispatch (5.1): an explicit `Deny` in a resource (bucket) policy must
  override a public-read ACL, matching AWS's Deny-overrides precedence. The
  anonymous-access gate collapsed the policy decision to a bool and ORed the
  ACL onto it, so a public-read ACL could grant access the bucket owner had
  explicitly denied. Now an ExplicitDeny short-circuits the ACL grant.

- s3 (5.3): S3 Control access-point management (host `s3-control...`, path
  `/v20180820/accesspoint[/name]`) was authorized under the generic
  method-based object actions (GetObject/PutObject/DeleteObject) instead of
  the dedicated s3:CreateAccessPoint / GetAccessPoint / DeleteAccessPoint /
  ListAccessPoints actions, making access-point IAM policies ineffective.
  `iam_action_for` now detects the s3-control access-point shape first.

- aws/sigv4 (5.4): a concrete `x-amz-content-sha256` payload hash is now
  verified against the actual request body during canonicalization. Without
  this, a captured valid signature could be replayed over a swapped body
  since the canonical request used the signed header hash verbatim.
  `UNSIGNED-PAYLOAD` / `STREAMING-*` markers are signed literally and skip
  the rehash.

Tests: sigv4 body-hash mismatch/match/unsigned-marker unit tests; s3
access-point `iam_action_for` mapping unit test; e2e explicit-Deny-overrides
-public-ACL under `--iam strict`.

Dropped as verified false positives (documented, not deferred):
- 5.5 (deny signed-but-unresolvable AKID under strict): fakecloud
  deliberately tolerates unverified signed creds when `verify_sigv4` is off
  (the dummy-cred UX; the default `AKIAIOSFODNN7EXAMPLE` is itself
  unresolved), and the genuinely-unverifiable case (`verify_sigv4` on)
  already returns InvalidClientTokenId in dispatch. Enforcing it would break
  the documented UX and existing strict-mode tests.
- 5.8 (map GetSecurityTokenServicePreferences to an IAM action): it is a
  fakecloud read-back extension absent from AWS's iam.json; an existing test
  pins it to no IAM action, and adding it would fail the handwritten-test
  audit (no Smithy model to checksum).
@vieiralucas vieiralucas requested a review from Copilot July 16, 2026 00:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…y_sigv4)

The 5.4 body-hash check re-hashed `req.body` inside sigv4 canonicalization and
rejected mismatches with IncompleteSignature. But `req.body` is not reliably the
signed payload (streaming / aws-chunked S3 routes leave it empty), so it
rejected legitimate signed requests: under FAKECLOUD_VERIFY_SIGV4=true the S3
IAM-enforcement e2e tests (s3_get_object_resource_scoped,
s3_request_tag_denies_put_with_wrong_tag,
s3_tag_keys_for_all_values_restricts_allowed_keys) got IncompleteSignature
instead of the expected AccessDenied.

Binding x-amz-content-sha256 to the actual body belongs at the S3 service layer
(XAmzContentSHA256Mismatch, 400), not in sigv4 verification — that is a separate
follow-up. Signature-integrity is still enforced (verify_rejects_tampered_body).
5.1 (explicit-Deny precedence) and 5.3 (access-point action mapping) are
unaffected and remain.
@vieiralucas vieiralucas requested a review from Copilot July 16, 2026 03:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vieiralucas vieiralucas merged commit e43a730 into main Jul 16, 2026
155 checks passed
@vieiralucas vieiralucas deleted the bh-auth branch July 16, 2026 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants