Ticket 54/step 9 review fixes - #117
Merged
Merged
Conversation
… proof metadata Fixes the findings from the review of PR #116. Critical 1. Proof options are now canonicalized under the document context extended with https://w3id.org/security/suites/jws-2020/v1, so created, verificationMethod, proofPurpose, challenge and domain are covered by the signature. Previously the plain credentials/v1 context dropped all of them and the canonical proof options collapsed to a single type triple, leaving challenge and domain attacker-controlled. assertProofOptionsCovered fails closed if a populated field produces no triple. 2. LDProofChecker binds the proof key to the claimed identity: a credential proof must come from the credential's issuer, a presentation proof from the presentation's holder. It also requires the matching proofPurpose and, when the DID document declares verification relationships, that the key is authorized for authentication / assertionMethod. 3. JSON-LD credentials without a proof are rejected (ErrorUnsignedCredential) instead of silently accepted. 4. JSON-LD credentials embedded in a JWT VP go through the same credential verification as those in a JSON-LD VP. Major 5. Tests run against the real, vendored W3C credentials/v1 and jws-2020 contexts (common/contexts, embedded in the binary) instead of a synthetic context that defined the proof terms at the top level. Verification no longer depends on the network for those contexts either. 6. Rewrote the abandoned TestParseJSONLDPresentation_ValidVPWithCredentials into tests that actually sign a VP containing a signed credential, plus negative cases for unsigned, forged and tampered credentials. 7. GenerateToken (vp_token and token-exchange grants) enforces the domain binding; an omitted domain is a mismatch rather than a skipped check. 8. CLAUDE.md updated; the working IMPLEMENTATION_PLAN.md is replaced by docs/json-ld-proof-verification.md describing the resulting design. Minor 9. gofmt applied to the files this branch touches. 10. Collapsed jsonldVPProofKey into common.VPKeyProof; no literal "proof". 11. Removed the duplicated CachingStatusListClient doc comment and the overstated MITM claim. 12. Explicit EC curve cross-check (ES256 -> P-256, ES384 -> P-384, ES512 -> P-521). 13. signVerifiablePresentation falls back to a default document loader when M2MTokenProvider was built as a struct literal. 14. InitPresentationParser now runs before InitVerifier so the status-list client is handed the LDProofChecker explicitly instead of reaching for a package global. 15. proofPurpose is validated. 16. IMPLEMENTATION_PLAN.md removed from the repo root. Behaviour changes worth calling out: - Status lists must be issued by the issuer of the credential that references them (StatusListCredentialClient.Fetch takes the expected issuer). - Signing adds the suite context to the presentation, so M2M tokens carry an additional @context entry. - m2m.verificationMethod must be an absolute DID URL; the built-in default ("JsonWebKey2020") is not, and a startup warning is logged.
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.
No description provided.