feat(signing): SigningProvider Protocol for KMS-backed signing (#283)#323
Merged
feat(signing): SigningProvider Protocol for KMS-backed signing (#283)#323
Conversation
Decouples the RFC 9421 request-signing profile from in-process key storage so KMS / HSM / Vault deployments can plug in without forking the signer or pulling private material out of the managed store at boot. * `SigningProvider` Protocol — async `sign(signature_base) -> bytes`, `key_id() -> str`, `algorithm() -> Literal["ed25519","ecdsa-p256-sha256"]`. Contract docstring covers lazy init, public-key type-checking, rotation tripwire, key separation by `adcp_use`, and fingerprint redaction. * `InMemorySigningProvider` — default adapter; validates Ed25519 vs. EC, EC curve = SECP256R1, and key_id non-empty at construction. * `async_sign_request(provider=...)` — async entry point sharing the canonicalization spine (`_prepare_signature`) with the sync `sign_request`, so both paths produce byte-identical Signature-Input. * `pem_to_adcp_jwk(pem, *, kid, purpose, password=None)` — derives the public JWK for KMS adopters whose private half never leaves the managed store. Accepts both PKCS#8 private and SPKI public PEMs. * RFC 8941 §3.3.3 escaping fix at `signer.py:_escape_sf_string` — applies to `keyid`, `nonce`, `tag`. Rejects characters outside printable ASCII 0x20-0x7E to close header-injection / parser- divergence vectors at non-httpx integrators. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `label` kwarg on `sign_request` / `async_sign_request` is a public input that lands unquoted in both the `Signature-Input` and `Signature` headers. Without validation, a CRLF or other non-token character in the label would inject extra header bytes (at non-httpx integrators that don't sanitize embedded line terminators) or produce a label that conformant verifiers parse differently from this serializer. Add `_validate_sf_key()` enforcing the RFC 8941 §3.1.2 token grammar: must start with `[a-z*]`, then `[a-z0-9_\-.*]`. Applied at the entry of `_prepare_signature` so both sync and async signers are covered. Same parser-divergence / header-injection class as the `keyid`/`nonce`/`tag` escaping fix already in this PR — just for the remaining unguarded input. Also adds the SPKI-public-key RSA rejection test for `pem_to_adcp_jwk` that was missing from the previous coverage round. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7dfa75a to
c62a422
Compare
bokelley
added a commit
that referenced
this pull request
Apr 30, 2026
Brings in main's recent work since the foundation branch forked: - 046b15e / d92cfb1 — PR #321 + #322 storyboard fixes (the same fixes my earlier cherry-pick commit 597d00a brought in; main's version supersedes since it includes additional fixture-dependent failure closures from #322). - 4de648e — SigningProvider Protocol (PR #283 / #323). - d3e1a0f — seller_agent PRODUCTS schema regression test. - ce4c5df — handler-authoring docs expansion (salesagent migration patterns). - e37930a — release 4.2.0. - ce7e9ab — seed_creative_format scenario + force_session_status. - 9244d46 — comply_test_controller dict return fix. Conflicts resolved: - examples/seller_agent.py — accepted main's version. Main has the PR #321 + #322 fixes plus storyboard fixture products (outdoor_display_q2, outdoor_video_q2, sports_preroll_q2, etc.) added directly to the PRODUCTS list. Foundation's cherry-pick commit 597d00a brought in only PR #321; main has the more complete state. - tests/test_seller_agent_storyboard.py — accepted main's version (same rationale: more comprehensive coverage from #321 + #322 combined). Merged state verified: - mypy clean across src/adcp/decisioning/ (13 files) - ruff clean across src/adcp/decisioning/ - Full suite: 2588 passed, 17 skipped, 1 xfailed (+30 from main's added tests) Foundation PR #316 is now up-to-date with main and CI should clear on the next run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 30, 2026
Brings in main's recent work since the foundation branch forked: - 046b15e / d92cfb1 — PR #321 + #322 storyboard fixes (the same fixes my earlier cherry-pick commit 597d00a brought in; main's version supersedes since it includes additional fixture-dependent failure closures from #322). - 4de648e — SigningProvider Protocol (PR #283 / #323). - d3e1a0f — seller_agent PRODUCTS schema regression test. - ce4c5df — handler-authoring docs expansion. - e37930a — release 4.2.0. - ce7e9ab — seed_creative_format scenario + force_session_status. - 9244d46 — comply_test_controller dict return fix. Conflicts resolved by accepting main's version for examples/seller_agent.py and tests/test_seller_agent_storyboard.py — main has the PR #321 + #322 combined state, more complete than the foundation's earlier cherry-pick. Merged state: mypy clean, ruff clean, full suite 2588 passed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Closes #283.
Summary
Adds a
SigningProviderProtocol so the RFC 9421 request-signing profile no longer requires the private key to live in process memory. Operators using AWS KMS / GCP KMS / Azure Key Vault / Vault Transit can implement the Protocol against their managed key store without forking the signer.The 2026-04-28 retriage on #283 green-lit this exact scope: Protocol +
InMemorySigningProvider+async_sign_request()+pem_to_adcp_jwk+ the RFC 8941key_idescaping fix atsigner.py:105.What's added
SigningProviderProtocol (src/adcp/signing/provider.py) —async sign(signature_base: bytes) -> bytes,key_id() -> str,algorithm() -> Literal["ed25519", "ecdsa-p256-sha256"]. Contract docstring covers lazy init, public-key type-checking, rotation tripwire, key separation byadcp_use, and fingerprint-logging redaction.InMemorySigningProvider— default adapter wrapping the existing in-process path. Validates Ed25519 vs. EC, EC curve = SECP256R1, andkey_idnon-empty at construction (closes the constructor-time alg/key/curve binding gap flagged in original triage and reaffirmed by security-reviewer this round).async_sign_request(provider=...)— async entry point. Shares_prepare_signaturewith the sync path so both produce byte-identicalSignature-Input. Syncsign_requestis unchanged.pem_to_adcp_jwk(pem, *, kid, purpose, password=None)— derives the public JWK from a PEM. Accepts PKCS#8 private OR SPKI public PEMs (the latter is what KMS adopters typically have, since the private half never leaves the managed store). Output is byte-shape-identical togenerate_signing_keypair's public half, so the spec-requiredadcp_useclaim and JWAalgvalue are correct by construction.signer.py:_escape_sf_string) — applied tokeyid,nonce,tagbefore interpolation intoSignature-Input. Rejects anything outside printable ASCII 0x20-0x7E, including CRLF (HTTP-header-injection vector at non-httpx integrators) and lookalike Unicode quotes (parser-divergence vector against Go/JS verifiers).Reviewer-flagged items addressed
This PR went through code-reviewer, ad-tech-protocol-expert, and security-reviewer. Their feedback is incorporated:
test_sync_and_async_signature_input_identical_for_es256(signature value skipped due to random k).payload→signature_baseon Protocol-----BEGIN PUBLIC KEY-----instead of substring_escape_sf_stringmust reject characters outside §3.3.3 grammarValueErroron non-printable-ASCII; tests cover CRLF, NULL, and Unicode quote.InMemorySigningProviderTest plan
pytest tests/conformance/signing/— 375 passed, 6 skippedtest_signing_provider.pycovering: round-trip async sign+verify (ed25519 + es256), sync/async byte-identity (both algs), RFC 8941 escape with"and\, control-char + non-ASCII rejection,pem_to_adcp_jwkshape parity for both algs, SPKI public-PEM acceptance, RSA rejection, alg/key mismatch and wrong-curve rejectionruff check— clean on changed filesmypy src/adcp/signing/— cleanblack --check— clean on changed filesNotes
jwks_uri.🤖 Generated with Claude Code