fix(rest/python): verify request signatures per RFC 9421 instead of accepting everything#122
Open
vishkaty wants to merge 7 commits into
Open
Conversation
damaz91
added
the
status:needs-triage
vishkaty
force-pushed
the
feat/rfc9421-signatures
branch
from
July 13, 2026 16:04
7490f89 to
f02d471
Compare
Adds ucp_signing.py: a dependency-light (cryptography-only) implementation of the UCP request-signature layer defined in signatures.md -- RFC 9421 signature base construction, the UCP covered-component set, RFC 9530 Content-Digest over raw body bytes, ES256 with fixed-width raw r||s (never ASN.1/DER, per the spec MUST), Ed25519, and signer key discovery from the UCP-Agent profile. Correctness is anchored to three independent oracles in ucp_signing_test.py: RFC 9421 Appendix B (byte-exact Ed25519 B.2.6, verify-direction ES256 B.2.4) and RFC 9530 digest vectors, an explicit DER-rejection test, and a differential comparison against the independent http-message-signatures library (dev-only). No behavior change yet -- this commit only adds the module and its tests.
…erything The previous verify_signature accepted any request, short-circuited on the literal string "test", read a bespoke Request-Signature header, and its docstring described HMAC -- none of which matches signatures.md, which mandates asymmetric RFC 9421 signatures (ES256 baseline) with key discovery from the UCP-Agent profile. verify_signature now performs real RFC 9421 verification via ucp_signing: Content-Digest, the UCP covered-component coverage gate, raw-r||s ES256 / Ed25519, and signer-key discovery from the UCP-Agent profile's signing_keys. Compatibility is preserved by default: - Two flags (config.py): --require_signatures (default false) and --allow_insecure_profile_urls (default false, for localhost demos/CI). - With enforcement off, signatures are still verified when present and logged, but unsigned/invalid requests are allowed. No profile fetch occurs unless a Signature-Input header is present, so unsigned traffic (including the exact header shape the official conformance suite sends) is unaffected. - The bespoke Request-Signature header is now optional but still accepted. signature_integration_test.py proves both modes end to end against an in-process localhost profile server: permissive passthrough (including an official-conformance replay that asserts no key fetch happens) and, under enforcement, the spec's error code for every failure mode.
The demo client now signs every request per RFC 9421 by default: it generates an ephemeral ES256 key, publishes the public key from a small localhost profile server, and points the UCP-Agent header at it so the merchant can discover the key and verify the signature. Signing is applied centrally via an httpx auth flow, so it covers the exact serialized body bytes. --disable_signatures restores the previous unsigned behaviour (used to keep the exported curl dialog replayable, since a real signature is bound to the body and host).
Server README: a Request Signatures (RFC 9421) section covering what is verified, the --require_signatures / --allow_insecure_profile_urls flags, the spec error codes, and a local sign-then-verify demo recipe. Client README: documents --disable_signatures.
…rifier Keeps the PR at request verification; adds robustness fixes surfaced by an adversarial self-review and drives the verifier to complete test coverage. Verifier fixes (RFC 9421 exactness): - @authority: strip the scheme default port (Section 2.2.3) so a signer that signs `host` still verifies behind an intermediary that presents `host:443`. - @path: empty path normalises to `/` on the verify side too (Section 2.2.6). - covered field values are OWS-trimmed (Section 2.1). - coverage gate no longer requires `signature-agent`: it is a WBA-shape component this default-UCP verifier does not parse, so the promise now matches what it verifies (WBA-shape remains a documented follow-up). Tests: ucp_signing.py to 100% line+branch coverage; positive and negative request-verification cases (Ed25519 through verify_request, body without Content-Digest, malformed digests/JWKs/headers, SSRF resolve paths, key cache, no-profile wiring in both modes). 53 -> 91 tests, green on Python 3.10 and 3.12, ruff clean.
keys[] is the canonical RFC 7517 JWK Set field (ucp#566 removed signing_keys[]), and verifiers resolve keyid only among signature-capable keys. - _extract_keys reads keys[] only; the removed signing_keys[] is not read, so the reference verifier models the merged spec exactly. - verify_request filters to signature-capable keys before the kid match: skip use:"enc" and any key_ops that omits "verify" (RFC 7517 Sections 4.2, 4.3), matching the merged verify pseudocode. Closes the two-verifier divergence noted on #566 (a use:"enc" key with the matching kid must not verify). - the demo client and integration tests now publish keys[] (not the removed signing_keys[]), so the sample teaches the canonical shape. - docstrings/README updated to keys[]. Validated against CURRENT upstream (samples main + python-sdk main): full suite green on Python 3.10 and 3.12, ucp_signing.py at 100% line+branch coverage, ruff clean, enforced-mode happy-path E2E completes.
vishkaty
force-pushed
the
feat/rfc9421-signatures
branch
from
July 15, 2026 16:44
f02d471 to
ec53ea8
Compare
Contributor
Author
|
Rebased onto current main following #131 — the branch now builds and tests against the pip-installed ucp-sdk (0.4.3): 106 integration + 74 unit tests green on the rebased tree. One robustness fix folded in: the signature test base suppressed inherited lifecycle tests via a hard-coded name list, so tests added to integration_test.py after this branch (e.g. #128's discount test) leaked into enforced mode unsigned; the suppression is now programmatic. Ready for review. |
Contributor
Author
|
Understand this is a large review — happy to split it into smaller, individually-mergeable PRs if that's easier to take in stages, e.g.:
It's rebased onto current main and models merged #566 exactly (keys[]-only, |
Class sweep across every inbound route: /mcp was the one entry point without the verify_signature dependency, so unsigned POSTs succeeded even under --require_signatures. Discovery-only today (tools/call is rejected), but bridging tools/call later would have silently opened an unsigned transport into the checkout handlers. Same policy as every other route: verify-if-present by default, so unsigned MCP discovery keeps working in the default mode (covered by a new permissive-mode test); required under --require_signatures (covered by a new enforced-mode test mirroring the webhook one).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #121.
What was wrong
verify_signatureinrest/python/server/dependencies.pyaccepted every request, read a bespokeRequest-Signatureheader that isn't part of the protocol, and its docstring described HMAC-SHA256 — whilesignatures.mdmandates asymmetric RFC 9421 HTTP Message Signatures. A newcomer copying the reference server builds the wrong mechanism entirely (#121 has the full analysis).What changed (4 commits, reviewable independently)
ucp_signing.py+ unit tests — a self-contained RFC 9421/9530 module (only new runtime dep:cryptography): Signature-Input/Signature parsing (RFC 8941 subset), signature-base construction, RFC 9530Content-Digestover raw bytes, ES256 verification in fixed-width rawr||s(a DER-encoded signature is rejected — the encodingsignatures.mdexplicitly forbids), Ed25519, JWK handling, and signer-key discovery from theUCP-Agentprofile'ssigning_keys[](withkeys[]fallback per the in-flight canonicalization in ucp#566), including the HTTPS/no-redirect/special-use-IP guards the spec requires (with an explicit--allow_insecure_profile_urlscarve-out for localhost demos, mirroring the conformance harness's own localhost profile server).verify_signature(same name, same wiring) now actually verifies: parses the headers, enforces the spec's coverage gate (@method/@authority/@path, content-digest+content-type when a body is present, idempotency-key/ucp-agent when those headers are present), checks the digest, resolves the key, verifies the signature, and maps failures to the spec's error codes (401 signature_missing/signature_invalid/key_not_found; 400 digest_mismatch/algorithm_unsupported; 424 profile_unreachable; 422 profile_malformed). Enforcement is opt-in via a new--require_signaturesflag (default off).signing_keys[], and real key discovery end to end.--disable_signaturesrestores the old behavior.Compatibility (designed to change nothing by default)
request-signature: "test", no RFC 9421 headers)Signature-Inputis present. A dedicated test replays the suite's exact header shape.Verification
http-message-signatures==2.0.1(dev-only dependency, droppable on request) — both directions, ES256 and Ed25519.algparameter present (the spec says the algorithm comes from the JWK, not the header), SSRF-guard cases, and profile-fetch failures.--all-files) green; runs green in my fork: https://github.com/vishkaty/samples/actions/runs/28903273501uv.lockdiffs are mechanical (cryptography+ the dev-only test oracle).Scope and follow-ups
Deliberately not in this PR (happy to do as follow-ups in whichever order you prefer): signing outbound webhooks (the spec's "webhooks MUST be signed" —
_notify_webhookcurrently sets no headers), response signing, and extracting the module toward the SDK.Covered after a route sweep: the MCP transport endpoint (
/mcp) now takes the sameverify_signaturedependency as every other inbound route — it was the one entry point accepting unsigned traffic even in enforced mode, and bridgingtools/calllater would have silently opened an unsigned path into the checkout handlers (verify-if-present by default, so unsigned MCP discovery is unaffected; both modes tested). Known scope boundary: the rest/nodejs sample has the same accept-everything gap; this PR is rest/python only, happy to mirror it there as a follow-up.