fix(schema): refuse private key material in cnf.jwk - #296
Merged
Conversation
GHSA-vc4p-h84j-7qxj. RFC 8747 defines cnf as a confirmation key: the public half, carried so a verifier can bind the record to the key that signed it. models.TrustRecord already refuses d, p, q, dp, dq, qi and k, with the comment "cnf.jwk is a public proof-of-possession key". The verification path validates against the schema rather than the model, and the schema's jwk block constrained only kty, crv, x and y shapes. Everything else fell through additionalProperties, so a record carrying its own private key validated and sign.verify_record accepted it. No attacker is involved. This is a producer mistake the format did not defend against, and its consequence is durable: the record is signed, self-authenticating and typically anchored, so the only remedy afterwards is to revoke the identity. A format that can be handed the signing key by accident should say no in the half that verification actually calls. Both copies of the schema get the constraint and stay byte-identical, with a test that keeps them that way. sign.sign_record was never able to produce this, because it builds cnf from key_to_jwk, which returns the public half only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbDBXDWWvMFa7c2jGgyq9t
lywinged
added a commit
to lywinged/trace-spec
that referenced
this pull request
Sep 9, 2026
`schema/trace-claim.json` says of the confirmation key that "Keys must carry
actual key material", and enforced it for `OKP` and `EC` and for nothing else. A
`cnf.jwk` of `{"kty": "RSA"}` with no `n` and no `e` validated, and the record
then failed inside the verifier, where `jwk_thumbprint` reports a missing
required thumbprint member. Nothing was accepted that should have been refused,
since every path downstream fails closed. What was wrong is which instrument
spoke: the schema is the artifact an implementation in any language validates
against, and it was not the thing that told the producer the key was unusable.
What it tracks, since a schema change has to track normative text rather than a
description: section 3.2.2 requires the record to be bound by a signature made by
the key in `cnf`, and section 3.3 rule 1 has a verifier check that binding before
any other field is trusted. A confirmation key carrying no key material cannot
satisfy either, so refusing it enforces what those sections already require.
`sign.jwk_thumbprint` already knows `OKP`, `EC` and `RSA`, so naming the RSA
members adds no key type the package does not handle.
The reasoning for both of those sits in the `$comment` beside the constraint
rather than in `description`, which is the convention the private-member fix set
in the same block, and the `n` and `e` declarations are load-bearing rather than
decorative: without them a non-string modulus reaches `additionalProperties`,
which admits any canonicalizable value. A test covers that.
A `kty` enum would be a different act and is deliberately not done here. Section
3.2.1 states signing algorithms per envelope context and fixes no set for the
embedded-signature form of section 3.2.2, so narrowing `kty` in the schema would
add a constraint the specification does not make. That is a normative question,
and RFC 9964's registration of `AKP` for ML-DSA is the reason to answer it
deliberately rather than by hard-coding a list now.
Both copies of the schema move together, the published one and the packaged one,
and the new test asserts they are the same bytes, as `test_provenance_cnf_boundary`
already does for its own reason. The packaged v0.1 schema carries the same two
branches and is left alone, on the grounds agentrust-io#296 left it alone: nothing loads it, and
`test_the_superseded_schema_is_still_loaded_by_nothing` fails if anything starts to.
Signed-off-by: Louie Lu <48041247+lywinged@users.noreply.github.com>
9 tasks
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
1 participant
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 GHSA-vc4p-h84j-7qxj. Reproduced against current
mainbefore changing anything.The gap
RFC 8747 defines
cnfas a confirmation key: the public half, carried so a verifier can bind the record to the key that signed it.models.TrustRecordalready knew that. It refusesd,p,q,dp,dq,qiandkvia_JWK_PRIVATE_PARAMS, with the comment "cnf.jwk is a public proof-of-possession key".But the verification path validates against
schema/trace-claim.json, not the model. The schema'sjwkblock constrained only thekty/crv/x/yshapes through twoif/thenentries; everything else fell throughadditionalProperties. So a Trust Record carrying its own private key validated cleanly andverify_recordaccepted it.The rule existed, and only in the half that verification does not call.
Why it matters even with no attacker
There is no attacker step here. This is a producer mistake the format did not defend against. What makes it worth fixing rather than documenting is that the consequence is durable: the record is signed, self-authenticating, and typically anchored. Once it is out, the key is out, and the only remedy is to revoke the identity.
sign.sign_recordcould never produce this, because it buildscnffromkey_to_jwk, which returns the public half only. The exposure is a record assembled by hand or by another implementation, which is exactly the population a published schema exists to constrain.The fix
A
not/anyOfconstraint over the seven private JWK members, placed alongside the existingallOfshape rules, mirroring_JWK_PRIVATE_PARAMSin the reference model. Public JWKs are unaffected.Applied to both copies,
schema/trace-claim.json(the published artifact) andsrc/agentrust_trace/schema/trace-v0.2.json(what the package ships). They were byte-identical and still are, with a test that keeps them that way, since drifting copies is how this kind of gap survives a fix.Tests
Nine in
test_provenance_cnf_boundary.py: one per private member, two that ordinary OKP and EC public JWKs still validate, and the copy-equality check. Reverting the schema turns all seven member tests red.Full suite: 1172 passed, with the same 4 failures that reproduce on a clean
main(three fixture-regeneration tests and one schema-classification test, all unrelated).Not fixed here
agentrust-io/trace-testscarries a third copy of this schema with the same gap, and its conformance modules do not inspectcnf.jwkmembers (TR-ENV-004checks only thatktyis present, so a record carryingdpasses the suite). Tracked separately against that repo.🤖 Generated with Claude Code
https://claude.ai/code/session_01XbDBXDWWvMFa7c2jGgyq9t