From c39a59b70034113595e6a848d5d67fc796fe709a Mon Sep 17 00:00:00 2001 From: Nezumi-2711 Date: Tue, 25 Aug 2026 11:09:52 +0700 Subject: [PATCH] fix: aws signature --- docs/authentication.md | 2 +- src/aws-signature.ts | 90 +++++++++++++++++++++++------------------- test/s3.test.ts | 37 +++++++++++++++++ 3 files changed, 88 insertions(+), 41 deletions(-) diff --git a/docs/authentication.md b/docs/authentication.md index 085631b..7563c9f 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -40,7 +40,7 @@ The canonical request is built from: 5. the signed-header list; and 6. `x-amz-content-sha256`, defaulting to `UNSIGNED-PAYLOAD`. -The Worker deliberately canonicalizes a signed `accept-encoding` header to `identity`. Cloudflare can rewrite the received value at the edge; S3 SDKs that sign this header use `identity` for this reason. Browser code must not sign `accept-encoding` because browser networking controls it. +A signed `accept-encoding` header gets special treatment. Cloudflare rewrites the received value at the edge (usually to `gzip, br`), so the delivered header cannot be compared against what the client signed, and clients disagree on the value anyway: aws-sdk-go-v2 (memos and most Go clients) signs `identity` on every operation, while rclone and AWS CLI v2 sign `gzip` for `GetObject` and `identity` elsewhere. The Worker therefore verifies against the pre-rewrite value in `request.cf.clientAcceptEncoding` when the edge supplies it, and otherwise retries the signature against each value a client plausibly signs (the delivered header, `identity`, `gzip`, empty). Only this header's canonical value varies — the signature must still be produced with the secret key. Clients that sign some other value can drop the header from the signature instead (rclone: `--s3-sign-accept-encoding=false`). Browser code must not sign `accept-encoding` because browser networking controls it. Payload hashes are **not** verified. Browser and BFF clients should use `x-amz-content-sha256: UNSIGNED-PAYLOAD`; this is a deliberate streaming limitation, not an integrity guarantee. diff --git a/src/aws-signature.ts b/src/aws-signature.ts index 53cf47e..dc3db17 100644 --- a/src/aws-signature.ts +++ b/src/aws-signature.ts @@ -36,28 +36,43 @@ async function getSigningKey(secret: string, date: string, region: string, servi } /** - * aws-sdk-go-v2 (used by rclone/AWS CLI v2) signs Accept-Encoding as "gzip" specifically for - * GetObject — it wants a compressed transfer of the object body — but as "identity" for every - * other operation (HeadObject, ListObjectsV2, PutObject, multipart list/parts), since those - * don't return arbitrary object data. Confirmed by capturing rclone's own raw outgoing requests: - * HEAD and GET-without-key send "identity"; GET-with-key (GetObject) sends "gzip". Cloudflare's - * edge always rewrites the incoming header before the Worker sees it, so the literal value can - * never be read back — this replicates what the client actually signed instead. + * Clients that sign `Accept-Encoding` don't agree on the value they sign, and the value delivered + * to the Worker is not necessarily the one that was signed: * - * This is a best-effort fallback: some proxies between the client and this Worker (including - * Cloudflare's own edge) can still rewrite Accept-Encoding in ways clients don't anticipate, - * which is why rclone/aws-sdk-go-v2 also expose `--s3-sign-accept-encoding=false` to drop this - * header from what's signed entirely — see the "Accept-Encoding" note in the README. + * - aws-sdk-go-v2 (memos and most Go S3 clients) sets and signs "identity" on every operation, + * GetObject included, because it disables the transport's automatic gzip handling. + * - rclone / AWS CLI v2 sign "gzip" for GetObject when gzip transfer is enabled, "identity" for + * everything else. Confirmed by capturing rclone's own raw outgoing requests. + * - Browsers and BFF clients sign whatever they actually sent, if they sign the header at all. + * + * Cloudflare's edge rewrites the incoming Accept-Encoding before the Worker sees it (typically to + * "gzip, br"), so the delivered header cannot be compared against what was signed. `request.cf` + * exposes the pre-rewrite value as `clientAcceptEncoding` whenever the edge changed it; where that + * is missing (local dev, or no rewrite happened) we fall back to trying each value a real client + * plausibly signs. Trying several candidates only varies this one header's canonical value — the + * signature must still be produced with the secret key — so it costs a few extra HMACs rather than + * any authentication strength. Clients that still can't be matched can drop the header from the + * signature entirely (rclone: `--s3-sign-accept-encoding=false`); see docs/authentication.md. */ -function isGetObjectRequest(method: string, url: URL): boolean { - if (method !== "GET") return false; - if (url.searchParams.has("uploadId") || url.searchParams.has("uploads")) return false; - return url.pathname.split("/").filter(Boolean).length > 1; +function acceptEncodingCandidates(request: Request): string[] { + const raw = [(request.cf as IncomingRequestCfProperties | undefined)?.clientAcceptEncoding, request.headers.get("accept-encoding") ?? undefined, "identity", "gzip", ""]; + + const candidates: string[] = []; + for (const value of raw) { + if (value === undefined) continue; + const trimmed = value.trim(); + if (!candidates.includes(trimmed)) candidates.push(trimmed); + } + return candidates; } -async function createCanonicalRequest(request: Request, isQueryAuth: boolean): Promise { - const url = new URL(request.url); +function getSignedHeadersList(request: Request, url: URL, isQueryAuth: boolean): string[] { + if (isQueryAuth) return (url.searchParams.get("X-Amz-SignedHeaders") ?? "host").split(";"); + const match = (request.headers.get("Authorization") ?? "").match(/SignedHeaders=([^,\s]+)/); + return match ? match[1].split(";") : ["host"]; +} +function createCanonicalRequest(request: Request, url: URL, signedHeadersList: string[], acceptEncoding: string): string { const method = request.method; const canonicalUri = url.pathname || "/"; @@ -72,15 +87,6 @@ async function createCanonicalRequest(request: Request, isQueryAuth: boolean): P .map(([key, val]) => `${encodeRFC3986(key)}=${encodeRFC3986(val)}`) .join("&"); - let signedHeadersList: string[]; - if (isQueryAuth) { - signedHeadersList = (url.searchParams.get("X-Amz-SignedHeaders") ?? "host").split(";"); - } else { - const authHeader = request.headers.get("Authorization") ?? ""; - const match = authHeader.match(/SignedHeaders=([^,\s]+)/); - signedHeadersList = match ? match[1].split(";") : ["host"]; - } - const canonicalHeaders = signedHeadersList .map((h) => { const headerName = h.toLowerCase(); @@ -93,7 +99,7 @@ async function createCanonicalRequest(request: Request, isQueryAuth: boolean): P headerValue += `:${port}`; } } else if (headerName === "accept-encoding") { - headerValue = isGetObjectRequest(method, url) ? "gzip" : "identity"; + headerValue = acceptEncoding; } else { headerValue = request.headers.get(headerName)?.trim() ?? ""; } @@ -180,24 +186,28 @@ export async function verifySignature(request: Request, env: Env): Promise h.toLowerCase() === "accept-encoding") ? acceptEncodingCandidates(request) : [""]; + + for (const acceptEncoding of candidates) { + const canonicalRequest = createCanonicalRequest(request, url, signedHeadersList, acceptEncoding); + const hashedCanonicalRequest = await sha256(canonicalRequest); + const stringToSign = ["AWS4-HMAC-SHA256", datetime, credentialScope, hashedCanonicalRequest].join("\n"); + const signatureHex = bufToHex(await hmacSha256(signingKey, stringToSign)); + if (constantTimeEqual(signatureHex, expectedSignature)) return { ok: true }; + } + + return signatureMismatch(); } diff --git a/test/s3.test.ts b/test/s3.test.ts index a64989a..cc97fec 100644 --- a/test/s3.test.ts +++ b/test/s3.test.ts @@ -116,6 +116,43 @@ describe("S3 compatibility", () => { expect(await response.text()).toBe("hello"); }); + it("verifies GetObject signatures with Accept-Encoding: identity (aws-sdk-go-v2 signs identity on every operation)", async () => { + await worker.fetch(await signed("/test-bucket/ae-get-identity.txt", { method: "PUT", body: "hello", headers: { "accept-encoding": "identity" } }), ENV, CTX); + const original = await signed("/test-bucket/ae-get-identity.txt?x-id=GetObject", { method: "GET", headers: { "accept-encoding": "identity" } }); + const rewrittenHeaders = new Headers(original.headers); + rewrittenHeaders.set("accept-encoding", "gzip, br"); + const mutated = new Request(original, { headers: rewrittenHeaders }); + + const response = await worker.fetch(mutated, ENV, CTX); + expect(response.status).toBe(200); + expect(await response.text()).toBe("hello"); + }); + + it("verifies GetObject signatures against an unusual Accept-Encoding recovered from request.cf.clientAcceptEncoding", async () => { + await worker.fetch(await signed("/test-bucket/ae-get-cf.txt", { method: "PUT", body: "hello" }), ENV, CTX); + const original = await signed("/test-bucket/ae-get-cf.txt", { method: "GET", headers: { "accept-encoding": "deflate" } }); + const rewrittenHeaders = new Headers(original.headers); + rewrittenHeaders.set("accept-encoding", "gzip, br"); + const mutated = new Request(original, { headers: rewrittenHeaders, cf: { clientAcceptEncoding: "deflate" } } as RequestInit); + + const response = await worker.fetch(mutated, ENV, CTX); + expect(response.status).toBe(200); + expect(await response.text()).toBe("hello"); + }); + + it("still rejects a GET whose signature matches no plausible Accept-Encoding value", async () => { + await worker.fetch(await signed("/test-bucket/ae-get-bad.txt", { method: "PUT", body: "hello" }), ENV, CTX); + const original = await signed("/test-bucket/ae-get-bad.txt", { method: "GET", headers: { "accept-encoding": "identity" } }); + const tampered = new Headers(original.headers); + tampered.set("accept-encoding", "gzip, br"); + tampered.set("x-amz-content-sha256", "UNSIGNED-PAYLOAD-TAMPERED"); + const mutated = new Request(original, { headers: tampered }); + + const response = await worker.fetch(mutated, ENV, CTX); + expect(response.status).toBe(403); + expect(await response.text()).toContain("SignatureDoesNotMatch"); + }); + it("verifies signatures for requests carrying the aws-sdk-go x-id tracing param (rclone/AWS CLI v2 GetObject)", async () => { // aws-sdk-go-v2 (used by rclone) signs the x-id param as part of the request by default // (opt.UseXID defaults to true) — it's part of the canonical query string, not appended