Wire CryptoExt into WhitelistRegistry for brainpool-curve support - #153
Conversation
additional_trusted_roots was parsed via x509.CertPool.AppendCertsFromPEM directly, which can't handle curves stdlib crypto/x509 doesn't recognize (e.g. brainpoolP256r1, used by real ISO 18013-5/eIDAS reader- CA roots - confirmed live against the Geneva 2026 interop event's OpenID4VP verifier root). Worse, one such root failed the *entire* registry's CA pool construction, denying every other whitelisted verifier too, not just the one with the unsupported root. WhitelistRegistry now accepts a CryptoExt (WithWhitelistCryptoExt), using the same registry.ParseCertificatesPEM helper mdocrical/vical/ etc. already use, and both CLI construction paths in cmd/gt/main.go wire it up. Regression tests added using the real Geneva root, both with and without CryptoExt wired, to lock in the fix and document the original failure mode.
There was a problem hiding this comment.
Pull request overview
This PR wires go-cryptoutil CryptoExt support into static.WhitelistRegistry so additional_trusted_roots can be parsed even when certificates use curves unsupported by Go’s stdlib crypto/x509 (e.g., brainpool), and updates the CLI construction paths accordingly. This improves interoperability with real-world ISO 18013-5/eIDAS ecosystems and avoids failing registry CA pool construction due to unsupported-curve roots when CryptoExt is configured.
Changes:
- Add
WithWhitelistCryptoExtoption and storecryptoExtinWhitelistRegistryfor CryptoExt-aware parsing ofAdditionalTrustedRoots. - Switch CA pool augmentation from
x509.CertPool.AppendCertsFromPEMtoregistry.ParseCertificatesPEM(..., cryptoExt)+AddCert. - Add regression tests using a real brainpool-based root and wire CryptoExt in
cmd/gt/main.gowhitelist registry setup paths.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/registry/static/whitelist.go | Adds CryptoExt plumbing to parse additional trusted roots with non-stdlib curve support. |
| pkg/registry/static/whitelist_test.go | Adds regression coverage for brainpool-root behavior with/without CryptoExt. |
| cmd/gt/main.go | Wires a shared CryptoExt (with brainpool registered) into whitelist registry construction paths. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| certs, err := registry.ParseCertificatesPEM([]byte(pemCert), r.cryptoExt) | ||
| if err != nil || len(certs) == 0 { | ||
| r.systemCertPoolErr = fmt.Errorf("additional_trusted_roots[%d]: failed to parse PEM certificate", i) | ||
| return | ||
| } |
|
Summary
additional_trusted_rootswas parsed viax509.CertPool.AppendCertsFromPEMdirectly, which can't handle curves stdlibcrypto/x509doesn't recognize (e.g. brainpoolP256r1, used by real ISO 18013-5/eIDAS reader-CA roots) — confirmed live against the Geneva 2026 interop event's OpenID4VP verifier root.WhitelistRegistrynow accepts aCryptoExt(WithWhitelistCryptoExt), using the sameregistry.ParseCertificatesPEMhelpermdocrical/vical/etc. already use, and both CLI construction paths incmd/gt/main.gowire it up.Test plan
go test ./pkg/registry/static/...— full suite green, including new regression tests using the real Geneva root, both with and withoutCryptoExtwiredgo build ./.../go vet ./...cleansystem CA pool unavailable: additional_trusted_roots[N]: failed to parse PEM certificateand denied trust forverifier.multipaz.orgtoo🤖 Generated with Claude Code