diff --git a/core/src/main/java/org/keycloak/crypto/ECDSAAlgorithm.java b/core/src/main/java/org/keycloak/crypto/ECDSAAlgorithm.java new file mode 100644 index 0000000000..8bdc7efd1a --- /dev/null +++ b/core/src/main/java/org/keycloak/crypto/ECDSAAlgorithm.java @@ -0,0 +1,52 @@ +/* + * Copyright 2023 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.crypto; + +import java.io.IOException; +import org.keycloak.common.crypto.CryptoIntegration; + +/** + * + * @author rmartinc + */ +public enum ECDSAAlgorithm { + ES256(64), + ES384(96), + ES512(132); + + private final int signatureLength; + + ECDSAAlgorithm(int signatureLength) { + this.signatureLength = signatureLength; + } + + public int getSignatureLength() { + return this.signatureLength; + } + + public static int getSignatureLength(String alg) { + return valueOf(alg).getSignatureLength(); + } + + public static byte[] concatenatedRSToASN1DER(final byte[] signature, int signLength) throws IOException { + return CryptoIntegration.getProvider().getEcdsaCryptoProvider().concatenatedRSToASN1DER(signature, signLength); + } + + public static byte[] asn1derToConcatenatedRS(final byte[] derEncodedSignatureValue, int signLength) throws IOException { + return CryptoIntegration.getProvider().getEcdsaCryptoProvider().asn1derToConcatenatedRS(derEncodedSignatureValue, signLength); + } +} diff --git a/core/src/main/java/org/keycloak/crypto/ECDSASignatureSignerContext.java b/core/src/main/java/org/keycloak/crypto/ECDSASignatureSignerContext.java new file mode 100644 index 0000000000..80f3a929db --- /dev/null +++ b/core/src/main/java/org/keycloak/crypto/ECDSASignatureSignerContext.java @@ -0,0 +1,38 @@ +/* + * Copyright 2023 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.crypto; + +/** + * + * @author rmartinc + */ +public class ECDSASignatureSignerContext extends AsymmetricSignatureSignerContext { + + public ECDSASignatureSignerContext(KeyWrapper key) throws SignatureException { + super(key); + } + + @Override + public byte[] sign(byte[] data) throws SignatureException { + try { + int size = ECDSAAlgorithm.getSignatureLength(getAlgorithm()); + return ECDSAAlgorithm.asn1derToConcatenatedRS(super.sign(data), size); + } catch (Exception e) { + throw new SignatureException("Signing failed", e); + } + } +} diff --git a/core/src/main/java/org/keycloak/protocol/oidc/client/authentication/JWTClientCredentialsProvider.java b/core/src/main/java/org/keycloak/protocol/oidc/client/authentication/JWTClientCredentialsProvider.java index 9451b359e7..d18c639c0c 100644 --- a/core/src/main/java/org/keycloak/protocol/oidc/client/authentication/JWTClientCredentialsProvider.java +++ b/core/src/main/java/org/keycloak/protocol/oidc/client/authentication/JWTClientCredentialsProvider.java @@ -28,6 +28,7 @@ import org.keycloak.common.util.KeystoreUtil; import org.keycloak.common.util.Time; import org.keycloak.crypto.Algorithm; import org.keycloak.crypto.AsymmetricSignatureSignerContext; +import org.keycloak.crypto.ECDSASignatureSignerContext; import org.keycloak.crypto.JavaAlgorithm; import org.keycloak.crypto.KeyType; import org.keycloak.crypto.KeyUse; @@ -62,22 +63,7 @@ public class JWTClientCredentialsProvider implements ClientCredentialsProvider { } public void setupKeyPair(KeyPair keyPair, String algorithm) { - // check the algorithm is valid - switch (keyPair.getPublic().getAlgorithm()) { - case KeyType.RSA: - if (!JavaAlgorithm.isRSAJavaAlgorithm(algorithm)) { - throw new RuntimeException("Invalid algorithm for a RSA KeyPair: " + algorithm); - } - break; - case KeyType.EC: - if (!JavaAlgorithm.isECJavaAlgorithm(algorithm)) { - throw new RuntimeException("Invalid algorithm for a EC KeyPair: " + algorithm); - } - break; - default: - throw new RuntimeException("Invalid KeyPair algorithm: " + keyPair.getPublic().getAlgorithm()); - } - // create the key and signature context + // create a key wrapper for the key pair KeyWrapper keyWrapper = new KeyWrapper(); keyWrapper.setKid(KeyUtils.createKeyId(keyPair.getPublic())); keyWrapper.setAlgorithm(algorithm); @@ -85,8 +71,26 @@ public class JWTClientCredentialsProvider implements ClientCredentialsProvider { keyWrapper.setPublicKey(keyPair.getPublic()); keyWrapper.setType(keyPair.getPublic().getAlgorithm()); keyWrapper.setUse(KeyUse.SIG); + + // check the algorithm is valid + switch (keyPair.getPublic().getAlgorithm()) { + case KeyType.RSA: + if (!JavaAlgorithm.isRSAJavaAlgorithm(algorithm)) { + throw new RuntimeException("Invalid algorithm for a RSA KeyPair: " + algorithm); + } + this.sigCtx = new AsymmetricSignatureSignerContext(keyWrapper); + break; + case KeyType.EC: + if (!JavaAlgorithm.isECJavaAlgorithm(algorithm)) { + throw new RuntimeException("Invalid algorithm for a EC KeyPair: " + algorithm); + } + this.sigCtx = new ECDSASignatureSignerContext(keyWrapper); + break; + default: + throw new RuntimeException("Invalid KeyPair algorithm: " + keyPair.getPublic().getAlgorithm()); + } + // create the key and signature context this.keyPair = keyPair; - this.sigCtx = new AsymmetricSignatureSignerContext(keyWrapper); } public void setTokenTimeout(int tokenTimeout) { diff --git a/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java b/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java index 4e0d0d4473..f70386f097 100644 --- a/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java +++ b/services/src/main/java/org/keycloak/crypto/ClientECDSASignatureVerifierContext.java @@ -34,12 +34,8 @@ public class ClientECDSASignatureVerifierContext extends AsymmetricSignatureVeri @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); + int expectedSize = ECDSAAlgorithm.getSignatureLength(getAlgorithm()); + byte[] derSignature = ECDSAAlgorithm.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/ECDSASignatureProvider.java b/services/src/main/java/org/keycloak/crypto/ECDSASignatureProvider.java index 134dc2b58e..2a3efaa851 100644 --- a/services/src/main/java/org/keycloak/crypto/ECDSASignatureProvider.java +++ b/services/src/main/java/org/keycloak/crypto/ECDSASignatureProvider.java @@ -1,11 +1,8 @@ package org.keycloak.crypto; import org.keycloak.common.VerificationException; -import org.keycloak.common.crypto.CryptoIntegration; import org.keycloak.models.KeycloakSession; -import java.io.IOException; - public class ECDSASignatureProvider implements SignatureProvider { private final KeycloakSession session; @@ -42,28 +39,4 @@ public class ECDSASignatureProvider implements SignatureProvider { public boolean isAsymmetricAlgorithm() { return true; } - - public static byte[] concatenatedRSToASN1DER(final byte[] signature, int signLength) throws IOException { - return CryptoIntegration.getProvider().getEcdsaCryptoProvider().concatenatedRSToASN1DER(signature, signLength); - } - - public static byte[] asn1derToConcatenatedRS(final byte[] derEncodedSignatureValue, int signLength) throws IOException { - return CryptoIntegration.getProvider().getEcdsaCryptoProvider().asn1derToConcatenatedRS(derEncodedSignatureValue, signLength); - } - - 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/ServerECDSASignatureSignerContext.java b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureSignerContext.java index 9758e2b784..a0a9fd9747 100644 --- a/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureSignerContext.java +++ b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureSignerContext.java @@ -2,7 +2,7 @@ package org.keycloak.crypto; import org.keycloak.models.KeycloakSession; -public class ServerECDSASignatureSignerContext extends AsymmetricSignatureSignerContext { +public class ServerECDSASignatureSignerContext extends ECDSASignatureSignerContext { public ServerECDSASignatureSignerContext(KeycloakSession session, String algorithm) throws SignatureException { super(ServerAsymmetricSignatureSignerContext.getKey(session, algorithm)); @@ -11,14 +11,4 @@ public class ServerECDSASignatureSignerContext extends AsymmetricSignatureSigner 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 index 6af8fa69a5..bfedabaad5 100644 --- a/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java +++ b/services/src/main/java/org/keycloak/crypto/ServerECDSASignatureVerifierContext.java @@ -15,12 +15,8 @@ public class ServerECDSASignatureVerifierContext extends AsymmetricSignatureVer @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); + int expectedSize = ECDSAAlgorithm.getSignatureLength(getAlgorithm()); + byte[] derSignature = ECDSAAlgorithm.concatenatedRSToASN1DER(signature, expectedSize); return super.verify(data, derSignature); } catch (Exception e) { throw new VerificationException("Signing failed", e); 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 21430e9d75..adc2d18af7 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 @@ -40,7 +40,7 @@ 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.crypto.ECDSAAlgorithm; import org.keycloak.crypto.KeyUse; import org.keycloak.events.Details; import org.keycloak.events.Errors; @@ -1318,12 +1318,12 @@ public class AccessTokenTest extends AbstractKeycloakTest { } private void validateTokenECDSASignature(String expectedAlg) { - assertThat(ECDSASignatureProvider.ECDSA.values(), hasItemInArray(ECDSASignatureProvider.ECDSA.valueOf(expectedAlg))); + assertThat(ECDSAAlgorithm.values(), hasItemInArray(ECDSAAlgorithm.valueOf(expectedAlg))); try { TokenSignatureUtil.changeRealmTokenSignatureProvider(adminClient, expectedAlg); TokenSignatureUtil.changeClientAccessTokenSignatureProvider(ApiUtil.findClientByClientId(adminClient.realm("test"), "test-app"), expectedAlg); - validateTokenSignatureLength(ECDSASignatureProvider.ECDSA.valueOf(expectedAlg).getSignatureLength()); + validateTokenSignatureLength(ECDSAAlgorithm.getSignatureLength(expectedAlg)); } finally { TokenSignatureUtil.changeRealmTokenSignatureProvider(adminClient, Algorithm.RS256); TokenSignatureUtil.changeClientAccessTokenSignatureProvider(ApiUtil.findClientByClientId(adminClient.realm("test"), "test-app"), Algorithm.RS256); 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 da9d4b5f90..6f12fab10c 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 @@ -57,7 +57,7 @@ import org.keycloak.common.util.UriUtils; import org.keycloak.common.util.KeystoreUtil.KeystoreFormat; import org.keycloak.constants.ServiceUrlConstants; import org.keycloak.crypto.Algorithm; -import org.keycloak.crypto.ECDSASignatureProvider; +import org.keycloak.crypto.ECDSAAlgorithm; import org.keycloak.crypto.KeyType; import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.events.Details; @@ -398,7 +398,7 @@ public class ClientAuthSignedJWTTest extends AbstractKeycloakTest { 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); + assertEquals(ECDSAAlgorithm.getSignatureLength(alg), signature.length); } private String getClientSignedToken(String alg) throws Exception { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/DPoPTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/DPoPTest.java index 49ba52483b..787fc3a513 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/DPoPTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/DPoPTest.java @@ -24,15 +24,12 @@ import static org.keycloak.jose.jwk.JWKUtil.toIntegerBytes; import java.io.IOException; import java.security.InvalidAlgorithmParameterException; -import java.security.InvalidKeyException; import java.security.Key; import java.security.KeyPair; import java.security.KeyPairGenerator; import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.security.SecureRandom; -import java.security.Signature; -import java.security.SignatureException; import java.security.interfaces.ECPublicKey; import java.security.interfaces.RSAPublicKey; import java.security.spec.ECGenParameterSpec; @@ -55,8 +52,13 @@ import org.keycloak.common.util.Base64Url; import org.keycloak.common.util.KeyUtils; import org.keycloak.common.util.Time; import org.keycloak.crypto.Algorithm; -import org.keycloak.crypto.JavaAlgorithm; +import org.keycloak.crypto.AsymmetricSignatureSignerContext; import org.keycloak.crypto.KeyType; +import org.keycloak.crypto.KeyUse; +import org.keycloak.crypto.KeyWrapper; +import org.keycloak.crypto.ECDSASignatureSignerContext; +import org.keycloak.crypto.SignatureException; +import org.keycloak.crypto.SignatureSignerContext; import org.keycloak.jose.jwk.ECPublicJWK; import org.keycloak.jose.jwk.JWK; import org.keycloak.jose.jwk.RSAPublicJWK; @@ -64,7 +66,6 @@ import org.keycloak.jose.jws.JWSHeader; import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper; import org.keycloak.representations.AccessToken; import org.keycloak.representations.RefreshToken; -import org.keycloak.representations.UserInfo; import org.keycloak.representations.dpop.DPoP; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.RealmRepresentation; @@ -566,6 +567,17 @@ public class DPoPTest extends AbstractTestRealmKeycloakTest { return keyPair; } + private static SignatureSignerContext createSignatureSignerContext(KeyWrapper keyWrapper) { + switch (keyWrapper.getType()) { + case KeyType.RSA: + return new AsymmetricSignatureSignerContext(keyWrapper); + case KeyType.EC: + return new ECDSASignatureSignerContext(keyWrapper); + default: + throw new IllegalArgumentException("No signer provider for key algorithm type " + keyWrapper.getType()); + } + } + private static String generateSignedDPoPProof(String jti, String htm, String htu, Long iat, String algorithm, JWSHeader jwsHeader, PrivateKey privateKey) throws IOException { String dpopProofHeaderEncoded = Base64Url.encode(JsonSerialization.writeValueAsBytes(jwsHeader)); @@ -579,14 +591,18 @@ public class DPoPTest extends AbstractTestRealmKeycloakTest { String dpopProofPayloadEncoded = Base64Url.encode(JsonSerialization.writeValueAsBytes(dpop)); try { - Signature signature = Signature.getInstance(JavaAlgorithm.getJavaAlgorithm(algorithm)); - signature.initSign(privateKey); + KeyWrapper keyWrapper = new KeyWrapper(); + keyWrapper.setKid(jwsHeader.getKeyId()); + keyWrapper.setAlgorithm(algorithm); + keyWrapper.setPrivateKey(privateKey); + keyWrapper.setType(privateKey.getAlgorithm()); + keyWrapper.setUse(KeyUse.SIG); + SignatureSignerContext sigCtx = createSignatureSignerContext(keyWrapper); + String data = dpopProofHeaderEncoded + "." + dpopProofPayloadEncoded; - byte[] dataByteArray = data.getBytes(); - signature.update(dataByteArray); - byte[] signatureByteArray = signature.sign(); + byte[] signatureByteArray = sigCtx.sign(data.getBytes()); return data + "." + Base64Url.encode(signatureByteArray); - } catch (NoSuchAlgorithmException | InvalidKeyException | SignatureException e) { + } catch (SignatureException e) { throw new RuntimeException(e); } }