fix(ecdsa-sd-2023): include root type quads in mandatory selection pe… #239
Conversation
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
…r W3C spec This commit fixes ECDSA-SD-2023 BASE proof verification for real-world credentials (such as Singapore Academy of Law eApostille) by implementing W3C VC-DI-ECDSA spec Section 3.4.11 createInitialSelection correctly. The Problem: - SAL eApostille credentials failed verification despite W3C test vectors (59/59) passing - Root cause: mandatory hash mismatch between Go and reference implementation (Digital Bazaar di-sd-primitives) The Fix: Per W3C spec Section 3.4.11: 'The selection MUST include all types in the path of any JSON Pointer, including any root document type.' Changes: - sd_helpers.go: Updated selectMandatoryNQuads to track container paths and include root document rdf:type quads when any pointer touches the root - sd_helpers.go: Added helper functions parseJSONPointer, getValueAtPointer, parseEphemeralPublicKey, ellipticMarshalCompressed, applyHMACLabelReplacement - sd_suite.go: Updated verifyBaseProof to use the fixed mandatory selection logic and properly validate BASE proofs Test Results: - All 24 ECDSA tests pass (go test -tags vc20 ./pkg/vc20/crypto/ecdsa/) - SAL eApostille verification now succeeds - W3C test vectors continue to pass (59/59)
ADR-10: Mandatory N-Quad selection must include type quads per W3C spec 3.4.11 ADR-11: Real-world test vectors required beyond W3C conformance tests ADR-12: Support both BASE and DERIVED proof verification ADR-13: Align with Digital Bazaar reference implementation for interop
Add real-world test vectors from Singapore issuers: - Accredify corporate and citizen ID credentials (eddsa-rdfc-2022) - Singapore Academy of Law eApostille credentials (ecdsa-sd-2023) Add integration tests: - singapore_testvectors_test.go: Tests for verifying Singapore credentials using DID resolution against live issuer endpoints - gotrust_testserver_test.go: Tests using go-trust test server for controlled integration testing Update dependencies: - Bump go-trust for test server support - Add transitive dependencies for SAML/XML signing support Fix sd_suite_coverage_test.go: - Improve proof tampering test to modify signature region rather than just the trailing character, ensuring proper tampering detection These test vectors validate the ECDSA-SD-2023 BASE proof verification fix from the previous commit against real-world credentials.
Add verify-sal.mjs - a Node.js script using Digital Bazaar's ecdsa-sd-2023-cryptosuite reference implementation for verifying Singapore credentials. Useful for debugging and comparing behavior against the Go implementation.
Fix RDF credential normalization and serialization for proper Data Integrity proof generation and verification.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes ECDSA-SD-2023 BASE proof verification for real-world credentials by correctly implementing W3C VC-DI-ECDSA spec Section 3.4.11 (createInitialSelection). The key fix ensures that root document type quads are included in mandatory selection when any JSON pointer touches the root level.
Changes:
- Fixed mandatory N-Quad selection to include root type quads per W3C spec
- Added support for BASE proof verification (CBOR tag 0xd95d00) alongside DERIVED proofs
- Implemented helper functions for JSON pointer parsing, ephemeral key handling, and HMAC label replacement
- Added comprehensive test coverage with Singapore Academy of Law eApostille test vectors
Reviewed changes
Copilot reviewed 16 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/vc20/crypto/ecdsa/sd_helpers.go |
Added selectMandatoryNQuads, parseJSONPointer, parseEphemeralPublicKey, and HMAC helper functions |
pkg/vc20/crypto/ecdsa/sd_suite.go |
Fixed BASE proof creation/verification with proper mandatory selection and CBOR tag handling |
pkg/vc20/crypto/ecdsa/sd_suite_coverage_test.go |
Updated tampered proof test to affect base signature region |
pkg/vc20/credential/rdf_credential.go |
Enhanced CredentialWithoutProofForTypes and added ToCompactJSON method |
pkg/vc20/crypto/ecdsa/sd_eapostille_test.go |
Added 814 lines of comprehensive eApostille verification tests |
pkg/keyresolver/singapore_testvectors_test.go |
Added 939 lines of Singapore test vector integration tests |
pkg/keyresolver/gotrust_testserver_test.go |
Added 2453 lines of GoTrust resolver tests with real did:web resolution |
testdata/sg-test-vectors/README.md |
Documented Singapore test vectors and usage |
testdata/verify-sal.mjs |
Added JavaScript debug script for reference implementation comparison |
go.mod, go.sum, vendor/modules.txt |
Updated go-trust dependency and added new transitive dependencies |
docs/adr/10-12-13.md |
Added four ADRs documenting design decisions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import {DataIntegrityProof} from '@digitalbazaar/data-integrity'; | ||
| import jsigs from 'jsonld-signatures'; | ||
| import {readFileSync} from 'fs'; | ||
| import {createLoader} from '@digitalbazaar/did-io'; |
There was a problem hiding this comment.
Unused import createLoader.
Suggested change
| import {createLoader} from '@digitalbazaar/did-io'; |
| import jsigs from 'jsonld-signatures'; | ||
| import {readFileSync} from 'fs'; | ||
| import {createLoader} from '@digitalbazaar/did-io'; | ||
| import {documentLoader as defaultLoader} from '@digitalbazaar/jsonld-document-loader'; |
There was a problem hiding this comment.
Unused import defaultLoader.
Suggested change
| import {documentLoader as defaultLoader} from '@digitalbazaar/jsonld-document-loader'; |
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.
…r W3C spec
This commit fixes ECDSA-SD-2023 BASE proof verification for real-world credentials (such as Singapore Academy of Law eApostille) by implementing W3C VC-DI-ECDSA spec Section 3.4.11 createInitialSelection correctly.
The Problem:
The Fix:
Per W3C spec Section 3.4.11: 'The selection MUST include all types in the path of any JSON Pointer, including any root document type.'
Changes:
Test Results: