Skip to content

fix(proxy): classify a presigned SigV4A URL as presigned, not unsigned - #521

Open
MaxFreedomPollard wants to merge 1 commit into
anthropics:mainfrom
MaxFreedomPollard:fix/presigned-sigv4a-policy
Open

fix(proxy): classify a presigned SigV4A URL as presigned, not unsigned#521
MaxFreedomPollard wants to merge 1 commit into
anthropics:mainfrom
MaxFreedomPollard:fix/presigned-sigv4a-policy

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown

Summary

detectSigv4 recognizes a presigned URL by the X-Amz-Algorithm query parameter, but it only accepted AWS4-HMAC-SHA256 (src/sandbox/aws-sigv4.ts:91). A presigned SigV4A URL spells that parameter AWS4-ECDSA-P256-SHA256 and adds X-Amz-Region-Set, so it returned null even when the X-Amz-Credential scope named a masked pair's fake access key id.

createSigv4Planner returns undefined on a null detection, so such a request was forwarded exactly as if no masked credential were involved. That slips past both knobs that should govern it: credentials.sigv4.presigned (the signature is in the URL) and credentials.sigv4.sigv4a (which only ever sees the Authorization-header form). Both default to deny, so the promised behaviour is a 403 naming the case; instead the sandboxed client got an opaque upstream signature failure, because the URL was signed with the placeholder secret. Nothing leaks in either direction, but the policy is not enforced and the failure is not diagnosable.

Fix

Accept either signing algorithm in the query check. The kind stays presigned, which is what the shape means here: the module already documents that the query check "applies whenever the Authorization header does not itself classify", and Sigv4ConfigSchema describes presigned by shape, "X-Amz-Algorithm/X-Amz-Signature in the query ... the signature lives in the URL itself". That is true of a presigned SigV4A URL for exactly the same reason, and neither algorithm can be re-signed without rewriting the URL. The header-Authorization SigV4A case still classifies as sigv4a, and an unrecognized X-Amz-Algorithm is still not a SigV4 signature at all.

Tests

test/sandbox/aws-sigv4.test.ts: a presigned SigV4A URL now classifies as presigned with the access key id from X-Amz-Credential, plus a negative case pinning that an unknown X-Amz-Algorithm alongside X-Amz-Signature and X-Amz-Credential stays undetected.

test/sandbox/credential-aws-pairs.test.ts: the same URL through createSigv4Planner denies by default with credentials.sigv4.presigned named in the reason, and returns undefined under presigned: "passthrough".

Both new assertions fail on main at 66d35e5 (detectSigv4 returns null; the planner returns undefined instead of a deny) and pass with the change. Verified on macOS arm64 with bun test (1364 tests), npx eslint ., npm run typecheck, npm run build, and npx prettier --check. The suite has two failures on this machine that reproduce identically on unmodified main: a 5s timeout in test/sandbox/proxy-deny-violations.test.ts, and mitm-leaf: leaf validity span is <= 99 days, which measures 99.0417 days because clampValidity adds 99 calendar days with Date.setDate and my local timezone leaves DST inside that window (UTC CI runners never see it).

detectSigv4 only accepted AWS4-HMAC-SHA256 as the query-string
X-Amz-Algorithm, so a presigned SigV4A URL (X-Amz-Algorithm
AWS4-ECDSA-P256-SHA256, plus X-Amz-Region-Set) referencing a masked
pair's fake access key id returned null. createSigv4Planner then had
nothing to act on and the request went upstream untouched, escaping
credentials.sigv4.presigned and credentials.sigv4.sigv4a alike --
including their "deny" defaults. The client got an opaque upstream
signature error instead of the 403 the policy promises, and a
"passthrough" setting was never what let it through.

src/sandbox/aws-sigv4.ts:91 now accepts either signing algorithm in the
query check. The shape decides the kind, as the module already documents
for a non-AWS Authorization value alongside presigned params: the
signature is in the URL, so re-signing would rewrite the URL, which is
exactly what "presigned" names. The header-Authorization SigV4A case
still classifies as sigv4a, and an unknown X-Amz-Algorithm is still not
a SigV4 signature.
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.

1 participant