From 25511d4dbf009f305c5501e2d647fafda591a43f Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Fri, 1 Nov 2019 14:44:56 +0100 Subject: [PATCH] KEYCLOAK-9651 Wrong ECDSA signature R and S encoding --- .../ClientECDSASignatureVerifierContext.java | 36 +++++++ .../ECDSAClientSignatureVerifierProvider.java | 21 +++++ .../crypto/ECDSASignatureProvider.java | 94 +++++++++++++++++++ ...lientSignatureVerifierProviderFactory.java | 2 +- .../crypto/ES256SignatureProviderFactory.java | 2 +- ...lientSignatureVerifierProviderFactory.java | 2 +- .../crypto/ES384SignatureProviderFactory.java | 2 +- ...lientSignatureVerifierProviderFactory.java | 2 +- .../crypto/ES512SignatureProviderFactory.java | 2 +- ...erverAsymmetricSignatureSignerContext.java | 2 +- ...verAsymmetricSignatureVerifierContext.java | 2 +- .../ServerECDSASignatureSignerContext.java | 24 +++++ .../ServerECDSASignatureVerifierContext.java | 29 ++++++ ...stingOIDCEndpointsApplicationResource.java | 12 ++- .../keycloak/testsuite/util/OAuthClient.java | 37 +++++++- .../testsuite/oauth/AccessTokenTest.java | 38 ++++++++ .../oauth/ClientAuthSignedJWTTest.java | 52 ++++++++-- 17 files changed, 341 insertions(+), 18 deletions(-) create mode 100644 services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java create mode 100644 services/src/main/java/org/keycloak/crypto/ECDSAClientSignatureVerifierProvider.java create mode 100644 services/src/main/java/org/keycloak/crypto/ECDSASignatureProvider.java create mode 100644 services/src/main/java/org/keycloak/crypto/ServerECDSASignatureSignerContext.java create mode 100644 services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java diff --git a/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java b/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java new file mode 100644 index 00000000000..c9c96e82aa4 --- /dev/null +++ b/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java @@ -0,0 +1,36 @@ +package org.keycloak.crypto; + +import org.keycloak.common.VerificationException; +import org.keycloak.jose.jws.JWSInput; +import org.keycloak.keys.loader.PublicKeyStorageManager; +import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; + +public class ClientECDSASignatureVerifierContext extends AsymmetricSignatureVerifierContext { + public ClientECDSASignatureVerifierContext(KeycloakSession session, ClientModel client, JWSInput input) throws VerificationException { + super(getKey(session, client, input)); + } + + private static KeyWrapper getKey(KeycloakSession session, ClientModel client, JWSInput input) throws VerificationException { + KeyWrapper key = PublicKeyStorageManager.getClientPublicKeyWrapper(session, client, input); + if (key == null) { + throw new VerificationException("Key not found"); + } + return key; + } + + @Override + public boolean verify(byte[] data, byte[] signature) throws VerificationException { + try { + /* + Fallback for backwards compatibility of ECDSA signed tokens which were issued in previous versions. + TODO remove by https://issues.jboss.org/browse/KEYCLOAK-11911 + */ + int expectedSize = ECDSASignatureProvider.ECDSA.valueOf(getAlgorithm()).getSignatureLength(); + byte[] derSignature = expectedSize != signature.length && signature[0] == 0x30 ? signature : ECDSASignatureProvider.concatenatedRSToASN1DER(signature, expectedSize); + return super.verify(data, derSignature); + } catch (Exception e) { + throw new VerificationException("Signing failed", e); + } + } +} diff --git a/services/src/main/java/org/keycloak/crypto/ECDSAClientSignatureVerifierProvider.java b/services/src/main/java/org/keycloak/crypto/ECDSAClientSignatureVerifierProvider.java new file mode 100644 index 00000000000..8182e7e2801 --- /dev/null +++ b/services/src/main/java/org/keycloak/crypto/ECDSAClientSignatureVerifierProvider.java @@ -0,0 +1,21 @@ +package org.keycloak.crypto; + +import org.keycloak.common.VerificationException; +import org.keycloak.jose.jws.JWSInput; +import org.keycloak.models.ClientModel; +import org.keycloak.models.KeycloakSession; + +public class ECDSAClientSignatureVerifierProvider implements ClientSignatureVerifierProvider { + private final KeycloakSession session; + private final String algorithm; + + public ECDSAClientSignatureVerifierProvider(KeycloakSession session, String algorithm) { + this.session = session; + this.algorithm = algorithm; + } + + @Override + public SignatureVerifierContext verifier(ClientModel client, JWSInput input) throws VerificationException { + return new ClientECDSASignatureVerifierContext(session, client, input); + } +} diff --git a/services/src/main/java/org/keycloak/crypto/ECDSASignatureProvider.java b/services/src/main/java/org/keycloak/crypto/ECDSASignatureProvider.java new file mode 100644 index 00000000000..f9cf09bdb42 --- /dev/null +++ b/services/src/main/java/org/keycloak/crypto/ECDSASignatureProvider.java @@ -0,0 +1,94 @@ +package org.keycloak.crypto; + +import org.bouncycastle.asn1.ASN1InputStream; +import org.bouncycastle.asn1.ASN1Integer; +import org.bouncycastle.asn1.ASN1Primitive; +import org.bouncycastle.asn1.ASN1Sequence; +import org.bouncycastle.asn1.DERSequenceGenerator; +import org.bouncycastle.asn1.x9.X9IntegerConverter; +import org.keycloak.common.VerificationException; +import org.keycloak.models.KeycloakSession; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.math.BigInteger; + +public class ECDSASignatureProvider implements SignatureProvider { + + private final KeycloakSession session; + private final String algorithm; + + public ECDSASignatureProvider(KeycloakSession session, String algorithm) { + this.session = session; + this.algorithm = algorithm; + } + + @Override + public SignatureSignerContext signer() throws SignatureException { + return new ServerECDSASignatureSignerContext(session, algorithm); + } + + @Override + public SignatureVerifierContext verifier(String kid) throws VerificationException { + return new ServerECDSASignatureVerifierContext(session, kid, algorithm); + } + + public static byte[] concatenatedRSToASN1DER(final byte[] signature, int signLength) throws IOException { + int len = signLength / 2; + int arraySize = len + 1; + + byte[] r = new byte[arraySize]; + byte[] s = new byte[arraySize]; + System.arraycopy(signature, 0, r, 1, len); + System.arraycopy(signature, len, s, 1, len); + BigInteger rBigInteger = new BigInteger(r); + BigInteger sBigInteger = new BigInteger(s); + + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + DERSequenceGenerator seqGen = new DERSequenceGenerator(bos); + + seqGen.addObject(new ASN1Integer(rBigInteger.toByteArray())); + seqGen.addObject(new ASN1Integer(sBigInteger.toByteArray())); + seqGen.close(); + bos.close(); + + return bos.toByteArray(); + } + + public static byte[] asn1derToConcatenatedRS(final byte[] derEncodedSignatureValue, int signLength) throws IOException { + int len = signLength / 2; + + ASN1InputStream asn1InputStream = new ASN1InputStream(derEncodedSignatureValue); + ASN1Primitive asn1Primitive = asn1InputStream.readObject(); + asn1InputStream.close(); + + ASN1Sequence asn1Sequence = (ASN1Sequence.getInstance(asn1Primitive)); + ASN1Integer rASN1 = (ASN1Integer) asn1Sequence.getObjectAt(0); + ASN1Integer sASN1 = (ASN1Integer) asn1Sequence.getObjectAt(1); + X9IntegerConverter x9IntegerConverter = new X9IntegerConverter(); + byte[] r = x9IntegerConverter.integerToBytes(rASN1.getValue(), len); + byte[] s = x9IntegerConverter.integerToBytes(sASN1.getValue(), len); + + byte[] concatenatedSignatureValue = new byte[signLength]; + System.arraycopy(r, 0, concatenatedSignatureValue, 0, len); + System.arraycopy(s, 0, concatenatedSignatureValue, len, len); + + return concatenatedSignatureValue; + } + + public enum ECDSA { + ES256(64), + ES384(96), + ES512(132); + + private final int signatureLength; + + ECDSA(int signatureLength) { + this.signatureLength = signatureLength; + } + + public int getSignatureLength() { + return this.signatureLength; + } + } +} diff --git a/services/src/main/java/org/keycloak/crypto/ES256ClientSignatureVerifierProviderFactory.java b/services/src/main/java/org/keycloak/crypto/ES256ClientSignatureVerifierProviderFactory.java index c03f040652b..2fb432a3f93 100644 --- a/services/src/main/java/org/keycloak/crypto/ES256ClientSignatureVerifierProviderFactory.java +++ b/services/src/main/java/org/keycloak/crypto/ES256ClientSignatureVerifierProviderFactory.java @@ -29,7 +29,7 @@ public class ES256ClientSignatureVerifierProviderFactory implements ClientSignat @Override public ClientSignatureVerifierProvider create(KeycloakSession session) { - return new AsymmetricClientSignatureVerifierProvider(session, Algorithm.ES256); + return new ECDSAClientSignatureVerifierProvider(session, Algorithm.ES256); } } diff --git a/services/src/main/java/org/keycloak/crypto/ES256SignatureProviderFactory.java b/services/src/main/java/org/keycloak/crypto/ES256SignatureProviderFactory.java index 6062b2dd390..66d5bfc79ef 100644 --- a/services/src/main/java/org/keycloak/crypto/ES256SignatureProviderFactory.java +++ b/services/src/main/java/org/keycloak/crypto/ES256SignatureProviderFactory.java @@ -29,7 +29,7 @@ public class ES256SignatureProviderFactory implements SignatureProviderFactory { @Override public SignatureProvider create(KeycloakSession session) { - return new AsymmetricSignatureProvider(session, Algorithm.ES256); + return new ECDSASignatureProvider(session, Algorithm.ES256); } } diff --git a/services/src/main/java/org/keycloak/crypto/ES384ClientSignatureVerifierProviderFactory.java b/services/src/main/java/org/keycloak/crypto/ES384ClientSignatureVerifierProviderFactory.java index 715daf5c77a..4e04078e854 100644 --- a/services/src/main/java/org/keycloak/crypto/ES384ClientSignatureVerifierProviderFactory.java +++ b/services/src/main/java/org/keycloak/crypto/ES384ClientSignatureVerifierProviderFactory.java @@ -29,7 +29,7 @@ public class ES384ClientSignatureVerifierProviderFactory implements ClientSignat @Override public ClientSignatureVerifierProvider create(KeycloakSession session) { - return new AsymmetricClientSignatureVerifierProvider(session, Algorithm.ES384); + return new ECDSAClientSignatureVerifierProvider(session, Algorithm.ES384); } } diff --git a/services/src/main/java/org/keycloak/crypto/ES384SignatureProviderFactory.java b/services/src/main/java/org/keycloak/crypto/ES384SignatureProviderFactory.java index 4a48c2d4c50..f8a588dd4d3 100644 --- a/services/src/main/java/org/keycloak/crypto/ES384SignatureProviderFactory.java +++ b/services/src/main/java/org/keycloak/crypto/ES384SignatureProviderFactory.java @@ -29,7 +29,7 @@ public class ES384SignatureProviderFactory implements SignatureProviderFactory { @Override public SignatureProvider create(KeycloakSession session) { - return new AsymmetricSignatureProvider(session, Algorithm.ES384); + return new ECDSASignatureProvider(session, Algorithm.ES384); } } diff --git a/services/src/main/java/org/keycloak/crypto/ES512ClientSignatureVerifierProviderFactory.java b/services/src/main/java/org/keycloak/crypto/ES512ClientSignatureVerifierProviderFactory.java index cb766570183..8ee6d8afcdd 100644 --- a/services/src/main/java/org/keycloak/crypto/ES512ClientSignatureVerifierProviderFactory.java +++ b/services/src/main/java/org/keycloak/crypto/ES512ClientSignatureVerifierProviderFactory.java @@ -29,7 +29,7 @@ public class ES512ClientSignatureVerifierProviderFactory implements ClientSigna @Override public ClientSignatureVerifierProvider create(KeycloakSession session) { - return new AsymmetricClientSignatureVerifierProvider(session, Algorithm.ES512); + return new ECDSAClientSignatureVerifierProvider(session, Algorithm.ES512); } } diff --git a/services/src/main/java/org/keycloak/crypto/ES512SignatureProviderFactory.java b/services/src/main/java/org/keycloak/crypto/ES512SignatureProviderFactory.java index 6762433dffe..e460b91149c 100644 --- a/services/src/main/java/org/keycloak/crypto/ES512SignatureProviderFactory.java +++ b/services/src/main/java/org/keycloak/crypto/ES512SignatureProviderFactory.java @@ -29,7 +29,7 @@ public class ES512SignatureProviderFactory implements SignatureProviderFactory { @Override public SignatureProvider create(KeycloakSession session) { - return new AsymmetricSignatureProvider(session, Algorithm.ES512); + return new ECDSASignatureProvider(session, Algorithm.ES512); } } diff --git a/services/src/main/java/org/keycloak/crypto/ServerAsymmetricSignatureSignerContext.java b/services/src/main/java/org/keycloak/crypto/ServerAsymmetricSignatureSignerContext.java index d16a73a8981..14b33f6d00a 100644 --- a/services/src/main/java/org/keycloak/crypto/ServerAsymmetricSignatureSignerContext.java +++ b/services/src/main/java/org/keycloak/crypto/ServerAsymmetricSignatureSignerContext.java @@ -24,7 +24,7 @@ public class ServerAsymmetricSignatureSignerContext extends AsymmetricSignatureS super(getKey(session, algorithm)); } - private static KeyWrapper getKey(KeycloakSession session, String algorithm) { + static KeyWrapper getKey(KeycloakSession session, String algorithm) { KeyWrapper key = session.keys().getActiveKey(session.getContext().getRealm(), KeyUse.SIG, algorithm); if (key == null) { throw new SignatureException("Active key for " + algorithm + " not found"); diff --git a/services/src/main/java/org/keycloak/crypto/ServerAsymmetricSignatureVerifierContext.java b/services/src/main/java/org/keycloak/crypto/ServerAsymmetricSignatureVerifierContext.java index c8242e7b7e5..7809d2deb60 100644 --- a/services/src/main/java/org/keycloak/crypto/ServerAsymmetricSignatureVerifierContext.java +++ b/services/src/main/java/org/keycloak/crypto/ServerAsymmetricSignatureVerifierContext.java @@ -25,7 +25,7 @@ public class ServerAsymmetricSignatureVerifierContext extends AsymmetricSignatur super(getKey(session, kid, algorithm)); } - private static KeyWrapper getKey(KeycloakSession session, String kid, String algorithm) throws VerificationException { + static KeyWrapper getKey(KeycloakSession session, String kid, String algorithm) throws VerificationException { KeyWrapper key = session.keys().getKey(session.getContext().getRealm(), kid, KeyUse.SIG, algorithm); if (key == null) { throw new VerificationException("Key not found"); diff --git a/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureSignerContext.java b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureSignerContext.java new file mode 100644 index 00000000000..9758e2b784b --- /dev/null +++ b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureSignerContext.java @@ -0,0 +1,24 @@ +package org.keycloak.crypto; + +import org.keycloak.models.KeycloakSession; + +public class ServerECDSASignatureSignerContext extends AsymmetricSignatureSignerContext { + + public ServerECDSASignatureSignerContext(KeycloakSession session, String algorithm) throws SignatureException { + super(ServerAsymmetricSignatureSignerContext.getKey(session, algorithm)); + } + + public ServerECDSASignatureSignerContext(KeyWrapper key) { + super(key); + } + + @Override + public byte[] sign(byte[] data) throws SignatureException { + try { + int size = ECDSASignatureProvider.ECDSA.valueOf(getAlgorithm()).getSignatureLength(); + return ECDSASignatureProvider.asn1derToConcatenatedRS(super.sign(data), size); + } catch (Exception e) { + throw new SignatureException("Signing failed", e); + } + } +} diff --git a/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java new file mode 100644 index 00000000000..6af8fa69a5a --- /dev/null +++ b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java @@ -0,0 +1,29 @@ +package org.keycloak.crypto; + +import org.keycloak.common.VerificationException; +import org.keycloak.models.KeycloakSession; + +public class ServerECDSASignatureVerifierContext extends AsymmetricSignatureVerifierContext { + public ServerECDSASignatureVerifierContext(KeycloakSession session, String kid, String algorithm) throws VerificationException { + super(ServerAsymmetricSignatureVerifierContext.getKey(session, kid, algorithm)); + } + + public ServerECDSASignatureVerifierContext(KeyWrapper key) { + super(key); + } + + @Override + public boolean verify(byte[] data, byte[] signature) throws VerificationException { + try { + /* + Fallback for backwards compatibility of ECDSA signed tokens which were issued in previous versions. + TODO remove by https://issues.jboss.org/browse/KEYCLOAK-11911 + */ + int expectedSize = ECDSASignatureProvider.ECDSA.valueOf(getAlgorithm()).getSignatureLength(); + byte[] derSignature = expectedSize != signature.length && signature[0] == 0x30 ? signature : ECDSASignatureProvider.concatenatedRSToASN1DER(signature, expectedSize); + return super.verify(data, derSignature); + } catch (Exception e) { + throw new VerificationException("Signing failed", e); + } + } +} diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestingOIDCEndpointsApplicationResource.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestingOIDCEndpointsApplicationResource.java index de9605cc4f5..dcfd647fd34 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestingOIDCEndpointsApplicationResource.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/rest/resource/TestingOIDCEndpointsApplicationResource.java @@ -28,6 +28,7 @@ import org.keycloak.crypto.AsymmetricSignatureSignerContext; import org.keycloak.crypto.KeyType; import org.keycloak.crypto.KeyUse; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.ServerECDSASignatureSignerContext; import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.jose.jwe.JWEConstants; import org.keycloak.jose.jwk.JSONWebKeySet; @@ -214,7 +215,16 @@ public class TestingOIDCEndpointsApplicationResource { keyWrapper.setAlgorithm(clientData.getSigningKeyAlgorithm()); keyWrapper.setKid(kid); keyWrapper.setPrivateKey(privateKey); - SignatureSignerContext signer = new AsymmetricSignatureSignerContext(keyWrapper); + SignatureSignerContext signer; + switch (clientData.getSigningKeyAlgorithm()) { + case Algorithm.ES256: + case Algorithm.ES384: + case Algorithm.ES512: + signer = new ServerECDSASignatureSignerContext(keyWrapper); + break; + default: + signer = new AsymmetricSignatureSignerContext(keyWrapper); + } clientData.setOidcRequest(new JWSBuilder().kid(kid).jsonContent(oidcRequest).sign(signer)); } } diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java index f0c848ff75a..a75c3889485 100644 --- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java @@ -37,14 +37,21 @@ import org.keycloak.OAuth2Constants; import org.keycloak.TokenVerifier; import org.keycloak.broker.provider.util.SimpleHttp; import org.keycloak.common.VerificationException; +import org.keycloak.common.util.KeyUtils; import org.keycloak.common.util.KeystoreUtil; import org.keycloak.constants.AdapterConstants; +import org.keycloak.crypto.Algorithm; +import org.keycloak.crypto.AsymmetricSignatureSignerContext; import org.keycloak.crypto.AsymmetricSignatureVerifierContext; import org.keycloak.crypto.KeyUse; import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.ServerECDSASignatureSignerContext; +import org.keycloak.crypto.ServerECDSASignatureVerifierContext; +import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.jose.jwk.JSONWebKeySet; import org.keycloak.jose.jwk.JWK; import org.keycloak.jose.jwk.JWKParser; +import org.keycloak.jose.jws.JWSBuilder; import org.keycloak.jose.jws.JWSInput; import org.keycloak.models.Constants; import org.keycloak.models.utils.KeycloakModelUtils; @@ -75,6 +82,7 @@ import java.net.URISyntaxException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.security.KeyStore; +import java.security.PrivateKey; import java.security.PublicKey; import java.util.Collections; import java.util.HashMap; @@ -710,7 +718,16 @@ public class OAuthClient { String kid = verifier.getHeader().getKeyId(); String algorithm = verifier.getHeader().getAlgorithm().name(); KeyWrapper key = getRealmPublicKey(realm, algorithm, kid); - AsymmetricSignatureVerifierContext verifierContext = new AsymmetricSignatureVerifierContext(key); + AsymmetricSignatureVerifierContext verifierContext; + switch (algorithm) { + case Algorithm.ES256: + case Algorithm.ES384: + case Algorithm.ES512: + verifierContext = new ServerECDSASignatureVerifierContext(key); + break; + default: + verifierContext = new AsymmetricSignatureVerifierContext(key); + } verifier.verifierContext(verifierContext); verifier.verify(); return verifier.getToken(); @@ -719,6 +736,24 @@ public class OAuthClient { } } + public SignatureSignerContext createSigner(PrivateKey privateKey, String kid, String algorithm) { + KeyWrapper keyWrapper = new KeyWrapper(); + keyWrapper.setAlgorithm(algorithm); + keyWrapper.setKid(kid); + keyWrapper.setPrivateKey(privateKey); + SignatureSignerContext signer; + switch (algorithm) { + case Algorithm.ES256: + case Algorithm.ES384: + case Algorithm.ES512: + signer = new ServerECDSASignatureSignerContext(keyWrapper); + break; + default: + signer = new AsymmetricSignatureSignerContext(keyWrapper); + } + return signer; + } + public String getClientId() { return clientId; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java index 336eff7493b..562487f6142 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java @@ -24,6 +24,7 @@ import org.apache.http.client.methods.HttpPost; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.message.BasicNameValuePair; +import org.hamcrest.collection.IsArrayContaining; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -34,7 +35,9 @@ import org.keycloak.admin.client.resource.ClientScopeResource; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.enums.SslRequired; +import org.keycloak.common.util.Base64Url; import org.keycloak.crypto.Algorithm; +import org.keycloak.crypto.ECDSASignatureProvider; import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.jose.jws.JWSHeader; @@ -94,6 +97,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.keycloak.testsuite.admin.AbstractAdminTest.loadJson; import static org.keycloak.testsuite.admin.ApiUtil.findClientByClientId; @@ -1141,6 +1145,40 @@ public class AccessTokenTest extends AbstractKeycloakTest { conductAccessTokenRequest(Algorithm.HS256, Algorithm.ES512, Algorithm.RS256); } + @Test + public void validateECDSASignatures() { + validateTokenECDSASignature(Algorithm.ES256); + validateTokenECDSASignature(Algorithm.ES384); + validateTokenECDSASignature(Algorithm.ES512); + } + + private void validateTokenECDSASignature(String expectedAlg) { + assertThat(ECDSASignatureProvider.ECDSA.values(), IsArrayContaining.hasItemInArray(ECDSASignatureProvider.ECDSA.valueOf(expectedAlg))); + + try { + TokenSignatureUtil.changeRealmTokenSignatureProvider(adminClient, expectedAlg); + TokenSignatureUtil.changeClientAccessTokenSignatureProvider(ApiUtil.findClientByClientId(adminClient.realm("test"), "test-app"), expectedAlg); + validateTokenSignatureLength(ECDSASignatureProvider.ECDSA.valueOf(expectedAlg).getSignatureLength()); + } finally { + TokenSignatureUtil.changeRealmTokenSignatureProvider(adminClient, Algorithm.RS256); + TokenSignatureUtil.changeClientAccessTokenSignatureProvider(ApiUtil.findClientByClientId(adminClient.realm("test"), "test-app"), Algorithm.RS256); + } + } + + private void validateTokenSignatureLength(int expectedLength) { + oauth.doLogin("test-user@localhost", "password"); + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, "password"); + + String token = response.getAccessToken(); + oauth.verifyToken(token); + + String encodedSignature = token.split("\\.",3)[2]; + byte[] signature = Base64Url.decode(encodedSignature); + Assert.assertEquals(expectedLength, signature.length); + oauth.openLogout(); + } + private void conductAccessTokenRequest(String expectedRefreshAlg, String expectedAccessAlg, String expectedIdTokenAlg) throws Exception { try { /// Realm Setting is used for ID Token Signature Algorithm diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java index fb55c7dc19d..2858510ec76 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientAuthSignedJWTTest.java @@ -46,9 +46,8 @@ import org.keycloak.common.constants.ServiceAccountConstants; import org.keycloak.common.util.*; import org.keycloak.constants.ServiceUrlConstants; import org.keycloak.crypto.Algorithm; -import org.keycloak.crypto.AsymmetricSignatureSignerContext; +import org.keycloak.crypto.ECDSASignatureProvider; import org.keycloak.crypto.KeyType; -import org.keycloak.crypto.KeyWrapper; import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.events.Details; import org.keycloak.events.Errors; @@ -295,6 +294,48 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { testCodeToTokenRequestSuccess(Algorithm.PS256); } + @Test + public void testECDSASignature() throws Exception { + testECDSASignatureLength(getClientSignedToken(Algorithm.ES256), Algorithm.ES256); + testECDSASignatureLength(getClientSignedToken(Algorithm.ES384), Algorithm.ES384); + testECDSASignatureLength(getClientSignedToken(Algorithm.ES512), Algorithm.ES512); + } + + private void testECDSASignatureLength(String clientSignedToken, String alg) { + String encodedSignature = clientSignedToken.split("\\.",3)[2]; + byte[] signature = Base64Url.decode(encodedSignature); + assertEquals(ECDSASignatureProvider.ECDSA.valueOf(alg).getSignatureLength(), signature.length); + } + + private String getClientSignedToken(String alg) throws Exception { + ClientRepresentation clientRepresentation = app2; + ClientResource clientResource = getClient(testRealm.getRealm(), clientRepresentation.getId()); + clientRepresentation = clientResource.toRepresentation(); + String clientSignedToken; + try { + // setup Jwks + KeyPair keyPair = setupJwks(alg, clientRepresentation, clientResource); + PublicKey publicKey = keyPair.getPublic(); + PrivateKey privateKey = keyPair.getPrivate(); + + // test + oauth.clientId("client2"); + oauth.doLogin("test-user@localhost", "password"); + + String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE); + clientSignedToken = createSignedRequestToken("client2", getRealmInfoUrl(), privateKey, publicKey, alg); + OAuthClient.AccessTokenResponse response = doAccessTokenRequest(code, clientSignedToken); + + assertEquals(200, response.getStatusCode()); + oauth.verifyToken(response.getAccessToken()); + oauth.openLogout(); + return clientSignedToken; + } finally { + // Revert jwks_url settings + revertJwksSettings(clientRepresentation, clientResource); + } + } + private void testCodeToTokenRequestSuccess(String algorithm) throws Exception { ClientRepresentation clientRepresentation = app2; ClientResource clientResource = getClient(testRealm.getRealm(), clientRepresentation.getId()); @@ -1154,14 +1195,9 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { private String createSignedRequestToken(String clientId, String realmInfoUrl, PrivateKey privateKey, PublicKey publicKey, String algorithm) { JsonWebToken jwt = createRequestToken(clientId, realmInfoUrl); String kid = KeyUtils.createKeyId(publicKey); - KeyWrapper keyWrapper = new KeyWrapper(); - keyWrapper.setAlgorithm(algorithm); - keyWrapper.setKid(kid); - keyWrapper.setPrivateKey(privateKey); - SignatureSignerContext signer = new AsymmetricSignatureSignerContext(keyWrapper); + SignatureSignerContext signer = oauth.createSigner(privateKey, kid, algorithm); String ret = new JWSBuilder().kid(kid).jsonContent(jwt).sign(signer); return ret; - } private JsonWebToken createRequestToken(String clientId, String realmInfoUrl) {