nuts-foundation / nuts-foundation/nuts-node
VP JWT format and verification inconsistencies
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 28
- Forks
- 23
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 76
Description
While working on #4191 (JWTProfile refactor) we surfaced a number of inconsistencies in how Verifiable Presentations are constructed and verified in this codebase. None are immediate vulnerabilities, but together they make it hard to reason about VP security and complicate consistent verification.
Findings
1. iss is not set on VP JWTs (spec deviation)
vcr/holder/presenter.go:buildJWTPresentation puts the holder DID in the sub claim and omits iss. Per W3C VC Data Model 1.1 §6.3.1, for a JWT VP iss MUST be the holder; sub is not specified.
Consequence: the signer identity has to be derived from the kid header, which means the standard iss-to-kid binding check we use for VCs (in crypto.IssuerKidValidator) cannot be applied to VPs.
2. aud is not consistently required on verification
VP builders set aud:
- discovery: from
service.ID(viaDomain) - OpenID4VP: from
ClientIDParam(viaAudience)
But aud is only checked in the v2 OpenID4VP flow (validatePresentationAudience in auth/api/iam/validation.go). VerifyVP itself doesn't check it, so callers like discovery/module.go:269 and vcr/api/vcr/v2/api.go accept VPs regardless of audience. A VP intended for verifier A could be replayed against verifier B.
3. exp is not consistently required on verification
After #4191 the VP builder always sets exp (default 15 min), but the verifier still doesn't require it because other nodes may produce VPs without one. Caller-side max-age checks are inconsistent:
| Caller | Behavior when `exp` missing |
|---|---|
| `validateS2SPresentationMaxValidity` (s2s OpenID4VP) | rejects ("missing creation or expiration date") |
| `discovery/module.go:247` | silently passes (`time.Until(zeroTime)` is hugely negative) |
| `vcr/api/vcr/v2/api.go` (external API) | no check |
| `discovery/client.go:372` | no check |
| `auth/api/iam/openid4vp.go:539` | no check |
4. typ is "JWT" instead of "vp+jwt"
W3C VC Data Model 2.0 specifies typ: "vp+jwt" to prevent JWT type confusion. This was the root cause of GHSA-9hmg-827w-9rhj (VP replayed as access token). The fix for that advisory addressed the verifier side; updating the VP builder is out of scope for safety reasons (rolling-upgrade compatibility), but should be considered in a coordinated network upgrade.
Suggested actions
A consolidated fix would involve:
- Update
buildJWTPresentationto setiss(in addition tosub, for backward compat) andtyp: "vp+jwt". - Fix
discovery/module.goto properly reject VPs with missingexp(or usenbf + maxValidityas a fallback). - Add audience checks to all VP-accepting endpoints, not just
validatePresentationAudience. - Once the network has been upgraded, tighten the
vpJWTProfileto requireexp, validateiss-to-kidbinding, and rejecttyp != "vp+jwt".
Each step needs to be staged carefully to avoid breaking interop with older nodes.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with vcr/holder/presenter.go:buildJWTPresentation and compare its claims with auth/api/iam/validation.go:validatePresentationAudience. Then trace VP acceptance in discovery/module.go, vcr/api/vcr/v2/api.go, discovery/client.go, and auth/api/iam/openid4vp.go. Done means the staged verification behavior is consistent without breaking older-node interoperability.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100