Support https based issuers - #119
Conversation
The previous implementation plan covered EBSI TIR v5 API support, which is already implemented in the codebase. This replaces it with a plan that actually addresses the ticket: supporting HTTPS-based credential issuer identifiers following the OAuth2.0/OIDC trust model with .well-known metadata discovery (OID4VCI, RFC 8414). 4 steps: HTTPS issuer metadata resolver, JWTProofChecker integration, trust list type wiring, and end-to-end tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implementation plan created with 4 steps for supporting HTTPS-based client identifiers in VCVerifier.
- Remove accidentally committed gh_2.50.0_linux_amd64/ folder (gh CLI installation artifact, not part of the project) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add proper spec references for jwks_uri (SD-JWT VC spec Section 5.2) and clarify it is NOT from the OpenID4VCI spec - Add support for OpenID4VCI authorization_servers fallback path to resolve JWKS via OAuth Authorization Server metadata (RFC 8414) - Add separate metadata structs for each well-known endpoint - Remove "https" trust list type concept; instead treat Issuer.ID as a generic URI and auto-detect DID vs HTTPS URL in trust validation - Update overview and all steps to reflect URI-based approach Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-53/plan into ticket-53/work Reviewed-on: http://localhost:3001/general-agent-4/VCVerifier/pulls/7 Reviewed-by: wistefan <wistefan@dev-env.local>
Implement HttpsIssuerResolver interface and CachingHttpsIssuerResolver for discovering and resolving signing keys from HTTPS-based credential issuer identifiers. Supports two metadata discovery paths: - Primary: SD-JWT VC issuer metadata via /.well-known/jwt-vc-issuer with jwks_uri or inline jwks - Fallback: OpenID4VCI credential issuer metadata via /.well-known/openid-credential-issuer with authorization_servers chain resolution Includes caching layer using common.Cache to avoid redundant HTTP requests, issuer mismatch validation per RFC 8414 Section 3.3, and comprehensive test coverage with 14 test cases covering both happy paths and error scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ata resolver' (#8) from ticket-53/step-1 into ticket-53/work Reviewed-on: http://localhost:3001/general-agent-4/VCVerifier/pulls/8 Reviewed-by: wistefan <wistefan@dev-env.local>
Add HTTPS-based issuer support to the JWT signature verification pipeline. When a JWT's iss claim starts with "https://", the proof checker now routes key resolution through the HttpsIssuerResolver (metadata discovery + JWKS) instead of DID resolution. DID-based issuers continue to work unchanged. Changes: - jwt_proof_checker.go: Add httpsResolver field, WithHttpsResolver setter, isHttpsIssuer helper, verifyHttpsIssuerJWT method, and ErrorHttpsIssuerNotSupported error sentinel. - presentation_parser.go: Wire up CachingHttpsIssuerResolver in InitPresentationParser via WithHttpsResolver. - jwt_proof_checker_test.go: Add 10 test cases covering HTTPS issuer success, no resolver configured, resolver errors, wrong key, kid handling, and DID bypass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…esolution into JWTProofChecker' (#9) from ticket-53/step-2 into ticket-53/work Reviewed-on: http://localhost:3001/general-agent-4/VCVerifier/pulls/9 Reviewed-by: wistefan <wistefan@dev-env.local>
Add HTTPS-URL-based issuer support to trusted participant and trusted issuer validation. When the credential issuer ID is an HTTPS URL rather than a DID, trust is established via URL matching against configured trust list entries instead of external registry lookups (EBSI, Gaia-X). DID-based issuers continue through existing type-based dispatch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…URIs in trust validation' (#10) from ticket-53/step-3 into ticket-53/work Reviewed-on: http://localhost:3001/general-agent-4/VCVerifier/pulls/10 Reviewed-by: wistefan <wistefan@dev-env.local>
Add integration-level tests for the full AuthenticationResponse flow with HTTPS-based credential issuers including same-device, cross-device, and multi-issuer scenarios. Add tests verifying getTrustRegistriesValidationContext propagates HTTPS issuer URLs correctly through the validation pipeline. Add YAML test fixture demonstrating HTTPS issuer URLs configured alongside DID-based registry entries in trustedIssuersLists and trustedParticipantsLists. Add reusable HTTPS issuer mock helpers in integration_test/helpers/ for serving .well-known/jwt-vc-issuer metadata, .well-known/openid-credential-issuer metadata (OpenID4VCI fallback), and JWKS endpoints. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dentifiers in the vc-verifier' (#11) from ticket-53/step-4 into ticket-53/work Reviewed-on: http://localhost:3001/general-agent-4/VCVerifier/pulls/11 Reviewed-by: wistefan <wistefan@dev-env.local>
…t-53/work Resolved conflicts: - verifier/jwt_proof_checker.go: kept both the HTTPS issuer verification path (ticket-53) and the delegation of resolveKey to the shared ResolveKeyFromDID helper (ticket-54). - verifier/presentation_parser.go: kept both imports; the HTTPS issuer resolver wiring and the new LDProofChecker wiring coexist. - verifier/verifier_test.go: kept both appended test blocks. - IMPLEMENTATION_PLAN.md: removed, following upstream's cleanup of working-plan artifacts.
Upstream main added a JSON-LD Linked Data Proof verification path and issuer-bound status-list verification. Both resolved issuer keys through DID resolution only, so a credential from an HTTPS-based issuer failed there even though the JWT path already supported it. - ld_proof_checker.go: resolve an https:// verificationMethod through the HttpsIssuerResolver. The JWKS kid is taken from the URI fragment (httpsJwksKeyId), since a JWKS is not keyed by the full URI. A JWKS has no verification relationships, so the authentication/assertionMethod requirement is logged as unenforceable rather than silently dropped — proofPurpose and issuer/holder binding still apply. - credential_status_client.go: resolve an https:// status-list iss claim through the resolver (resolveKeyFromIssuer dispatches on the scheme). - presentation_parser.go / verifier.go: create the resolver once and share it via GetHttpsIssuerResolver(), so one JWKS cache serves the JWT path, the JSON-LD path and status-list verification. Both paths fail closed with ErrorHttpsIssuerNotSupported when no resolver is configured. Adds table-driven tests for both paths (valid proof, missing resolver, resolver error, foreign key, signer/issuer mismatch, tampered payload, resolver arguments, DID issuers unaffected) and documents the feature in docs/https-issuer-identifiers.md and CLAUDE.md.
… HTTPS issuer support to the JSON-LD and status-list paths' (#12) from ticket-53/merge-upstream into ticket-53/work Reviewed-on: http://localhost:3001/general-agent-4/VCVerifier/pulls/12 Reviewed-by: wistefan <wistefan@dev-env.local>
|
Please apply one of the following labels to the PR: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Please apply one of the following labels to the PR: |
|
Please apply one of the following labels to the PR: |
|
Please apply one of the following labels to the PR: |
wistefan
added
the
minor
vramperez
left a comment
vramperez
left a comment
There was a problem hiding this comment.
Reviewed the HTTPS-issuer support. The resolver is carefully built and well documented; my comments are mostly about the outer edge of it - the first hop and the failure cache - plus a few smaller consistency points.
| // discovery. Only absolute http(s) URLs with a host are accepted; https is what | ||
| // production identifiers use, http is accepted so tests can run against a local | ||
| // server. | ||
| func parseIssuerURL(issuerURL string) (*url.URL, error) { |
There was a problem hiding this comment.
The metadata-derived hops are pinned, but the first hop isn't: the issuer URL comes straight from an unauthenticated vp_token and proof verification runs before any trust-registry check, so iss: "https://10.0.0.5:8443/x" or a link-local address makes the verifier issue that GET, and varying the path probes the internal network. Consider rejecting issuer identifiers that resolve to loopback/RFC1918/link-local addresses (ideally checked in DialContext to avoid rebind), and softening the "not an SSRF primitive" note in the docs to cover only the later hops.
| func issuerCacheKey(issuerBase *url.URL) string { | ||
| key := strings.ToLower(issuerBase.Scheme) + "://" + strings.ToLower(issuerBase.Host) + | ||
| strings.TrimSuffix(issuerBase.EscapedPath(), "/") | ||
| if issuerBase.RawQuery != "" { |
There was a problem hiding this comment.
issuerCacheKey includes query and fragment while issuerPath drops them when building the well-known URL, so https://victim.example?1, ?2, ... all hit the same endpoint but each is a cache miss - the failure cache is bypassable with a counter in the query. RFC 8414 §2 forbids query/fragment in an issuer identifier anyway; rejecting them in parseIssuerURL closes this and lets the two branches here go away.
| candidates = append(candidates, key) | ||
| } | ||
|
|
||
| if len(candidates) == 0 { |
There was a problem hiding this comment.
Keys that declare no kid are dropped when the JWS carries one, so an issuer publishing a single JWKS key without a kid never verifies and re-triggers a refetch every MinJwksRefetchInterval. Since kid is only a hint (RFC 7515 §4.1.4) and the signature still has to validate, consider falling back to all signature-capable keys when nothing matches the requested kid.
| // denylist: the `alg` header comes from the token, so anything not listed here | ||
| // — `none` above all, and the symmetric HS* family, where a public key doubles | ||
| // as the shared secret — must never reach the verification step. | ||
| var allowedSignatureAlgorithms = map[string]bool{ |
There was a problem hiding this comment.
verifyWithX5C still passes the raw header alg to jws.Verify (credential_status_client.go:569), so the status-list x5c fallback is the one path the new allowlist doesn't cover. Routing it through verifyJWSWithCandidateKeys with the imported cert key would make the pinning uniform.
| continue | ||
| } | ||
| seconds, err := strconv.Atoi(strings.TrimPrefix(directive, cacheControlMaxAge)) | ||
| if err != nil || seconds <= 0 { |
There was a problem hiding this comment.
max-age=0 and no-store/no-cache all collapse to 0 here, which jwksTTL reads as "no declaration" and caches for the full 15 minutes - the opposite of what the origin asked. Consider returning (duration, ok) and skipping the cache write in that case; note cache.Set with a 0 TTL means "default expiration" in go-cache, not "don't cache".
| unbounded walk would let one metadata document drive thousands of requests. | ||
| - Each request is bounded by `httpClientTimeout` (10s) **and** the whole | ||
| resolution by `resolutionTimeout` (30s), so the number of hops a document | ||
| asks for cannot extend the total. The caller's context is honoured, so once |
There was a problem hiding this comment.
All three call sites currently pass context.Background() (as their own comments note), so this reads as a guarantee that doesn't hold yet - worth rephrasing as "the resolver honours a caller context once one is threaded down".
| @@ -0,0 +1,3 @@ | |||
|
|
|||
| # Review artifacts, kept locally | |||
| review.md | |||
There was a problem hiding this comment.
This adds a repo-level .gitignore just for a personal review artifact; .git/info/exclude is the place for that, and it keeps the PR to the feature.
3 participants
Support issuers provided with a plain url to be resolved, following the OID4VC spec.