Remove backward compatibility for ECDSA tokens

Closes https://github.com/keycloak/keycloak/issues/23734

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);
+    }
+}

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);
+        }
+    }
+}

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
+        // create a key wrapper for the key pair
-        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
         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) {

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 {
-            /*
+            int expectedSize = ECDSAAlgorithm.getSignatureLength(getAlgorithm());
-            Fallback for backwards compatibility of ECDSA signed tokens which were issued in previous versions.
+            byte[] derSignature = ECDSAAlgorithm.concatenatedRSToASN1DER(signature, expectedSize);
-            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);

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;
-        }
-    }
 }

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);
-        }
-    }
 }

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 {
-            /*
+            int expectedSize = ECDSAAlgorithm.getSignatureLength(getAlgorithm());
-            Fallback for backwards compatibility of ECDSA signed tokens which were issued in previous versions.
+            byte[] derSignature = ECDSAAlgorithm.concatenatedRSToASN1DER(signature, expectedSize);
-            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);

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);

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 {

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));
+            KeyWrapper keyWrapper = new KeyWrapper();
-            signature.initSign(privateKey);
+            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();
+            byte[] signatureByteArray = sigCtx.sign(data.getBytes());
-            signature.update(dataByteArray);
-            byte[] signatureByteArray = signature.sign();
             return data + "." + Base64Url.encode(signatureByteArray);
-        } catch (NoSuchAlgorithmException | InvalidKeyException | SignatureException e) {
+        } catch (SignatureException e) {
             throw new RuntimeException(e);
         }
     }