Skip to content

R2: sign the canonical request SigV4 actually specifies - #818

Merged
anupsv merged 1 commit into
mainfrom
fix/r2-sigv4-canonical-request
Jul 31, 2026
Merged

R2: sign the canonical request SigV4 actually specifies#818
anupsv merged 1 commit into
mainfrom
fix/r2-sigv4-canonical-request

Conversation

@anupsv

@anupsv anupsv commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

What

The DFlash gates-only dry run keeps dying at step 36, "Prepare hidden correctness golden", with HTTP 403 SignatureDoesNotMatch. #817 fixed a real blank-signature 400 underneath this; the signature is now well-formed, and R2 still refuses it — because we sign a malformed canonical request.

SigV4 is

METHOD \n URI \n QUERY \n CanonicalHeaders \n SignedHeaders \n PayloadHash

and CanonicalHeaders is itself name:value\n per header, so there is a blank line between the last header and SignedHeaders. Both scripts spelled that terminating newline inside canonical_headers' own printf:

canonical_headers="$(printf 'host:%s\n...\nx-amz-date:%s\n' ...)"   # <- trailing \n
canonical_request="$(printf 'GET\n%s\n\n%s\n%s\n%s' ...)"           # <- only ONE \n after headers

$(...) strips every trailing newline, so canonical_headers arrives without its terminator and the canonical request goes on the wire one line short — 8 lines instead of 9. R2 hashes the 9-line form for the same request, the hashes disagree, 403.

That error reads like a bucket-permission fault and is not one. R2 returned SignatureDoesNotMatch, not InvalidAccessKeyId, not AccessDenied, not NoSuchBucket — it resolved the access key, the bucket and the key, and only disagreed about the signature.

Fix

Move both newlines into the canonical_request format string, where nothing can strip them: one terminates the last header line, one is the blank separator. Identical latent defect and identical fix in the upload-r2-object.sh (PUT) twin.

Why this survived to be found twice

It has never run anywhere. download_with_aws_cli() short-circuits the signer whenever aws is on the runner PATH, and the serial box has it:

run box announced
prod serial 30589381761 (SUCCESS) m5-bench using AWS CLI S3 path-style download ×3
dev serial 30168626867 (SUCCESS) m5-bench using AWS CLI S3 path-style download ×3
dev DFlash 30591193483 (403) m5-laguna-dflash using signed HTTPS download

M5-C's runner PATH is /usr/bin:/bin:/usr/sbin:/sbin (aws exists at /opt/homebrew/bin/aws but is off PATH), so it is the first box to execute this signer at all — and it has now surfaced two latent bugs in a row from the same unexercised path.

Consequently this PR also corrects #817's comment claiming "the serial box worked only because OpenSSL 3 was first on its PATH." That is false — the serial box never runs openssl in this script — and believing it sends the next debugger to audit PATH ordering on a box that does not run the code.

Ruled out

  • Wrong bucket / path-style vs virtual-host — the signed path and the aws-cli fallback derive the same path-style URL; the canonical request signs the same path that goes on the wire.
  • Wrong key prefixMLXFAST_CORRECTNESS_GOLDEN_R2_PATH and MLXFAST_GPQA_R2_PATH are byte-identical to serial's in both repos.
  • Credentials / wrong account — the dev repo's own serial workflow has successfully fetched these exact objects with these exact secrets.

The guard that missed it

theScriptSigningChainReproducesThePinnedSignature signs a string-to-sign ending in the literal deadbeef — a stand-in for the canonical-request hash. It proves the HMAC chain and says nothing about the canonical request being hashed. That is exactly how #817 shipped a correct signer over a malformed input.

New test theCanonicalRequestMatchesAnIndependentSigV4Implementation extracts the real canonical_headers/canonical_request assignments out of each shipped script (so it tracks the script, not a copy), evaluates them under pinned inputs, and asserts:

  1. the canonical request is exactly 9 lines,
  2. line 3 is the empty query and line 7 is the blank separator — the byte that was missing,
  3. its sha256 equals botocore's own CanonicalRequest hash for the identical request.

Verification

RED — revert either script to the pre-fix construction:

✘ .github/scripts/download-r2-object.sh canonical request has 8 lines, expected 9
  Expectation failed: (lines.count → 8) == 9
✘ .github/scripts/download-r2-object.sh canonical-request hash
  5a4af0b9…, expected 18ec091e… (botocore's own CanonicalRequest for the same GET request)
✘ .github/scripts/upload-r2-object.sh canonical request has 8 lines, expected 9
✘ .github/scripts/upload-r2-object.sh canonical-request hash
  6a643959…, expected 48c8f7da…

GREEN — with the fix: ✔ Test run with 3 tests in 1 suite passed.

Independently corroborated rather than taken on faith:

  • A from-scratch Python SigV4 canonical-request builder written from the AWS spec reproduces both pinned hashes (18ec091e…, 48c8f7da…) exactly.
  • End to end, the full Authorization header produced by the real script's own signing block — path derivation, canonical request, string-to-sign, 4-step key derivation, final signature — byte-matches that independent implementation (Signature=01223ee7…, public AWS-docs example credentials only).
  • swift test531 tests in 23 suites pass (530 + 1 new).
  • shellcheck -S warning clean on both scripts.

Not changed, deliberately

The head -c 400 truncation of the R2 error body. R2's SignatureDoesNotMatch document can echo the CanonicalRequest, which contains the host and bucket from the R2_BUCKET_ENDPOINT secret. GitHub masks whole secret values, not substrings, so widening it would leak the endpoint into the run log.

Follow-up, not blocking

curl --retry 5 --retry-all-errors retried a non-retryable 403 six times (23:45:39 → 23:45:50 in the real log). Cosmetic; worth not retrying 4xx.

No secret value was printed, logged, or transmitted. All signing tests use the public AWS-docs example credential pair. No hidden prompt, golden, or GPQA answer was authored, read, or hashed.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.

The DFlash gates-only dry run dies at "Prepare hidden correctness golden"
with HTTP 403 SignatureDoesNotMatch. #817 fixed a real blank-signature 400
underneath this and uncovered the next bug: the signature is well-formed,
and R2 is rejecting it because we sign a MALFORMED canonical request.

SigV4 is

    METHOD \n URI \n QUERY \n CanonicalHeaders \n SignedHeaders \n PayloadHash

and CanonicalHeaders is itself "name:value\n" per header, so a BLANK LINE
separates the last header from SignedHeaders. Both scripts spelled that
terminating newline inside canonical_headers' own printf:

    canonical_headers="$(printf 'host:%s\n...\nx-amz-date:%s\n' ...)"
    canonical_request="$(printf 'GET\n%s\n\n%s\n%s\n%s' ...)"

Command substitution strips every trailing newline, so canonical_headers
arrived without its terminator and the canonical request went on the wire
one line short -- 8 lines instead of 9. R2 hashes the 9-line form for the
same request, the hashes disagree, and it answers 403. That reads like a
bucket-permission fault and is not one: R2 resolved the access key, the
bucket and the key, and only disagreed about the signature.

Move both newlines into the canonical_request format string, where nothing
can strip them: one terminates the last header line, one is the blank
separator. Same defect and same fix in the upload twin.

Why this survived: it has never run anywhere. download_with_aws_cli()
short-circuits the signer whenever `aws` is on the runner PATH, and the
serial box has it -- every successful hidden-golden fetch in either repo
announced "using AWS CLI S3 path-style download", never "using signed HTTPS
download". M5-C's runner PATH is /usr/bin:/bin:/usr/sbin:/sbin, so it is the
first box to execute this code at all, and it has now surfaced two latent
bugs in a row from the same unexercised path.

So also correct #817's comment claiming "the serial box worked only because
OpenSSL 3 was first on its PATH." That is false -- the serial box never runs
openssl in this script -- and believing it sends the next debugger to audit
PATH ordering on a box that does not run the code.

The existing guard could not catch this. It signs a string-to-sign ending in
the literal `deadbeef`, a stand-in for the canonical-request hash, so it
proves the HMAC chain and says nothing about the canonical request being
hashed -- which is how #817 shipped a correct signer over a malformed input.
Add theCanonicalRequestMatchesAnIndependentSigV4Implementation: it extracts
the real canonical_headers/canonical_request assignments out of each shipped
script, evaluates them under pinned inputs, and asserts the request is 9
lines, that line 7 is the blank separator, and that its sha256 equals
botocore's own CanonicalRequest hash for the identical request.

Verified: reverting either script to the pre-fix construction fails the new
test with lines.count -> 8 and the exact hashes 5a4af0b9 (GET) / 6a643959
(PUT) against the expected 18ec091e / 48c8f7da. With the fix, the full
Authorization header produced by the real script's own signing block
byte-matches an independent Python SigV4 implementation. swift test: 531
tests in 23 suites pass. shellcheck clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@anupsv
anupsv requested a review from a team July 31, 2026 00:25
@anupsv
anupsv had a problem deploying to benchmark-private-prompts-v2 July 31, 2026 00:46 — with GitHub Actions Failure
@anupsv
anupsv merged commit cfc644b into main Jul 31, 2026
2 of 3 checks passed
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