Skip to content

[OID4VCI] Fix c_nonce replay protection#50120

Merged
mposolda merged 1 commit into
keycloak:mainfrom
adorsys:issue-48043
Jul 24, 2026
Merged

[OID4VCI] Fix c_nonce replay protection#50120
mposolda merged 1 commit into
keycloak:mainfrom
adorsys:issue-48043

Conversation

@forkimenjeckayang

Copy link
Copy Markdown
Contributor

Fixes OID4VCI c_nonce replay protection by marking verified proof nonces as consumed after successful credential request processing.

The nonce is stored in SingleUseObjectProvider using a hashed key, with the cache lifetime calculated from the remaining nonce validity plus clock skew. This prevents a captured proof JWT from being replayed within the nonce validity window.

Details

  • Add CNonceHandler.consumeCNonce() for explicit nonce consumption.
  • Implement single-use storage in JwtCNonceHandler with putIfAbsent().
  • Calculate cache TTL as exp - now + clockSkew.
  • Consume each distinct proof nonce once at the credential request boundary.
  • Keep verifyCNonce() side-effect free so multi-proof and key-attestation flows can validate the same nonce during one request.
  • Remove eager attestation nonce replay handling that used an incorrect TTL.
  • Add regression coverage for single-use nonce consumption.

closes #48043

@forkimenjeckayang
forkimenjeckayang requested a review from a team as a code owner June 18, 2026 11:37
Copilot AI review requested due to automatic review settings June 18, 2026 11:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes OID4VCI c_nonce replay protection by introducing explicit nonce consumption (single-use enforcement) after successful credential issuance, preventing replay of captured proof JWTs within the nonce validity window.

Changes:

  • Adds CNonceHandler.consumeCNonce() and implements it in JwtCNonceHandler using SingleUseObjectProvider.putIfAbsent() with TTL derived from remaining nonce validity plus clock skew.
  • Consumes distinct proof nonces once at the credential request boundary in OID4VCIssuerEndpoint (while keeping verifyCNonce() side-effect free for multi-proof flows).
  • Removes prior (incorrect) attestation nonce replay handling and adds regression coverage for consumption semantics.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/base/src/test/java/org/keycloak/tests/oid4vc/NonceEndpointTest.java Adds a regression test for single-use nonce consumption behavior (provider-level).
services/src/main/java/org/keycloak/protocol/oid4vc/issuance/OID4VCIssuerEndpoint.java Consumes proof c_nonce values once per successful credential request to block replay.
services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/JwtCNonceHandler.java Implements single-use consumption via SingleUseObjectProvider with hashed key and TTL.
services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/CNonceHandler.java Introduces the consumeCNonce() API (default throws when unsupported).
services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationProofValidator.java Removes earlier attestation-specific replay handling to centralize consumption at request boundary.

Comment thread tests/base/src/test/java/org/keycloak/tests/oid4vc/NonceEndpointTest.java Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

@Awambeng Awambeng left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Copilot AI review requested due to automatic review settings June 19, 2026 14:07
@dominikschlosser

Copy link
Copy Markdown
Contributor

@forkimenjeckayang You mention, that the implementation is kept side-effect free, so that "multi-proof and key-attestation flows can validate the same nonce during one request". But this not actually tested right now? I guess this is about batch issuance and key attestation/proof sharing the same nonce?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comment thread tests/base/src/test/java/org/keycloak/tests/oid4vc/NonceEndpointTest.java Outdated
@forkimenjeckayang

Copy link
Copy Markdown
Contributor Author

@forkimenjeckayang You mention, that the implementation is kept side-effect free, so that "multi-proof and key-attestation flows can validate the same nonce during one request". But this not actually tested right now? I guess this is about batch issuance and key attestation/proof sharing the same nonce?

Good point. I added an explicit assertion for the key-attestation case now.

The multi-proof case was already covered by testRequestMultipleCredentialsWithProofs, which sends two JWT proofs using the same c_nonce in one credential request.

For key attestation, testValidJwtProofWithKeyAttestation already used the same c_nonce in both the outer JWT proof and the key_attestation, but it did not assert the nonce tracking behavior explicitly. I added an assertion that the validated nonce is stored only once in VCIssuanceContext, so the test now covers the side-effect-free validation path more directly.

Copilot AI review requested due to automatic review settings June 22, 2026 07:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Copilot AI review requested due to automatic review settings June 22, 2026 08:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Comment thread tests/base/src/test/java/org/keycloak/tests/oid4vc/NonceEndpointTest.java Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment thread tests/base/src/test/java/org/keycloak/tests/oid4vc/NonceEndpointTest.java Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings June 26, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Closes keycloak#48043

Signed-off-by: forkimenjeckayang <forkimenjeckayang@gmail.com>
Signed-off-by: Ingrid Kamga <Ingrid.Kamga@adorsys.com>

@dominikschlosser dominikschlosser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@mposolda mposolda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@forkimenjeckayang @Awambeng @dominikschlosser Thanks for this PR and for the reviews!

@mposolda
mposolda merged commit d5756ef into keycloak:main Jul 24, 2026
89 checks passed
@Awambeng
Awambeng deleted the issue-48043 branch July 24, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[OID4VCI] c_nonce Replay

7 participants