From be2a65ec9824c4b3a7bffeac217f1848149f8610 Mon Sep 17 00:00:00 2001 From: forkimenjeckayang Date: Fri, 24 Jul 2026 12:19:18 +0100 Subject: [PATCH 1/2] Reject mismatched SD-JWT signature algorithms Signed-off-by: forkimenjeckayang --- .../java/org/keycloak/sdjwt/JwsToken.java | 7 +++++ .../keycloak/sdjwt/SdJwtVerificationTest.java | 22 +++++++++++++ .../java/org/keycloak/sdjwt/TestSettings.java | 26 ++++++++++++++++ .../sdjwtvp/SdJwtVPVerificationTest.java | 31 ++++++++++++++++++- 4 files changed, 85 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/keycloak/sdjwt/JwsToken.java b/core/src/main/java/org/keycloak/sdjwt/JwsToken.java index 4e102fff9cea..0f4ecc58efa9 100644 --- a/core/src/main/java/org/keycloak/sdjwt/JwsToken.java +++ b/core/src/main/java/org/keycloak/sdjwt/JwsToken.java @@ -78,6 +78,13 @@ public String sign(SignatureSignerContext signerContext) { public void verifySignature(SignatureVerifierContext verifier) throws VerificationException { Objects.requireNonNull(verifier, "verifier must not be null"); + String headerAlgorithm = jwsHeader == null ? null : jwsHeader.getRawAlgorithm(); + String verifierAlgorithm = verifier.getAlgorithm(); + if (headerAlgorithm == null || verifierAlgorithm == null || !headerAlgorithm.equals(verifierAlgorithm)) { + throw new VerificationException(String.format( + "JWS header algorithm '%s' does not match verifier algorithm '%s'", + headerAlgorithm, verifierAlgorithm)); + } try { if (!verifier.verify(jwsInput.getEncodedSignatureInput().getBytes(StandardCharsets.UTF_8), jwsInput.getSignature())) { diff --git a/core/src/test/java/org/keycloak/sdjwt/SdJwtVerificationTest.java b/core/src/test/java/org/keycloak/sdjwt/SdJwtVerificationTest.java index 075325cc3f42..177a1eca21df 100644 --- a/core/src/test/java/org/keycloak/sdjwt/SdJwtVerificationTest.java +++ b/core/src/test/java/org/keycloak/sdjwt/SdJwtVerificationTest.java @@ -25,6 +25,7 @@ import org.keycloak.OID4VCConstants; import org.keycloak.common.VerificationException; import org.keycloak.common.util.Time; +import org.keycloak.crypto.Algorithm; import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.crypto.SignatureVerifierContext; import org.keycloak.jose.jws.JWSHeader; @@ -81,6 +82,27 @@ public void testSdJwtVerification_FlatSdJwt() throws VerificationException { } } + @Test + public void sdJwtVerificationShouldFail_WhenHeaderAlgorithmDiffersFromVerifierAlgorithm() { + SdJwt sdJwt = SdJwt.builder() + .withIssuerSignedJwt(exampleFlatSdJwtV1().build()) + .withIssuerSigningContext(TestSettings.signerWithReportedAlgorithm( + testSettings.issuerSigContext, Algorithm.ES384)) + .build(); + + assertEquals(Algorithm.ES384, sdJwt.getIssuerSignedJWT().getJwsHeader().getRawAlgorithm()); + assertEquals(Algorithm.ES256, testSettings.issuerVerifierContext.getAlgorithm()); + + VerificationException exception = assertThrows( + VerificationException.class, + () -> sdJwt.verify( + defaultIssuerVerifyingKeys(), + optionalTimeClaimVerificationOpts().build()) + ); + + assertEquals("Invalid Issuer-Signed JWT: Signature could not be verified", exception.getMessage()); + } + @Test public void testSdJwtVerification_EnforceIdempotence() throws VerificationException { IssuerSignedJWT issuerSignedJWT = exampleFlatSdJwtV1().build(); diff --git a/core/src/test/java/org/keycloak/sdjwt/TestSettings.java b/core/src/test/java/org/keycloak/sdjwt/TestSettings.java index 005e1eb3215b..a838bb6558eb 100644 --- a/core/src/test/java/org/keycloak/sdjwt/TestSettings.java +++ b/core/src/test/java/org/keycloak/sdjwt/TestSettings.java @@ -34,6 +34,7 @@ import org.keycloak.crypto.ECDSASignatureVerifierContext; import org.keycloak.crypto.KeyUse; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.SignatureException; import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.crypto.SignatureVerifierContext; @@ -79,6 +80,31 @@ public SignatureVerifierContext getHolderVerifierContext() { return holderVerifierContext; } + public static SignatureSignerContext signerWithReportedAlgorithm(SignatureSignerContext delegate, + String reportedAlgorithm) { + return new SignatureSignerContext() { + @Override + public String getKid() { + return delegate.getKid(); + } + + @Override + public String getAlgorithm() { + return reportedAlgorithm; + } + + @Override + public String getHashAlgorithm() { + return delegate.getHashAlgorithm(); + } + + @Override + public byte[] sign(byte[] data) throws SignatureException { + return delegate.sign(data); + } + }; + } + // private constructor private TestSettings() { JsonNode testSettings = TestUtils.readClaimSet(getClass(), "sdjwt/test-settings.json"); diff --git a/core/src/test/java/org/keycloak/sdjwt/sdjwtvp/SdJwtVPVerificationTest.java b/core/src/test/java/org/keycloak/sdjwt/sdjwtvp/SdJwtVPVerificationTest.java index aae18aecc28f..97b67c4df272 100644 --- a/core/src/test/java/org/keycloak/sdjwt/sdjwtvp/SdJwtVPVerificationTest.java +++ b/core/src/test/java/org/keycloak/sdjwt/sdjwtvp/SdJwtVPVerificationTest.java @@ -24,6 +24,8 @@ import org.keycloak.OID4VCConstants; import org.keycloak.common.VerificationException; import org.keycloak.common.util.Time; +import org.keycloak.crypto.Algorithm; +import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.crypto.SignatureVerifierContext; import org.keycloak.rule.CryptoInitRule; import org.keycloak.sdjwt.IssuerSignedJwtVerificationOpts; @@ -187,6 +189,28 @@ public void testShouldFail_IfKeyBindingJwtSignatureInvalid() { ); } + @Test + public void testShouldFail_IfKeyBindingHeaderAlgorithmDiffersFromVerifierAlgorithm() { + SignatureSignerContext signer = TestSettings.signerWithReportedAlgorithm( + testSettings.holderSigContext, Algorithm.ES384); + SdJwtVP sdJwtVP = exampleSdJwtWithCustomKbPayload(exampleKbPayload(), signer); + + assertEquals(Algorithm.ES384, + sdJwtVP.getKeyBindingJWT().orElseThrow(AssertionError::new).getJwsHeader().getRawAlgorithm()); + + VerificationException exception = assertThrows( + VerificationException.class, + () -> sdJwtVP.verify( + defaultIssuerVerifyingKeys(), + defaultIssuerSignedJwtVerificationOpts().build(), + defaultKeyBindingJwtVerificationOpts().build()) + ); + + assertEquals("Key binding JWT invalid", exception.getMessage()); + assertEquals("JWS header algorithm 'ES384' does not match verifier algorithm 'ES256'", + exception.getCause().getMessage()); + } + @Test public void testShouldFail_IfNoCnfClaim() { testShouldFailGeneric( @@ -512,9 +536,14 @@ private ObjectNode exampleKbPayload() { } private SdJwtVP exampleSdJwtWithCustomKbPayload(ObjectNode kbPayloadSubstitute) { + return exampleSdJwtWithCustomKbPayload(kbPayloadSubstitute, testSettings.holderSigContext); + } + + private SdJwtVP exampleSdJwtWithCustomKbPayload(ObjectNode kbPayloadSubstitute, + SignatureSignerContext signerContext) { KeyBindingJWT keyBindingJWT = KeyBindingJWT.builder() .withPayload(kbPayloadSubstitute) - .withSignerContext(testSettings.holderSigContext) + .withSignerContext(signerContext) .build(); String sdJwtVPString = TestUtils.readFileAsString(getClass(), "sdjwt/s20.1-sdjwt+kb.txt"); From 53b590fcdd6ade5e66df63d1f91d7754d02218cd Mon Sep 17 00:00:00 2001 From: forkimenjeckayang Date: Fri, 24 Jul 2026 13:02:32 +0100 Subject: [PATCH 2/2] address copilot reviews Signed-off-by: forkimenjeckayang --- .../java/org/keycloak/sdjwt/JwsToken.java | 4 ++- .../keycloak/sdjwt/SdJwtVerificationTest.java | 20 +++++++++++++++ .../java/org/keycloak/sdjwt/TestUtils.java | 18 +++++++++++++ .../sdjwtvp/SdJwtVPVerificationTest.java | 25 +++++++++++++++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/keycloak/sdjwt/JwsToken.java b/core/src/main/java/org/keycloak/sdjwt/JwsToken.java index 0f4ecc58efa9..be9695fd4bc1 100644 --- a/core/src/main/java/org/keycloak/sdjwt/JwsToken.java +++ b/core/src/main/java/org/keycloak/sdjwt/JwsToken.java @@ -78,7 +78,9 @@ public String sign(SignatureSignerContext signerContext) { public void verifySignature(SignatureVerifierContext verifier) throws VerificationException { Objects.requireNonNull(verifier, "verifier must not be null"); - String headerAlgorithm = jwsHeader == null ? null : jwsHeader.getRawAlgorithm(); + String headerAlgorithm = jwsHeader == null || jwsHeader.getAlgorithm() == null + ? null + : jwsHeader.getRawAlgorithm(); String verifierAlgorithm = verifier.getAlgorithm(); if (headerAlgorithm == null || verifierAlgorithm == null || !headerAlgorithm.equals(verifierAlgorithm)) { throw new VerificationException(String.format( diff --git a/core/src/test/java/org/keycloak/sdjwt/SdJwtVerificationTest.java b/core/src/test/java/org/keycloak/sdjwt/SdJwtVerificationTest.java index 177a1eca21df..8df261af31af 100644 --- a/core/src/test/java/org/keycloak/sdjwt/SdJwtVerificationTest.java +++ b/core/src/test/java/org/keycloak/sdjwt/SdJwtVerificationTest.java @@ -103,6 +103,26 @@ public void sdJwtVerificationShouldFail_WhenHeaderAlgorithmDiffersFromVerifierAl assertEquals("Invalid Issuer-Signed JWT: Signature could not be verified", exception.getMessage()); } + @Test + public void sdJwtVerificationShouldFail_WhenHeaderAlgorithmIsMissing() { + SdJwt signedSdJwt = SdJwt.builder() + .withIssuerSignedJwt(exampleFlatSdJwtV1().build()) + .withIssuerSigningContext(testSettings.issuerSigContext) + .build(); + String jwsWithoutAlgorithm = TestUtils.removeAlgorithmFromJwsHeader( + signedSdJwt.getIssuerSignedJWT().getJws()); + SdJwt sdJwt = new SdJwt(new IssuerSignedJWT(jwsWithoutAlgorithm), null); + + VerificationException exception = assertThrows( + VerificationException.class, + () -> sdJwt.verify( + defaultIssuerVerifyingKeys(), + optionalTimeClaimVerificationOpts().build()) + ); + + assertEquals("Invalid Issuer-Signed JWT: Signature could not be verified", exception.getMessage()); + } + @Test public void testSdJwtVerification_EnforceIdempotence() throws VerificationException { IssuerSignedJWT issuerSignedJWT = exampleFlatSdJwtV1().build(); diff --git a/core/src/test/java/org/keycloak/sdjwt/TestUtils.java b/core/src/test/java/org/keycloak/sdjwt/TestUtils.java index 36bc686db94b..7658082a31f2 100644 --- a/core/src/test/java/org/keycloak/sdjwt/TestUtils.java +++ b/core/src/test/java/org/keycloak/sdjwt/TestUtils.java @@ -22,6 +22,8 @@ import java.io.InputStreamReader; import java.util.Objects; +import org.keycloak.common.util.Base64Url; + import com.fasterxml.jackson.databind.node.ObjectNode; /** @@ -61,4 +63,20 @@ public static String splitStringIntoLines(String input, int lineLength) { return result.toString(); } + public static String removeAlgorithmFromJwsHeader(String jws) { + String[] parts = jws.split("\\.", -1); + if (parts.length != 3) { + throw new IllegalArgumentException("Expected a compact JWS with three parts"); + } + + try { + ObjectNode header = (ObjectNode) SdJwtUtils.mapper.readTree(Base64Url.decode(parts[0])); + header.remove("alg"); + parts[0] = Base64Url.encode(SdJwtUtils.mapper.writeValueAsBytes(header)); + return String.join(".", parts); + } catch (IOException e) { + throw new IllegalArgumentException("Could not rewrite JWS header", e); + } + } + } diff --git a/core/src/test/java/org/keycloak/sdjwt/sdjwtvp/SdJwtVPVerificationTest.java b/core/src/test/java/org/keycloak/sdjwt/sdjwtvp/SdJwtVPVerificationTest.java index 97b67c4df272..16e066f2e055 100644 --- a/core/src/test/java/org/keycloak/sdjwt/sdjwtvp/SdJwtVPVerificationTest.java +++ b/core/src/test/java/org/keycloak/sdjwt/sdjwtvp/SdJwtVPVerificationTest.java @@ -211,6 +211,31 @@ public void testShouldFail_IfKeyBindingHeaderAlgorithmDiffersFromVerifierAlgorit exception.getCause().getMessage()); } + @Test + public void testShouldFail_IfKeyBindingHeaderAlgorithmIsMissing() { + KeyBindingJWT keyBindingJWT = KeyBindingJWT.builder() + .withPayload(exampleKbPayload()) + .withSignerContext(testSettings.holderSigContext) + .build(); + String sdJwtVPString = TestUtils.readFileAsString(getClass(), "sdjwt/s20.1-sdjwt+kb.txt"); + String sdJwtWithoutKb = sdJwtVPString.substring( + 0, sdJwtVPString.lastIndexOf(OID4VCConstants.SDJWT_DELIMITER) + 1); + SdJwtVP sdJwtVP = SdJwtVP.of(sdJwtWithoutKb + + TestUtils.removeAlgorithmFromJwsHeader(keyBindingJWT.getJws())); + + VerificationException exception = assertThrows( + VerificationException.class, + () -> sdJwtVP.verify( + defaultIssuerVerifyingKeys(), + defaultIssuerSignedJwtVerificationOpts().build(), + defaultKeyBindingJwtVerificationOpts().build()) + ); + + assertEquals("Key binding JWT invalid", exception.getMessage()); + assertEquals("JWS header algorithm 'null' does not match verifier algorithm 'ES256'", + exception.getCause().getMessage()); + } + @Test public void testShouldFail_IfNoCnfClaim() { testShouldFailGeneric(