fix(verify): enforce key revocation at verification time (#76) - #113
Open
zohebk8s wants to merge 1 commit into
Open
fix(verify): enforce key revocation at verification time (#76)#113zohebk8s wants to merge 1 commit into
zohebk8s wants to merge 1 commit into
Conversation
…o#76) Spec 3.2.1 requires that "Verifiers MUST consult current revocation status at verification time", but verify_record() checked only the signature and freshness. A record signed by a revoked or compromised key kept verifying, because a signature stays cryptographically valid forever and nothing inside a record can retract the key that made it. The verifier is the only place that withdrawal of trust can be applied. Add a revocation store to the verify path. It accepts either a container of revoked key identifiers, for a list the caller already holds, or a callable performing a live CRL, status-endpoint, or SCITT lookup. A listed key is rejected, and so is a store that cannot answer: an unavailable revocation source is not evidence that a key is unrevoked, and treating a lookup failure as a pass would mean an attacker holding a revoked key only has to make the endpoint unreachable. Keys are identified by RFC 7638 JWK Thumbprint or kid. The check reads the trusted key rather than record["cnf"]["jwk"], which is attacker-controlled until the signature verifies; keying the lookup on the embedded key would let a revoked issuer present an unlisted thumbprint and pass. Also exports jwk_thumbprint(), so a caller can key a revocation list on the same identifier the verifier derives. Pinned against the RFC 8037 A.3 known-answer vector. Additive and backward compatible: revocation defaults to None, which leaves verification purely offline and byte-for-byte unchanged. That mode cannot prove non-revocation, which LIMITATIONS.md and docs/verification.md now say plainly rather than leaving implied by "independently verifiable offline". No normative text, schema, or record field changed. Verified: 105 tests pass (14 new), ruff and mypy clean, all new lines covered. Downstream cmcp suite passes unchanged against the patched package, 893 passed before and after. Refs agentrust-io#76 Signed-off-by: zohebk8s <zoheb.shaik7@gmail.com>
Contributor
|
🟡 Contributor Check: MEDIUM
Automated check by AgenTrust Contributor Check. |
Author
|
Fork CI is green on this exact commit (283383d), both matrix legs: test (3.11) and test (3.12): lint, type check, and tests all pass. |
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.
What this changes
Closes #76.
verify_record()checked signature and freshness but never consulted revocation or transparency state, so a record signed by a revoked or compromised key kept verifying. This is a gap against the spec's own normative text, not a new requirement: §3.2.1 already says "Verifiers MUST consult current revocation status at verification time".A signature stays cryptographically valid forever and nothing inside a record can retract the key that made it, so the verifier is the only place withdrawal of trust can be applied.
The revocation store
verify_record(..., revocation=...)accepts either form:ValueError)revocationpassedFailing closed on an unreachable store is the part that matters: if a lookup failure fell through to "verified", an attacker holding a revoked key would only need to make the status endpoint unreachable.
Two design points worth review
The check reads the trusted key, not
record["cnf"]["jwk"]. The embedded key is attacker-controlled until the signature verifies, so keying the lookup on it would let a revoked issuer present an unlisted thumbprint and pass. There is a test pinning this.Identifiers are the RFC 7638 JWK Thumbprint or
kid. Thumbprints work when an issuer publishes nokid, and are stable across JWKs differing in optional members.jwk_thumbprint()is exported so a caller can key a list on the same identifier the verifier derives; it is pinned against the RFC 8037 Appendix A.3 known-answer vector.Scope
Deliberately not in this PR:
revocationdefaults toNonerather than being required. Making it mandatory is a breaking API change for every consumer (cMCP, ca2a) and is better decided alongside [pre-v1.0] Key revocation: TraceRevocation claim + entry-ID anchor #67, which designs theTraceRevocationclaim and the entry-ID anchor. This PR gives the verify path the hook; [pre-v1.0] Key revocation: TraceRevocation claim + entry-ID anchor #67 can decide what feeds it.Documented rather than left implied:
docs/verification.mdopens with "independently verifiable offline", which is true and is exactly why non-revocation is the one thing it cannot establish. That page andLIMITATIONS.mdnow say so directly.Type of change
None of the listed categories fit, so I have left them unticked rather than claim one inaccurately. This is a library and docs change that implements an existing normative MUST. No spec text, schema, or record field is modified. Happy to tick whichever box you consider correct.
Spec section
3.2.1 (Signing and key management), implemented rather than modified. No spec text changed.
Checklist
git commit -s)CHANGELOG.mdupdated<!-- CHANGED: #NNN — description -->in spec text (not applicable, no breaking change)None, so offline verification is byte-for-byte unchangedVerification
sign.pymisses are pre-existing).ruff check src testsandmypy src/agentrust_trace(strict) clean.