Skip to content

fix(security): bind cnf.jwk to the trusted signing key - #157

Merged
imran-siddique merged 3 commits into
mainfrom
agent/bind-cnf-to-trusted-key
Aug 11, 2026
Merged

fix(security): bind cnf.jwk to the trusted signing key#157
imran-siddique merged 3 commits into
mainfrom
agent/bind-cnf-to-trusted-key

Conversation

@imran-siddique

Copy link
Copy Markdown
Member

Why

TRACE §3.2.2 defines the signature binding as a signature made by the key in cnf. The verifier required a trusted key and verified the signature with it, but did not require signed cnf.jwk to name that same key. A trusted signer could therefore authenticate a record that directed downstream proof-of-possession checks to an unrelated key.

What changed

  • derive RFC 7638 thumbprints for the trusted and embedded keys
  • require both thumbprints to match before accepting the signature
  • validate the embedded key as Ed25519
  • compare key identity while ignoring optional JWK metadata such as kid and use
  • preserve revocation checks against the trusted key before inspecting the embedded key
  • clarify the two-key trust and binding process in both verification guides

Verification

  • focused signature tests: 47 passed
  • full suite: 326 passed, 1 skipped
  • ruff check src tests
  • mypy src/agentrust_trace
  • git diff --check

Regressions cover a valid trusted signature naming another confirmation key, missing cnf.jwk, optional trusted-key metadata, wrong trusted keys, tampering, public-key objects, and revocation ordering.

@lywinged

Copy link
Copy Markdown
Contributor

Noting a convergence rather than raising an objection, because it happened twice in eleven hours and that is the interesting part.

This PR compares jwk_thumbprint(embedded_jwk) against jwk_thumbprint(trusted_jwk) in sign.py, ignoring optional JWK metadata. #149, opened eleven hours earlier, does the same comparison in provenance.py for the same reason: a JWK is identified by its key material and kid carries none of it. Different files, different defects, no textual conflict, and the same technique arrived at independently by two people on the same day, in a package that already defines and exports jwk_thumbprint.

The near-term consequence is small and worth naming: when both land there will be two key-identity comparisons written separately. Whichever merges second is the natural place to reuse the first rather than restate it, and I am happy for that to be #149 — it is the smaller of the two and I can rebase it onto whatever this lands.

The longer-range version of the same observation, offered as an observation and not a proposal. Three code paths verify something signed; they share _canonical_bytes and _pubkey_from_jwk and nothing above them. What sits above them has diverged: sign.verify_record consults revocation, provenance.verify_record contains the string revocation zero times, and §3.2.1 puts that requirement on a verifier rather than on one format. Key identity has now diverged and been re-converged by hand. Freshness is bounded in both but separately, and this PR's sibling #155 changes it in one of them.

I am not proposing the refactor. An envelope layer owning canonical bytes, key resolution, revocation and freshness would couple the release cadence of modules that currently ship independently, and content_marking would use a fraction of it. That trade is a scheduling decision and it is yours, not mine. What I can contribute is the evidence that the gap is costing something: two defects and now one duplicated fix, all inside a week.

lywinged added a commit to lywinged/trace-spec that referenced this pull request Aug 11, 2026
And the strongest of the three, because it is not a defect. Upstream agentrust-io#157 and
our agentrust-io#149 wrote the same RFC 7638 key-identity comparison in two files of one
package, eleven hours apart, in a package that already exports the helper.

The first two pieces of evidence were things that went wrong. This one is work
done twice, which is what a missing shared layer costs when nothing is broken.

Offered on agentrust-io#157 with the refactor explicitly not proposed: the release-cadence
cost is the maintainer/s to weigh, and the only action named is that whichever
of the two merges second should reuse the first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: lywinged <48041247+lywinged@users.noreply.github.com>
@imran-siddique
imran-siddique marked this pull request as ready for review August 11, 2026 15:37
imran-siddique and others added 2 commits August 11, 2026 08:45
… absence first

Schema enforcement (#156) refuses a record with no cnf before the
cnf-to-trusted-key binding is reached, so the message changed while the
rejection did not. The binding is still covered by the two tests where cnf
is present and names the wrong key.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@imran-siddique
imran-siddique merged commit 0fe6512 into main Aug 11, 2026
5 checks passed
@imran-siddique
imran-siddique deleted the agent/bind-cnf-to-trusted-key branch August 11, 2026 15:49
lywinged added a commit to lywinged/trace-spec that referenced this pull request Aug 11, 2026
verify_record compared the record's cnf.jwk against the trusted key with dict
equality. A JWK is identified by the key material; kid, use and alg are optional
members that carry none of it. key_to_jwk emits the bare {crv, kty, x}, while a
key resolved from a JWKS endpoint normally carries kid, because that is how JWKS
distinguishes keys across rotation. The two are the same public key and not the
same dict, so a consumer holding its trusted key from a JWKS rejected every
record, with a message accusing the publisher of substituting a key:

    ProvenanceError: the record's embedded key is not the trusted key. A record
    signed by some other key is a record about a server somebody else is
    describing.

It was signed by exactly the right key. The signature verified; only the
comparison failed.

Compared by RFC 7638 thumbprint instead, which sign.jwk_thumbprint already
implements and whose docstring already states the property needed here: stable
across JWKs that differ in optional members such as kid, alg or use. A different
key still fails, and a cnf.jwk with no usable kty is now a refusal rather than an
uncaught ValueError.

This is the same comparison agentrust-io#157 has since landed in sign.verify_record, and it
is written the same way, compare_digest over the two thumbprints, so that the two
verifiers answer "is this the trusted key" identically. The error type differs
because each module raises its own: ProvenanceError here, ValueError there. After
agentrust-io#157 the divergence is visible in one tree, one verifier identifying a key by its
material and the other by its dict shape.

The regexes anchored with $, which in Python also matches before one final
newline, so publisher "did:web:acme.example\n" and a tool_catalog.hash with a
trailing newline passed verify_record. Both now anchor with \Z.

Raised on agentrust-io#146 and not part of that fix; agentrust-io#142 is closed, so both were untracked.

Six tests, each load-bearing: reverting the thumbprint comparison fails three,
reverting the anchors fails two. The embedded-key case is signed by hand, since
sign_record builds cnf itself and this implementation cannot emit a cnf.jwk
carrying kid - another implementation can, and the format permits it.

Rebased onto dd1b6e6. 345 tests pass; ruff and mypy clean.

Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
lywinged added a commit to lywinged/trace-spec that referenced this pull request Aug 12, 2026
verify_record compared the record's cnf.jwk against the trusted key with dict
equality. A JWK is identified by the key material; kid, use and alg are optional
members that carry none of it. key_to_jwk emits the bare {crv, kty, x}, while a
key resolved from a JWKS endpoint normally carries kid, because that is how JWKS
distinguishes keys across rotation. The two are the same public key and not the
same dict, so a consumer holding its trusted key from a JWKS rejected every
record, with a message accusing the publisher of substituting a key:

    ProvenanceError: the record's embedded key is not the trusted key. A record
    signed by some other key is a record about a server somebody else is
    describing.

It was signed by exactly the right key. The signature verified; only the
comparison failed.

Compared by RFC 7638 thumbprint instead, which sign.jwk_thumbprint already
implements and whose docstring already states the property needed here: stable
across JWKs that differ in optional members such as kid, alg or use. A different
key still fails, and a cnf.jwk with no usable kty is now a refusal rather than an
uncaught ValueError.

This is the same comparison agentrust-io#157 landed in sign.verify_record, written the same
way, compare_digest over the two thumbprints, so that the two verifiers answer
"is this the trusted key" identically. The error type differs because each module
raises its own: ProvenanceError here, ValueError there.

Rebased onto 5367970. The \Z anchor change this branch also carried is now
upstream via agentrust-io#154, so it is dropped here along with the two newline tests it
duplicated; agentrust-io#154's four parametrized cases cover the same ground and more.

Four tests, each load-bearing: reverting the thumbprint comparison fails three.
The embedded-key case is signed by hand, since sign_record builds cnf itself and
this implementation cannot emit a cnf.jwk carrying kid - another implementation
can, and the format permits it.

347 tests pass; ruff and mypy clean.

Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
imran-siddique pushed a commit that referenced this pull request Aug 14, 2026
…pe (#149)

verify_record compared the record's cnf.jwk against the trusted key with dict
equality. A JWK is identified by the key material; kid, use and alg are optional
members that carry none of it. key_to_jwk emits the bare {crv, kty, x}, while a
key resolved from a JWKS endpoint normally carries kid, because that is how JWKS
distinguishes keys across rotation. The two are the same public key and not the
same dict, so a consumer holding its trusted key from a JWKS rejected every
record, with a message accusing the publisher of substituting a key:

    ProvenanceError: the record's embedded key is not the trusted key. A record
    signed by some other key is a record about a server somebody else is
    describing.

It was signed by exactly the right key. The signature verified; only the
comparison failed.

Compared by RFC 7638 thumbprint instead, which sign.jwk_thumbprint already
implements and whose docstring already states the property needed here: stable
across JWKs that differ in optional members such as kid, alg or use. A different
key still fails, and a cnf.jwk with no usable kty is now a refusal rather than an
uncaught ValueError.

This is the same comparison #157 landed in sign.verify_record, written the same
way, compare_digest over the two thumbprints, so that the two verifiers answer
"is this the trusted key" identically. The error type differs because each module
raises its own: ProvenanceError here, ValueError there.

Rebased onto 5367970. The \Z anchor change this branch also carried is now
upstream via #154, so it is dropped here along with the two newline tests it
duplicated; #154's four parametrized cases cover the same ground and more.

Four tests, each load-bearing: reverting the thumbprint comparison fails three.
The embedded-key case is signed by hand, since sign_record builds cnf itself and
this implementation cannot emit a cnf.jwk carrying kid - another implementation
can, and the format permits it.

347 tests pass; ruff and mypy clean.

Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants