You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
authzen: return verifier identifiers in the evaluation response context (verifier_ids) - #204
Refs #203. Implements the first-implementation scope of #203: an optional verifier_ids member in the AuthZEN evaluation response context, so a wallet that answers a circuit_zk request with the pseudonym function gets a verified identifier string for the relying party without parsing certificates, resolving DIDs or walking federation chains.
Off by default. Enabled per policy: constraints.return_verifier_ids: [semantic, dns]. Existing responses and tests are unchanged. Unknown (and deferred) type names are rejected at config load (Config.Validate) and again when the policy is installed.
Request hint. The optional request context.verifier_id_type narrows the response to one type. It only narrows: return_verifier_ids is a server-side policy control and is stripped from client context like the other controls; the hint is added to the client-suppliable allowlist.
Types:semantic (access certificate only), dns / uri (after VerifyLeafBinding), did (didweb, didwebvh, didjwks, generic did; resolved + key matched), oidf_entity (validated chain + key matched).
authzenclient exposes it through authzen.EvaluationResponseContext.VerifierIDs; a test-server DecisionFunc can set it (round-trip test over HTTP).
Fail closed. Nothing is emitted for a deny, when the matched profile's validation failed (normally only a warning, so it is checked explicitly), or when the identifier is not verified. dns/uri re-run VerifyLeafBinding inside the enrichment (so correctness does not depend on the calling registry having done it); an unverified client_id is never echoed; wildcard client_ids are not identifiers. DID/OIDF entries are added only on the key-bound allow path (not resolution-only), and only when the resolved document id / chain leaf subject equals the requested identifier.
Canonicalised once, in the PDP.dns: ASCII lower case, no trailing dot. uri: byte-for-byte (the match is already exact; nothing goes through net/url). did: fragment dropped, nothing else (path part is case-sensitive). oidf_entity: byte-for-byte as in the chain. semantic: semantic_id: + value verbatim.
semantic is validated against ^[A-Z]{3}[A-Z]{2}-<value>$ (EN 319 412-1 5.1.3) and its type must fit the subject.
Finding: natural-person classification in wrpac.go (narrow fix)
WRPACProfile.ExtractIdentity read organizationIdentifier with a serialNumber fallback and called the certificate a legal person whenever Organization was set. For a natural-person certificate (NCP-n / QCP-n, ETSI TS 119 475 5.1.5) the semantic identifier legitimately lives in serialNumber (e.g. PNOSE-...), so it was relabelled organization_identifier, and a natural person with an affiliated organisation was classified legal_person. Fixed narrowly: when the WRPAC policy OIDs say natural person, serialNumber is not treated as an organisation identifier and the type is natural_person; behaviour is unchanged for legal-person and policy-less certificates. New rpcert.SemanticIdentifier encodes where the value lives per subject kind: legal person reads 2.5.4.97 (serialNumber fallback for legacy certs and fixtures, and an invalid 2.5.4.97 is not papered over by falling back), natural person reads serialNumber only. Natural-person types (PNO IDC PAS TAX TIN) are refused for legal-person certificates and vice versa. A serialNumber that is not a semantic identifier is never emitted.
Decisions where the issue was ambiguous
Policy option lives under constraints (registry-agnostic: it covers x5c, DID and OIDF), not etsi.
Intermediary requests: semantic is taken from the RP leaf (the request's x5c is the end RP's; the intermediary chain is unvalidated and never read). dns/uri are omitted because the client_id of a brokered request cannot be shown to name the end RP.
A type with no entry for a given response is simply absent; did_scid, service, spki_sha256 are rejected in config until implemented, so a typo cannot be mistaken for a deferred type that silently returns nothing.
Aggregating strategies (all) and composite registries build a new response and do not carry the member; first_match and sequential return the registry's own response and do. Documented.
oidf_entity: extractEntityID trims a trailing / for resolution, so the chain leaf subject is compared with the raw requested identifier instead; a request that names the entity with a trailing slash still resolves but gets no identifier.
Not done / not verified
Deferred per scope: did_scid, service, spki_sha256. Hence Refs, not Closes.
Registration certificate is not an input (as per the issue); documented.
The real round trip through go-wallet-backend (open question in the issue) is not verified here; the Go client/test-server round trip is.
No didwebvh end-to-end success test (a valid DID log fixture is heavy); the one-line call site is the same helper as the other three DID registries, which are tested.
Swagger was patched by hand to the new definitions: regenerating with swag pulled in unrelated drift in the committed docs, so only the verifier_ids additions are in the diff.
Tests
New tests for each acceptance item: member present/absent/off; canonical form and basis per type; intermediary; no emission on deny and on failed profile validation; natural-person form and non-semantic serialNumber; hint narrowing (incl. through RegistryManager, and that a client cannot set return_verifier_ids); config rejection; client/test-server round trip. Each was checked to be non-vacuous by breaking the behaviour and confirming the test fails (28 mutations; one, re-serialising a verified URI through net/url, is an equivalent mutant because URISANMatches already compares u.String()). golangci-lint clean; full go test ./... passes with GOEXPERIMENT=jsonv2.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Contradictory certificate policies can bypass fail-closed classification, natural-person identity extraction can discard the personal serial, and programmatic policy installation remains unvalidated.
Compare chain subject with the raw requested identifier
pkg/registry/oidfed/oidfed_registry.go:875
This compares the chain subject with entityID after extractEntityID has stripped trailing slashes, not with the identifier actually named by the request. For example, a request for https://rp.example/ can resolve a chain whose leaf is https://rp.example and emit that identifier even though the documented fail-closed rule requires byte-for-byte equality with the requested identifier. Preserve the raw subject/resource ID for this final equality check; normalization may still be used for resolution.
Addressed the finding from the second Copilot review (it was only in the review body, not an inline thread): in dcf6106 the oidf_entity identifier is now compared with the raw requested identifier rather than the slash-stripped one, with a test (TestEvaluate_VerifierIDs_OIDFEntity, trailing-slash case) shown to fail when the old comparison is restored. The same commit splits two functions that Sonar flagged for cognitive complexity.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Malformed organization identifiers can still incorrectly fall back to serialNumber, violating fail-closed semantics.
0 open findings
Files not reviewed (1)
docs/swagger/docs.go: Generated file
Previously missed (1)
In code that hasn't changed since last review
Prevent serialNumber fallback for malformed organization identifiers
pkg/registry/rpcert/semanticid.go:122
This still falls back to serialNumber when 2.5.4.97 is present but empty or has a non-string ASN.1 value, because realOrganizationIdentifier returns "" for both malformed and absent attributes. Such values survive x509.ParseCertificate, so a malformed organization identifier can be papered over by a valid semantic ID in serialNumber, contrary to the fail-closed rule. Track attribute presence separately and use the legacy fallback only when 2.5.4.97 is absent.
Addressed the finding from the third Copilot review (again review-body only) in the follow-up commit: SemanticIdentifier now tracks whether 2.5.4.97 is present, so an empty or non-string organizationIdentifier is treated as malformed and not papered over by serialNumber (test TestSemanticIdentifier_MalformedOrganizationIdentifierDoesNotFallBack, shown to fail when the old fallback is restored).
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Security-sensitive identity derivation now spans several trust mechanisms and warrants final human validation.
0 open findings
Files not reviewed (1)
docs/swagger/docs.go: Generated file
Previously missed (1)
In code that hasn't changed since last review
Document best_match VerifierIDs loss in compatibility note
README.md:850
best_match also delegates to evaluateAllWithPolicy, whose newly built response drops VerifierIDs, but this compatibility note names only all. Operators using best_match may therefore expect the member to survive; list it with the non-carrying strategies.
Fourth Copilot review (body only): README now lists best_match with the strategies that do not carry verifier_ids (it delegates to the aggregating strategy). Also removed the duplicated http/https literals Sonar flagged (S1192).
Add an optional, informational context member verifier_ids: a list of
{verifier_id_type, verifier_id, basis} entries the PDP derives from
authenticated material, so a wallet answering a circuit_zk request with a
pseudonym function does not have to parse certificates, resolve DIDs or walk
federation chains itself.
Off by default (policy constraints.return_verifier_ids; unknown type names
are rejected at config load). A request context member verifier_id_type
narrows the response to one type and can never enable one.
Types: semantic (access certificate only), dns and uri (x509_san_dns /
x509_san_uri, only after VerifyLeafBinding), did (resolved document + key
match), oidf_entity (validated trust chain + key match). did_scid, service
and spki_sha256 are deferred.
Fail closed: nothing is emitted for a deny, when profile validation failed,
or when the identifier was not verified. An unverified client_id is never
echoed. semantic is validated against the semantic-id syntax and its type
must fit the subject; a serialNumber that is not a semantic identifier is not
emitted. For intermediary requests semantic comes from the RP leaf and dns/uri
are omitted.
Narrow fix in WRPACProfile.ExtractIdentity: a natural-person certificate
(NCP-n/QCP-n) keeps its serialNumber as the person's identifier (ETSI TS 119
475 5.1.5) instead of relabelling it organization_identifier and classifying
the certificate legal_person when it names an affiliated organisation.
Refs #203
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VS8tGFQncGyEhFwkQecjPB
- Contradictory WRPAC policy OIDs (natural and legal) are now their own
SubjectKind and SemanticIdentifier refuses them, instead of treating them
like a policy-less certificate.
- WRPACProfile.ExtractIdentity keeps a natural person's serialNumber when the
certificate also carries an affiliated organisation's 2.5.4.97, and exposes
both.
- PolicyManager.RegisterPolicy and SetDefaultPolicy validate the constraints
and return an error, so the programmatic installation path rejects unknown
return_verifier_ids types as the config path does.
Refs #203
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VS8tGFQncGyEhFwkQecjPB
extractEntityID strips trailing slashes for resolution, so comparing the
chain's leaf subject with it could emit an identifier that differs from what
the request named. Compare against the raw requested identifier instead.
Also split deriveX5CVerifierIDs and ExtractIdentity's subject_type decision
into helpers to bring their cognitive complexity under the Sonar threshold.
Refs #203
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VS8tGFQncGyEhFwkQecjPB
…nIdentifier
SemanticIdentifier used the serialNumber fallback whenever the 2.5.4.97 value
was empty, which also covered an attribute that is present but empty or not a
string. Track presence separately so only an absent attribute falls back.
Refs #203
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VS8tGFQncGyEhFwkQecjPB
… URL prefix check
best_match delegates to the aggregating strategy and builds a new response, so
it does not carry verifier_ids; say so in the README. Replace the repeated
http/https prefix literals in the oidfed registry with a helper (Sonar S1192).
Refs #203
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VS8tGFQncGyEhFwkQecjPB
#210 removed the keyUsage/contentCommitment check, so noKU no longer
fails validation. Use the contact-information check instead.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Per the trust-framework alignment on #203, type names follow the
OpenID4VP client_id scheme names; semantic is the only exception:
dns -> x509_san_dns, uri -> x509_san_uri,
did -> decentralized_identifier, oidf_entity -> openid_federation
No alias for the old names: #204 is unreleased, and unknown names are
still rejected in return_verifier_ids. Value forms are unchanged.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
With neither a natural- nor a legal-person WRPAC policy OID, an
organizationIdentifier could be a natural person's affiliation rather
than the verifier, so SemanticIdentifier now returns none (fail closed).
Every WRPAC carries one of the four -n/-l policy OIDs, so real access
certificates are unaffected.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
CanonicalDNSName now returns (value, ok): ASCII A-label form, ASCII
lower case, one trailing dot removed, and refuses non-ASCII (no U-label
conversion), empty labels, over-long labels/names, whitespace and
wildcards, so nothing is emitted for them. The check is explicit rather
than left to the SAN binding. A client_id that binds only through a
wildcard SAN no longer yields an identifier.
Shared vectors in pkg/registry/testdata for the SDKs. The binding stays
exact, since DNSSANMatches is also the trust-evaluation path.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…cs (#203)
DNSSANMatches compares ASCII case-insensitively (DNS names are), so a
client_id differing from the SAN only in case binds and yields the same
lower-case verifier_id; the emitted value matches the match. Only ASCII
letters are folded: a U-label never matches and nothing is converted.
Internationalised names work via their A-label. The wildcard branch also
no longer matches an empty label (.example.com against *.example.com).
DNSSANMatches is also the ETSI trust-evaluation binding, so this widens
it only to hostnames that differ in case.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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
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.
Summary
Refs #203. Implements the first-implementation scope of #203: an optional
verifier_idsmember in the AuthZEN evaluation responsecontext, so a wallet that answers acircuit_zkrequest with the pseudonym function gets a verified identifier string for the relying party without parsing certificates, resolving DIDs or walking federation chains.constraints.return_verifier_ids: [semantic, dns]. Existing responses and tests are unchanged. Unknown (and deferred) type names are rejected at config load (Config.Validate) and again when the policy is installed.context.verifier_id_typenarrows the response to one type. It only narrows:return_verifier_idsis a server-side policy control and is stripped from client context like the other controls; the hint is added to the client-suppliable allowlist.semantic(access certificate only),dns/uri(afterVerifyLeafBinding),did(didweb,didwebvh,didjwks, genericdid; resolved + key matched),oidf_entity(validated chain + key matched).authzenclientexposes it throughauthzen.EvaluationResponseContext.VerifierIDs; a test-serverDecisionFunccan set it (round-trip test over HTTP).docs/CONFIGURATION.md(regenerated), README section,example/config.yaml.Rules, as implemented
dns/urire-runVerifyLeafBindinginside the enrichment (so correctness does not depend on the calling registry having done it); an unverifiedclient_idis never echoed; wildcardclient_ids are not identifiers. DID/OIDF entries are added only on the key-bound allow path (not resolution-only), and only when the resolved document id / chain leaf subject equals the requested identifier.dns: ASCII lower case, no trailing dot.uri: byte-for-byte (the match is already exact; nothing goes throughnet/url).did: fragment dropped, nothing else (path part is case-sensitive).oidf_entity: byte-for-byte as in the chain.semantic:semantic_id:+ value verbatim.semanticis validated against^[A-Z]{3}[A-Z]{2}-<value>$(EN 319 412-1 5.1.3) and its type must fit the subject.Finding: natural-person classification in
wrpac.go(narrow fix)WRPACProfile.ExtractIdentityreadorganizationIdentifierwith aserialNumberfallback and called the certificate a legal person wheneverOrganizationwas set. For a natural-person certificate (NCP-n / QCP-n, ETSI TS 119 475 5.1.5) the semantic identifier legitimately lives inserialNumber(e.g.PNOSE-...), so it was relabelledorganization_identifier, and a natural person with an affiliated organisation was classifiedlegal_person. Fixed narrowly: when the WRPAC policy OIDs say natural person,serialNumberis not treated as an organisation identifier and the type isnatural_person; behaviour is unchanged for legal-person and policy-less certificates. Newrpcert.SemanticIdentifierencodes where the value lives per subject kind: legal person reads 2.5.4.97 (serialNumber fallback for legacy certs and fixtures, and an invalid 2.5.4.97 is not papered over by falling back), natural person readsserialNumberonly. Natural-person types (PNO IDC PAS TAX TIN) are refused for legal-person certificates and vice versa. AserialNumberthat is not a semantic identifier is never emitted.Decisions where the issue was ambiguous
constraints(registry-agnostic: it covers x5c, DID and OIDF), notetsi.semanticis taken from the RP leaf (the request's x5c is the end RP's; the intermediary chain is unvalidated and never read).dns/uriare omitted because theclient_idof a brokered request cannot be shown to name the end RP.did_scid,service,spki_sha256are rejected in config until implemented, so a typo cannot be mistaken for a deferred type that silently returns nothing.all) and composite registries build a new response and do not carry the member;first_matchandsequentialreturn the registry's own response and do. Documented.oidf_entity:extractEntityIDtrims a trailing/for resolution, so the chain leaf subject is compared with the raw requested identifier instead; a request that names the entity with a trailing slash still resolves but gets no identifier.Not done / not verified
did_scid,service,spki_sha256. HenceRefs, notCloses.didwebvhend-to-end success test (a valid DID log fixture is heavy); the one-line call site is the same helper as the other three DID registries, which are tested.swagpulled in unrelated drift in the committed docs, so only the verifier_ids additions are in the diff.Tests
New tests for each acceptance item: member present/absent/off; canonical form and basis per type; intermediary; no emission on deny and on failed profile validation; natural-person form and non-semantic
serialNumber; hint narrowing (incl. throughRegistryManager, and that a client cannot setreturn_verifier_ids); config rejection; client/test-server round trip. Each was checked to be non-vacuous by breaking the behaviour and confirming the test fails (28 mutations; one, re-serialising a verified URI throughnet/url, is an equivalent mutant becauseURISANMatchesalready comparesu.String()).golangci-lintclean; fullgo test ./...passes withGOEXPERIMENT=jsonv2.🤖 Generated with Claude Code
https://claude.ai/code/session_01VS8tGFQncGyEhFwkQecjPB