Skip to content

fix(sigv4): use encoded request path for inbound signature verification#176

Merged
alukach merged 1 commit into
mainfrom
fix/sigv4-encoded-key-path
Jul 1, 2026
Merged

fix(sigv4): use encoded request path for inbound signature verification#176
alukach merged 1 commit into
mainfrom
fix/sigv4-encoded-key-path

Conversation

@alukach

@alukach alukach commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Problem

Uploading an object whose key contains a character the client percent-escapes — e.g. a space (foo barfoo%20bar) — fails with 403 SignatureDoesNotMatch:

[WARN] multistore::auth::sigv4: SigV4 signature mismatch
canonical_request=PUT
/alukach/upload-tests/foo bar     <-- literal space
...

Root cause

SigV4's canonical URI is the percent-encoded path the client signed. RequestParts::from_web_sys decodes the request path (correct for bucket/key routing — operation.key() needs the decoded foo bar), and we were passing that same decoded path as the signing_path. So the server rebuilt its canonical request with a literal space while the client (aws-sdk-js) signed %20 → signatures never match.

The outbound leg was already correct: build_backend_url hands the decoded key to url::Url::parse, which re-encodes the space to %20 consistently for both the forwarded request and its signature.

Fix

Recover the raw encoded path from the request URL via http::Uri::path() (which does not decode) and use it as the signing path. Routing/bucket-key parsing keeps the decoded path. Handles all escaped characters, not just spaces; falls back to the decoded path if the URL won't parse.

Tests

  • New characterization test in tests/routing.rs pinning the http behavior the fix relies on (Uri::path() preserves %20).
  • Full native suite + wasm clippy/check pass via the pre-commit hook.

⚠️ Verified locally only (the fetch handler is wasm-only and can't be unit-tested natively). Needs a staging deploy to confirm the real foo bar upload now succeeds.

🤖 Generated with Claude Code

Uploads of keys containing characters the client percent-escapes (e.g. a
space -> %20) failed with SignatureDoesNotMatch. SigV4's canonical URI is
the percent-encoded path the client signed, but `RequestParts` decodes the
path (correct for bucket/key routing) and we were passing that decoded path
as the `signing_path`. The server then built its canonical request with a
literal space while the client signed `%20`, so the signatures never matched.

Recover the raw encoded path from the request URL via `Uri::path()` (which
does not decode) and use it as the signing path. Routing keeps the decoded
path; outbound signing already re-encodes via `url::Url::parse`, so it was
unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @alukach's task in 4m 13s —— View job


Reviewing PR #176

  • Gather context (diff, changed files)
  • Review src/lib.rs changes
  • Review tests/routing.rs changes
  • Post final review

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

🚀 Latest commit deployed to https://source-data-proxy-pr-176.source-coop.workers.dev

  • Date: 2026-07-01T03:05:20Z
  • Commit: 4a82658

@alukach alukach merged commit 56a9520 into main Jul 1, 2026
13 checks passed
@alukach alukach deleted the fix/sigv4-encoded-key-path branch July 1, 2026 04:41
alukach pushed a commit that referenced this pull request Jul 2, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.2.0](v2.1.2...v2.2.0)
(2026-07-01)


### Features

* accept multiple audiences for /.sts token exchange
([#163](#163))
([e911496](e911496))
* **analytics:** log request duration and client IP
([#153](#153))
([cad41b1](cad41b1))
* authorize and enable writes to data connections
([#162](#162))
([85972e8](85972e8))
* make STS max session TTL configurable via env var
([#165](#165))
([39d15f5](39d15f5))
* OIDC provider
([#132](#132))
([5671b64](5671b64))
* per-connection backend authentication via OIDC federation
([#147](#147))
([2f7a12f](2f7a12f))
* **worker:** aggregate live-globe activity by datacenter
([#171](#171))
([c0a3169](c0a3169))


### Bug Fixes

* **deps:** bump quinn-proto to 0.11.15 (RUSTSEC-2026-0185)
([#161](#161))
([189e348](189e348))
* **registry:** sync product model with
source.coop[#284](https://github.com/source-cooperative/data.source.coop/issues/284)
(drop mirror config, use visibility)
([#149](#149))
([8ecf9b4](8ecf9b4))
* return clear 400 for keyless writes instead of misleading sha256 error
([#168](#168))
([f1187f5](f1187f5))
* **sigv4:** use encoded request path for inbound signature verification
([#176](#176))
([56a9520](56a9520))
* **sts:** bound the AssumeRoleWithWebIdentity call with a request
timeout
([#172](#172))
([fa463c7](fa463c7))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: source-release-bot[bot] <265100246+source-release-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant