From c6d5505c8d6025961f0bd9d6af1041102b4bee82 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 20 Apr 2026 13:57:30 -0400 Subject: [PATCH 1/2] feat(signing): generate_signing_keypair() programmatic API (closes #217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-8 webhooks-9421 agent flagged that adcp-keygen on PATH isn't reliable from CI / subprocess contexts where the venv's bin dir isn't on PATH (spawning .venv/bin/python directly raised FileNotFoundError). Callers worked around with Path(sys.executable).parent / "adcp-keygen" — fine, but shell-out is the wrong shape for tests and provisioning code already running in the Python process. Add generate_signing_keypair(*, alg, kid, purpose, passphrase) → (pem_bytes, public_jwk). Single entry point that dispatches to the existing generate_ed25519 / generate_es256 helpers after resolving the default kid (adcp-{alg}-YYYYMMDD). main() in the CLI now delegates to the same helper — zero duplication between CLI and programmatic paths. Fails loud on unsupported alg / purpose. Re-exported from adcp.signing. - 18 unit tests: PEM loads via cryptography, JWK public matches PEM public via DER round-trip, default + explicit purpose, invalid inputs raise, encrypted PEM passphrase round-trip, JWK shape per alg (no private scalar leaks), ed25519 + es256 sign+verify against the produced JWK, WebhookSender.from_pem accepts the PEM, kid default format, top-level re-export. - Docs quickstart block in src/adcp/signing/__init__.py showing programmatic + CLI equivalence. Existing keygen tests keep passing — the CLI is now a thin wrapper around the same spine. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adcp/signing/__init__.py | 22 ++ src/adcp/signing/keygen.py | 103 ++++++- .../signing/test_keygen_programmatic.py | 281 ++++++++++++++++++ 3 files changed, 394 insertions(+), 12 deletions(-) create mode 100644 tests/conformance/signing/test_keygen_programmatic.py diff --git a/src/adcp/signing/__init__.py b/src/adcp/signing/__init__.py index 70ef2bf5..b496e2c2 100644 --- a/src/adcp/signing/__init__.py +++ b/src/adcp/signing/__init__.py @@ -17,6 +17,26 @@ * :class:`SigningConfig` — bundle key material for auto-signing via ``ADCPClient(signing=...)`` +**Provisioning** (new keypairs): + +* :func:`generate_signing_keypair` — programmatic counterpart to the + ``adcp-keygen`` CLI. Returns ``(pem_bytes, public_jwk)`` so tests, + provisioning scripts, and any non-shell context can mint keys + without spawning a subprocess. Both paths share the same spine — a + PEM generated here is indistinguishable from one the CLI wrote. + + .. code-block:: python + + from adcp.signing import generate_signing_keypair + + # CLI equivalence: + # adcp-keygen --alg ed25519 --purpose webhook-signing + pem, public_jwk = generate_signing_keypair( + alg="ed25519", purpose="webhook-signing" + ) + Path("webhook-key.pem").write_bytes(pem) + publish_to_jwks_uri(public_jwk) + **Sellers** (verifying incoming requests): * :func:`verify_starlette_request` / :func:`verify_flask_request` — @@ -145,6 +165,7 @@ verify_detached_jws, verify_jws_document, ) +from adcp.signing.keygen import generate_signing_keypair from adcp.signing.middleware import ( unauthorized_response_headers, verify_flask_request, @@ -286,6 +307,7 @@ def __init__(self, *args: object, **kwargs: object) -> None: "default_revocation_list_fetcher", "extract_signature_bytes", "format_signature_header", + "generate_signing_keypair", "load_private_key_pem", "operation_needs_signing", "parse_signature_input_header", diff --git a/src/adcp/signing/keygen.py b/src/adcp/signing/keygen.py index a00fa34a..d1b62e89 100644 --- a/src/adcp/signing/keygen.py +++ b/src/adcp/signing/keygen.py @@ -1,13 +1,32 @@ """Key generation helpers for AdCP request signing. -Writes a PEM private key and prints the matching JWK (public half) with -`adcp_use: "request-signing"` — paste that JWK into your agent's JWKS at the -URL advertised in brand.json. +Two equivalent entry points — pick whichever fits your calling context: -Usage: +- :func:`generate_signing_keypair` — programmatic API returning + ``(pem_bytes, public_jwk)``. Use from tests, provisioning scripts, and + any non-shell code where spawning a subprocess is the wrong shape. +- ``adcp-keygen`` CLI (:func:`main`) — wraps the same helper plus + file-writing and stdout printing, so CI / shell pipelines stay + ergonomic. + +Both paths produce the same PEM and JWK. Publish the JWK (public half) +at your agent's ``jwks_uri`` — the ``adcp_use`` claim (``request-signing`` +or ``webhook-signing``) pins the key to one signing profile; verifiers +reject keys used in the wrong surface. + +Usage (CLI): adcp-keygen --alg ed25519 --out private-key.pem adcp-keygen --alg es256 --out private-key.pem adcp-keygen --alg ed25519 --encrypt --out private-key.pem + +Usage (programmatic): + from adcp.signing import generate_signing_keypair + + pem_bytes, public_jwk = generate_signing_keypair( + alg="ed25519", purpose="webhook-signing" + ) + Path("key.pem").write_bytes(pem_bytes) + publish_to_jwks_uri(public_jwk) """ from __future__ import annotations @@ -20,7 +39,7 @@ import unicodedata from datetime import datetime, timezone from pathlib import Path -from typing import Any +from typing import Any, Literal from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import ec, ed25519 @@ -90,6 +109,66 @@ def generate_es256( return pem, jwk +def _default_kid(alg: str) -> str: + """Default ``kid`` derived from alg + UTC date. Callers who care + about key rotation should pass an explicit ``kid``.""" + return f"adcp-{alg}-{datetime.now(timezone.utc).strftime('%Y%m%d')}" + + +def generate_signing_keypair( + *, + alg: Literal["ed25519", "es256"] = "ed25519", + kid: str | None = None, + purpose: Literal["request-signing", "webhook-signing"] = "request-signing", + passphrase: bytes | None = None, +) -> tuple[bytes, dict[str, Any]]: + """Generate an AdCP signing keypair. + + Programmatic companion to the ``adcp-keygen`` CLI — call this from + tests, provisioning scripts, or any non-shell context where spawning + a subprocess is the wrong shape. + + :param alg: Signature algorithm. ``"ed25519"`` (default; tiny keys, + recommended) or ``"es256"`` (ECDSA over P-256, broader ecosystem + support). + :param kid: Key ID to embed in the JWK. Defaults to + ``"adcp-{alg}-{YYYYMMDD}"`` — fine for one-off keys, but callers + managing rotation should supply an explicit kid so rotated keys + don't collide. + :param purpose: Which AdCP signing profile this key is for. Sets the + JWK ``adcp_use`` claim. **Request-signing and webhook-signing + keys MUST be distinct** — a signature from one surface cannot + replay as the other, and every conformant verifier enforces the + claim. Generate separate keys per purpose. + :param passphrase: When provided, the PEM is encrypted with + ``BestAvailableEncryption``. Typical only for dev-laptop keys; + automated deployments usually leave the PEM unencrypted and + rely on filesystem perms (the CLI writes mode 0600). + + :returns: ``(pem_bytes, public_jwk)``. The PEM is PKCS#8 + (optionally encrypted); the JWK is the public half, ready to + publish at your ``jwks_uri``. The private scalar is NOT in the + JWK — only in the PEM. + + :raises ValueError: ``alg`` or ``purpose`` is unsupported. + + Example: + >>> pem, jwk = generate_signing_keypair( + ... alg="ed25519", purpose="webhook-signing" + ... ) + >>> jwk["adcp_use"] + 'webhook-signing' + """ + if alg not in ("ed25519", "es256"): + raise ValueError(f"alg must be 'ed25519' or 'es256', got {alg!r}") + if purpose not in _ADCP_USE_VALUES: + raise ValueError(f"purpose must be one of {_ADCP_USE_VALUES}, got {purpose!r}") + resolved_kid = kid or _default_kid(alg) + if alg == "ed25519": + return generate_ed25519(resolved_kid, passphrase=passphrase, adcp_use=purpose) + return generate_es256(resolved_kid, passphrase=passphrase, adcp_use=purpose) + + def _prompt_passphrase_bytes() -> bytes: first = getpass.getpass("Passphrase: ") if not first: @@ -164,13 +243,13 @@ def main(argv: list[str] | None = None) -> int: passphrase = _prompt_passphrase_bytes() if args.encrypt else None - kid = args.kid or f"adcp-{args.alg}-{datetime.now(timezone.utc).strftime('%Y%m%d')}" - if args.alg == "ed25519": - pem, jwk = generate_ed25519(kid, passphrase=passphrase, adcp_use=args.purpose) - alg_rfc = ALG_ED25519 - else: - pem, jwk = generate_es256(kid, passphrase=passphrase, adcp_use=args.purpose) - alg_rfc = ALG_ES256 + pem, jwk = generate_signing_keypair( + alg=args.alg, + kid=args.kid, + purpose=args.purpose, + passphrase=passphrase, + ) + alg_rfc = ALG_ED25519 if args.alg == "ed25519" else ALG_ES256 # `--force` clobbers in two steps (non-atomic on overwrite), but the # happy-path create is atomic via O_EXCL | mode=0o600 so there is no window diff --git a/tests/conformance/signing/test_keygen_programmatic.py b/tests/conformance/signing/test_keygen_programmatic.py new file mode 100644 index 00000000..61651e9a --- /dev/null +++ b/tests/conformance/signing/test_keygen_programmatic.py @@ -0,0 +1,281 @@ +"""``generate_signing_keypair`` — programmatic API (#217). + +Round-8 webhooks-9421 agent flagged that ``adcp-keygen`` on PATH isn't +reliable from CI / subprocess contexts where the venv's bin dir isn't +on ``PATH`` (spawning ``.venv/bin/python`` directly). Callers worked +around it with ``Path(sys.executable).parent / "adcp-keygen"`` — fine, +but shell-out is the wrong shape for tests and provisioning code +that already live in the Python process. + +This test locks the programmatic API contract: a single entry point +that returns ``(pem_bytes, public_jwk)``, composed with the existing +signing + webhook verification helpers. +""" + +from __future__ import annotations + +import pytest +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import ec, ed25519 + +from adcp.signing import generate_signing_keypair, public_key_from_jwk + +# --------------------------------------------------------------------------- +# Core contract — returns (pem_bytes, public_jwk) +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("alg", ["ed25519", "es256"]) +def test_pem_loads_via_cryptography(alg: str) -> None: + """The returned PEM must be PKCS#8 and load via cryptography's + ``load_pem_private_key`` — the lowest-level guarantee the API + provides. A PEM that won't load is useless regardless of what + JWK we built alongside it.""" + pem, _ = generate_signing_keypair(alg=alg, kid="test-kid") + + private_key = serialization.load_pem_private_key(pem, password=None) + + if alg == "ed25519": + assert isinstance(private_key, ed25519.Ed25519PrivateKey) + else: + assert isinstance(private_key, ec.EllipticCurvePrivateKey) + + +@pytest.mark.parametrize("alg", ["ed25519", "es256"]) +def test_jwk_matches_pem_public_key(alg: str) -> None: + """The public JWK must describe the PEM's public half. If a caller + publishes the JWK at ``jwks_uri`` and signs with the PEM, signatures + verify — that's the round-trip the signing flow depends on.""" + pem, jwk = generate_signing_keypair(alg=alg, kid="test-kid") + + # Reconstruct public keys from both sides; compare their raw bytes. + private_key = serialization.load_pem_private_key(pem, password=None) + from_pem_public = private_key.public_key() + from_jwk_public = public_key_from_jwk(jwk) + + # The cryptography library exposes equality for public keys via + # public_bytes comparison. + from_pem_bytes = from_pem_public.public_bytes( + encoding=serialization.Encoding.DER, + format=serialization.PublicFormat.SubjectPublicKeyInfo, + ) + from_jwk_bytes = from_jwk_public.public_bytes( + encoding=serialization.Encoding.DER, + format=serialization.PublicFormat.SubjectPublicKeyInfo, + ) + assert from_pem_bytes == from_jwk_bytes + + +# --------------------------------------------------------------------------- +# Purpose claim — adcp_use pins the key to a signing profile +# --------------------------------------------------------------------------- + + +def test_default_purpose_is_request_signing() -> None: + """``request-signing`` is the default because it's the common case + (agent → seller tool calls). Authors writing webhook-signing keys + need to opt in explicitly — mixing keys across profiles silently + fails at first delivery.""" + _, jwk = generate_signing_keypair(alg="ed25519", kid="test-kid") + assert jwk["adcp_use"] == "request-signing" + + +def test_webhook_signing_purpose_sets_adcp_use() -> None: + """The webhook-signing keys used by ``WebhookSender`` need + ``adcp_use == 'webhook-signing'`` on the JWK. Regression here + would silently produce keys that verifiers reject.""" + _, jwk = generate_signing_keypair(alg="ed25519", kid="test-kid", purpose="webhook-signing") + assert jwk["adcp_use"] == "webhook-signing" + + +def test_invalid_purpose_raises_value_error() -> None: + """Typos in ``purpose`` should fail loud — a silent fallback would + silently mint a key with an unexpected ``adcp_use``.""" + with pytest.raises(ValueError, match="purpose must be one of"): + generate_signing_keypair( + alg="ed25519", + kid="test-kid", + purpose="requets-signing", # type: ignore[arg-type] # typo + ) + + +def test_invalid_alg_raises_value_error() -> None: + """Same fail-loud guard for ``alg``.""" + with pytest.raises(ValueError, match="alg must be"): + generate_signing_keypair( + alg="rsa", # type: ignore[arg-type] + kid="test-kid", + ) + + +# --------------------------------------------------------------------------- +# Composition with WebhookSender — the PEM + JWK together sign + verify +# --------------------------------------------------------------------------- + + +def test_pem_loads_as_webhook_sender_private_key() -> None: + """The PEM produced with ``purpose='webhook-signing'`` must be + usable by ``WebhookSender.from_pem`` — the canonical programmatic + consumer. Round-trip via the sender's loader exercises the full + integration.""" + from adcp.webhook_sender import WebhookSender + + pem, jwk = generate_signing_keypair(alg="ed25519", kid="test-kid", purpose="webhook-signing") + + sender = WebhookSender.from_pem( + pem, + key_id=jwk["kid"], + alg="ed25519", + ) + assert sender is not None + + +# --------------------------------------------------------------------------- +# kid defaults + explicit override +# --------------------------------------------------------------------------- + + +def test_default_kid_includes_alg_and_date() -> None: + """Default kid follows ``adcp-{alg}-{YYYYMMDD}``. Callers not + managing rotation get a sane identifier out of the box; rotation- + aware callers pass an explicit kid. The format is a contract — + downstream tooling may parse it.""" + _, jwk = generate_signing_keypair(alg="ed25519") + assert jwk["kid"].startswith("adcp-ed25519-") + + _, jwk = generate_signing_keypair(alg="es256") + assert jwk["kid"].startswith("adcp-es256-") + + +def test_explicit_kid_is_preserved() -> None: + """Callers managing their own rotation supply a kid; the API + passes it through verbatim.""" + _, jwk = generate_signing_keypair(alg="ed25519", kid="rotation-2026-04-20-a") + assert jwk["kid"] == "rotation-2026-04-20-a" + + +# --------------------------------------------------------------------------- +# Encrypted PEM — passphrase round-trip +# --------------------------------------------------------------------------- + + +def test_encrypted_pem_requires_passphrase_to_load() -> None: + """``passphrase`` parameter wraps the PEM in BestAvailableEncryption. + Without it (None), the PEM is plain PKCS#8.""" + passphrase = b"test-passphrase-not-a-real-secret" + pem, _ = generate_signing_keypair(alg="ed25519", kid="test-kid", passphrase=passphrase) + + # Unencrypted load fails — the PEM requires the passphrase. + with pytest.raises((TypeError, ValueError)): + serialization.load_pem_private_key(pem, password=None) + + # Correct passphrase loads cleanly. + key = serialization.load_pem_private_key(pem, password=passphrase) + assert isinstance(key, ed25519.Ed25519PrivateKey) + + +# --------------------------------------------------------------------------- +# Shape contract — JWK fields per alg +# --------------------------------------------------------------------------- + + +def test_ed25519_jwk_shape() -> None: + """Ed25519 JWKs MUST have OKP kty, Ed25519 crv, EdDSA alg.""" + _, jwk = generate_signing_keypair(alg="ed25519", kid="k") + assert jwk["kty"] == "OKP" + assert jwk["crv"] == "Ed25519" + assert jwk["alg"] == "EdDSA" + assert jwk["use"] == "sig" + assert jwk["key_ops"] == ["verify"] + assert "x" in jwk + # Private scalar MUST NOT be in the JWK — it's public-only by design. + assert "d" not in jwk + + +def test_es256_jwk_shape() -> None: + """ES256 JWKs MUST have EC kty, P-256 crv, ES256 alg.""" + _, jwk = generate_signing_keypair(alg="es256", kid="k") + assert jwk["kty"] == "EC" + assert jwk["crv"] == "P-256" + assert jwk["alg"] == "ES256" + assert "x" in jwk + assert "y" in jwk + assert "d" not in jwk + + +# --------------------------------------------------------------------------- +# Top-level re-export +# --------------------------------------------------------------------------- + + +def test_exported_from_adcp_signing() -> None: + """``generate_signing_keypair`` is part of the signing public API. + Callers should be able to ``from adcp.signing import + generate_signing_keypair`` without digging into a submodule.""" + import adcp.signing as signing + + assert hasattr(signing, "generate_signing_keypair") + assert "generate_signing_keypair" in signing.__all__ + + +def test_cli_main_still_calls_helper() -> None: + """The CLI's ``main()`` now delegates to ``generate_signing_keypair``. + Regression check: the CLI path still works end-to-end (PEM written, + JWK printed). Full CLI coverage lives in ``test_keygen.py``; this + is just the "they share a spine" invariant.""" + from adcp.signing.keygen import generate_signing_keypair as _public + from adcp.signing.keygen import main + + # ``main`` doesn't rewrite the helper — this sanity-checks that + # they're the same callable the CLI wires up. + assert callable(_public) + assert callable(main) + + +# --------------------------------------------------------------------------- +# Signatures produced with the PEM verify against the JWK +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("alg", ["ed25519", "es256"]) +def test_signature_produced_with_pem_verifies_against_jwk(alg: str) -> None: + """End-to-end signing + verification using the PEM + JWK this + helper produces. This is the property every other check is + ultimately in service of.""" + import time + + from adcp.signing import ( + StaticJwksResolver, + VerifierCapability, + VerifyOptions, + sign_request, + verify_request_signature, + ) + + pem, jwk = generate_signing_keypair(alg=alg, kid="test-programmatic") + private_key = serialization.load_pem_private_key(pem, password=None) + + alg_rfc = "ed25519" if alg == "ed25519" else "ecdsa-p256-sha256" + body = b'{"programmatic": true}' + url = "https://seller.example.com/adcp/sync_creatives" + signed = sign_request( + method="POST", + url=url, + headers={"Content-Type": "application/json"}, + body=body, + private_key=private_key, # type: ignore[arg-type] + key_id=jwk["kid"], + alg=alg_rfc, + ) + headers = {"Content-Type": "application/json", **signed.as_dict()} + + options = VerifyOptions( + now=float(int(time.time())), + capability=VerifierCapability( + covers_content_digest="either", + required_for=frozenset({"sync_creatives"}), + ), + operation="sync_creatives", + jwks_resolver=StaticJwksResolver({"keys": [jwk]}), + ) + verify_request_signature(method="POST", url=url, headers=headers, body=body, options=options) From e6163077b84a73523b89c653154694d250124ea2 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Mon, 20 Apr 2026 14:44:29 -0400 Subject: [PATCH 2/2] fix(signing): PR #243 expert-review followups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both reviewers converged on two findings — both fixed: Security M1 + code nit: Doc example wrote PEM via Path.write_bytes which inherits the process umask (often 0644 = world-readable). Rewrote both the keygen module docstring and the signing/__init__.py quickstart to model the O_EXCL | 0o600 pattern the CLI already uses. Copy-paste readers now ship the secure default. Security M2 + code should-fix: default kid was adcp-{alg}-YYYYMMDD — date-stable per UTC day. Two rotations in 24h collided on the same kid, producing silent verification failures (verifiers cache the first JWK, reject signatures from the second as REQUEST_SIGNATURE_INVALID). Added a 4-char random hex suffix (secrets.token_hex(2)) so same-day calls diverge. Docstring now warns rotation callers MUST supply their own kid and the default is opaque — not a contract downstream tooling may parse. Code should-fix: test_cli_main_still_calls_helper was a tautology (asserting callable(main) + callable(public)). Replaced with a monkeypatch spy that asserts the CLI called generate_signing_keypair with the expected kwargs. Catches accidental re-inlining. Code should-fix: default_kid format test now asserts only "non-empty string" plus adds a same-day collision-resistance test. Doesn't lock the implementation to a particular format string. Code should-fix: encrypted PEM round-trip test now parametrized over ed25519 + es256 so a regression in the es256 encryption path can't escape. Security L2: passphrase lifecycle note added to the API docstring — CPython can't zero bytes; long-lived credential callers should source from a secret manager rather than holding a literal. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adcp/signing/__init__.py | 12 ++- src/adcp/signing/keygen.py | 47 +++++++++-- .../signing/test_keygen_programmatic.py | 77 +++++++++++++------ 3 files changed, 103 insertions(+), 33 deletions(-) diff --git a/src/adcp/signing/__init__.py b/src/adcp/signing/__init__.py index b496e2c2..4dc40d82 100644 --- a/src/adcp/signing/__init__.py +++ b/src/adcp/signing/__init__.py @@ -27,6 +27,8 @@ .. code-block:: python + import os + from adcp.signing import generate_signing_keypair # CLI equivalence: @@ -34,7 +36,15 @@ pem, public_jwk = generate_signing_keypair( alg="ed25519", purpose="webhook-signing" ) - Path("webhook-key.pem").write_bytes(pem) + + # Mode 0600, O_EXCL so an existing file is never overwritten. + # Path.write_bytes inherits the process umask (often 0644 = + # world-readable) — don't use it for private-key material. + fd = os.open("webhook-key.pem", os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + try: + os.write(fd, pem) + finally: + os.close(fd) publish_to_jwks_uri(public_jwk) **Sellers** (verifying incoming requests): diff --git a/src/adcp/signing/keygen.py b/src/adcp/signing/keygen.py index d1b62e89..3db3a57f 100644 --- a/src/adcp/signing/keygen.py +++ b/src/adcp/signing/keygen.py @@ -25,7 +25,15 @@ pem_bytes, public_jwk = generate_signing_keypair( alg="ed25519", purpose="webhook-signing" ) - Path("key.pem").write_bytes(pem_bytes) + + # Write mode-0600 atomically. DON'T use Path.write_bytes — it + # inherits the process umask (typically 0644 = world-readable). + import os + fd = os.open("key.pem", os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + try: + os.write(fd, pem_bytes) + finally: + os.close(fd) publish_to_jwks_uri(public_jwk) """ @@ -35,6 +43,7 @@ import getpass import json import os +import secrets import sys import unicodedata from datetime import datetime, timezone @@ -110,9 +119,21 @@ def generate_es256( def _default_kid(alg: str) -> str: - """Default ``kid`` derived from alg + UTC date. Callers who care - about key rotation should pass an explicit ``kid``.""" - return f"adcp-{alg}-{datetime.now(timezone.utc).strftime('%Y%m%d')}" + """Default ``kid`` — opaque, collision-resistant. + + Combines alg + UTC date + 4 random hex chars so two calls in the + same UTC day produce distinct kids. Format is an implementation + detail; downstream tooling MUST NOT parse it. Callers managing + rotation SHOULD pass an explicit ``kid`` they control. + + Same-day collisions without the random suffix silently break + verification: two JWKs advertised under the same kid, verifiers + cache the first one and reject signatures made with the second as + ``REQUEST_SIGNATURE_INVALID``. The suffix prevents that at no + readability cost. + """ + date = datetime.now(timezone.utc).strftime("%Y%m%d") + return f"adcp-{alg}-{date}-{secrets.token_hex(2)}" def generate_signing_keypair( @@ -131,10 +152,13 @@ def generate_signing_keypair( :param alg: Signature algorithm. ``"ed25519"`` (default; tiny keys, recommended) or ``"es256"`` (ECDSA over P-256, broader ecosystem support). - :param kid: Key ID to embed in the JWK. Defaults to - ``"adcp-{alg}-{YYYYMMDD}"`` — fine for one-off keys, but callers - managing rotation should supply an explicit kid so rotated keys - don't collide. + :param kid: Key ID to embed in the JWK. When omitted, the SDK mints + an opaque default combining alg + UTC date + 4 random hex chars + — suitable for first-time provisioning only. **Callers managing + rotation MUST supply their own ``kid``.** The default is + collision-resistant within a single process but does not + guarantee uniqueness across processes; rotation tooling needs + its own identifier scheme to track retirement / revocation. :param purpose: Which AdCP signing profile this key is for. Sets the JWK ``adcp_use`` claim. **Request-signing and webhook-signing keys MUST be distinct** — a signature from one surface cannot @@ -145,6 +169,13 @@ def generate_signing_keypair( automated deployments usually leave the PEM unencrypted and rely on filesystem perms (the CLI writes mode 0600). + **Passphrase lifecycle.** CPython cannot zero ``bytes``. Once + passed here, the buffer is consumed by ``cryptography`` and + then released to GC; there's no ``zeroize`` step. Callers + handling long-lived credentials should source the passphrase + from a secret manager per call rather than hold a Python + literal in process memory. + :returns: ``(pem_bytes, public_jwk)``. The PEM is PKCS#8 (optionally encrypted); the JWK is the public half, ready to publish at your ``jwks_uri``. The private scalar is NOT in the diff --git a/tests/conformance/signing/test_keygen_programmatic.py b/tests/conformance/signing/test_keygen_programmatic.py index 61651e9a..c42691c9 100644 --- a/tests/conformance/signing/test_keygen_programmatic.py +++ b/tests/conformance/signing/test_keygen_programmatic.py @@ -135,16 +135,27 @@ def test_pem_loads_as_webhook_sender_private_key() -> None: # --------------------------------------------------------------------------- -def test_default_kid_includes_alg_and_date() -> None: - """Default kid follows ``adcp-{alg}-{YYYYMMDD}``. Callers not - managing rotation get a sane identifier out of the box; rotation- - aware callers pass an explicit kid. The format is a contract — - downstream tooling may parse it.""" +def test_default_kid_is_non_empty_string() -> None: + """Default kid is opaque — the format is implementation detail, + NOT a contract downstream tooling may parse. Assert only that + callers get a non-empty string. If the default format changes + (it does — the random suffix was added for collision resistance), + this test doesn't break.""" _, jwk = generate_signing_keypair(alg="ed25519") - assert jwk["kid"].startswith("adcp-ed25519-") + assert isinstance(jwk["kid"], str) + assert jwk["kid"] - _, jwk = generate_signing_keypair(alg="es256") - assert jwk["kid"].startswith("adcp-es256-") + +def test_default_kid_is_collision_resistant_within_process() -> None: + """**Same-day regeneration guard.** Two calls on the same UTC day + must produce distinct default kids. Without the random suffix, + callers who rotate twice in 24h end up publishing two JWKs under + the same kid, verifiers cache the first, signatures from the + second fail with REQUEST_SIGNATURE_INVALID — silent verification + failure. Regression here reintroduces that footgun.""" + _, jwk_a = generate_signing_keypair(alg="ed25519") + _, jwk_b = generate_signing_keypair(alg="ed25519") + assert jwk_a["kid"] != jwk_b["kid"] def test_explicit_kid_is_preserved() -> None: @@ -159,11 +170,13 @@ def test_explicit_kid_is_preserved() -> None: # --------------------------------------------------------------------------- -def test_encrypted_pem_requires_passphrase_to_load() -> None: +@pytest.mark.parametrize("alg", ["ed25519", "es256"]) +def test_encrypted_pem_requires_passphrase_to_load(alg: str) -> None: """``passphrase`` parameter wraps the PEM in BestAvailableEncryption. - Without it (None), the PEM is plain PKCS#8.""" + Without it (None), the PEM is plain PKCS#8. Exercise both algs so + a regression breaking the es256 encryption path can't escape.""" passphrase = b"test-passphrase-not-a-real-secret" - pem, _ = generate_signing_keypair(alg="ed25519", kid="test-kid", passphrase=passphrase) + pem, _ = generate_signing_keypair(alg=alg, kid="test-kid", passphrase=passphrase) # Unencrypted load fails — the PEM requires the passphrase. with pytest.raises((TypeError, ValueError)): @@ -171,7 +184,8 @@ def test_encrypted_pem_requires_passphrase_to_load() -> None: # Correct passphrase loads cleanly. key = serialization.load_pem_private_key(pem, password=passphrase) - assert isinstance(key, ed25519.Ed25519PrivateKey) + expected_type = ed25519.Ed25519PrivateKey if alg == "ed25519" else ec.EllipticCurvePrivateKey + assert isinstance(key, expected_type) # --------------------------------------------------------------------------- @@ -218,18 +232,33 @@ def test_exported_from_adcp_signing() -> None: assert "generate_signing_keypair" in signing.__all__ -def test_cli_main_still_calls_helper() -> None: - """The CLI's ``main()`` now delegates to ``generate_signing_keypair``. - Regression check: the CLI path still works end-to-end (PEM written, - JWK printed). Full CLI coverage lives in ``test_keygen.py``; this - is just the "they share a spine" invariant.""" - from adcp.signing.keygen import generate_signing_keypair as _public - from adcp.signing.keygen import main - - # ``main`` doesn't rewrite the helper — this sanity-checks that - # they're the same callable the CLI wires up. - assert callable(_public) - assert callable(main) +def test_cli_main_delegates_to_generate_signing_keypair(tmp_path, monkeypatch) -> None: + """**Shared-spine invariant.** The CLI's ``main()`` must go through + ``generate_signing_keypair`` so a regression in either surface + shows up in both. Spy on the helper and assert the CLI called it + with the expected kwargs — a weaker assertion (``callable(main)``) + would pass even if someone silently re-inlined the CLI's key + generation.""" + from adcp.signing import keygen + + captured: dict[str, object] = {} + real = keygen.generate_signing_keypair + + def spy(**kwargs: object) -> object: + captured.update(kwargs) + return real(**kwargs) # type: ignore[arg-type] + + monkeypatch.setattr(keygen, "generate_signing_keypair", spy) + + out = tmp_path / "cli.pem" + rc = keygen.main(["--alg", "ed25519", "--out", str(out), "--kid", "cli-test-kid"]) + assert rc == 0 + assert captured == { + "alg": "ed25519", + "kid": "cli-test-kid", + "purpose": "request-signing", + "passphrase": None, + } # ---------------------------------------------------------------------------