Skip to content

[OID4VCI] Fix JWT proof validation gaps and attestation verification#47995

Merged
mposolda merged 2 commits intokeycloak:mainfrom
adorsys:issue-47513-2
Apr 23, 2026
Merged

[OID4VCI] Fix JWT proof validation gaps and attestation verification#47995
mposolda merged 2 commits intokeycloak:mainfrom
adorsys:issue-47513-2

Conversation

@forkimenjeckayang
Copy link
Copy Markdown
Contributor

@forkimenjeckayang forkimenjeckayang commented Apr 13, 2026

Fixes validation gaps in OID4VCI JWT proof handling by ensuring proper key resolution (jwk, kid, x5c), enforcing JOSE header constraints, and rejecting unsupported trust_chain.

Adds full verification of key_attestation, including signature, claims, and proof-key membership in attested_keys.

Strengthens JWT payload validation (issuer handling, single audience, iat freshness, optional exp/nbf, and nonce checks), with nonce failures mapped to invalid_nonce.

Also introduces trusted attestation key loading from the realm and expands test coverage for key resolution, attestation validation, and error scenarios.

closes #47513

@forkimenjeckayang forkimenjeckayang requested review from a team as code owners April 13, 2026 09:40
@forkimenjeckayang forkimenjeckayang mentioned this pull request Apr 13, 2026
Closed
@mposolda mposolda self-assigned this Apr 14, 2026
tdiesler
tdiesler approved these changes Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@tdiesler tdiesler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mposolda mposolda mentioned this pull request Apr 23, 2026
Merged
mposolda
mposolda reviewed Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forkimenjeckayang Thanks for the PR! Nice stuff! Sorry for late review.

@tdiesler Thanks as well for the review!

I've did also some review and testing of the PR. Found some very small things. I've sent you the PR to your branch adorsys#272 . If you merge (or apply those changes), this PR is good to me.

But I think there might be some follow-up work, which should not block this PR IMO, but will be good to address in the future 1 bigger thing and 2 smaller things (will create GH issues once this PR is merged):

  1. The realm attributes Trusted Key IDs and Trusted Keys (JSON) are not so ideal for figure the attestation keys. As attestation is rather something, which might be different between various wallets. For example wallet lissi, which might be represented by client lissi-client will likely use completely different attestation keys than wallet heidi (represented by heidi-client) . The keys might be rather figured either per client (similarly like for example client keys used for private_key_jwt client authentication) or from identity provider (similarly like for example CLIENT_AUTH_FEDERATED feature is doing)

  2. Hardcoded constant JWK_PRIVATE_KEY_CLAIMS is not very reliable. May not work in case some custom key providers are deployed or when we support more keys in the future (which can happen in the near future probably as Keycloak will need to add something due the post-quantum cryptography)

  3. Test OID4VCJWTIssuerEndpointTest is some combination of E2E test and "unit" test. It uses endpoints, but at the same time, still uses KeycloakSession to figure some things from the server directly. Also it seems some functionality inside this test is already tested in the context of other tests though...

keycloak-github-bot[bot]
keycloak-github-bot Bot reviewed Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown

@keycloak-github-bot keycloak-github-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreported flaky test detected, please review

@keycloak-github-bot
Copy link
Copy Markdown

keycloak-github-bot Bot commented Apr 23, 2026

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.authz.PolicyEvaluationTest#

Keycloak CI - Base IT (3)

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

org.opentest4j.AssertionFailedError: expected: <PERMIT> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
...

Report flaky test

mposolda
mposolda approved these changes Apr 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@forkimenjeckayang Thanks!

@mposolda mposolda merged commit c090f82 into keycloak:main Apr 23, 2026
85 checks passed
This was referenced Apr 23, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mposolda mposolda mposolda approved these changes

+2 more reviewers

@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot[bot] left review comments

@tdiesler tdiesler tdiesler approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mposolda mposolda

Labels

flaky-test team/cloud-native team/core-clients team/core-iam

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Review JwtProofValidator

3 participants

@forkimenjeckayang @tdiesler @mposolda