KEYCLOAK-9089 IllegalArgumentException when trying to use ES256 as OI…

…DC access token signature
csfreitas · Dec 14, 2018 · 061693a · 061693a
1 parent 4150daa
commit 061693a
Show file tree

Hide file tree

Showing 22 changed files with 466 additions and 55 deletions.
diff --git a/core/src/main/java/org/keycloak/crypto/AsymmetricSignatureSignerContext.java b/core/src/main/java/org/keycloak/crypto/AsymmetricSignatureSignerContext.java
@@ -37,6 +37,11 @@ public String getAlgorithm() {
         return key.getAlgorithm();
     }
 
+    @Override
+    public String getHashAlgorithm() {
+        return JavaAlgorithm.getJavaAlgorithmForHash(key.getAlgorithm());
+    }
+
     @Override
     public byte[] sign(byte[] data) throws SignatureException {
         try {

diff --git a/core/src/main/java/org/keycloak/crypto/HashException.java b/core/src/main/java/org/keycloak/crypto/HashException.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2017 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 <a href="mailto:[email protected]">Marek Posolda</a>
+ */
+public class HashException extends RuntimeException {
+
+    public HashException(String message) {
+        super(message);
+    }
+
+    public HashException(String message, Throwable cause) {
+        super(message, cause);
+    }
+}
diff --git a/core/src/main/java/org/keycloak/crypto/JavaAlgorithm.java b/core/src/main/java/org/keycloak/crypto/JavaAlgorithm.java
@@ -29,6 +29,10 @@ public class JavaAlgorithm {
     public static final String ES512 = "SHA512withECDSA";
     public static final String AES = "AES";
 
+    public static final String SHA256 = "SHA-256";
+    public static final String SHA384 = "SHA-384";
+    public static final String SHA512 = "SHA-512";
+
     public static String getJavaAlgorithm(String algorithm) {
         switch (algorithm) {
             case Algorithm.RS256:
@@ -56,4 +60,32 @@ public static String getJavaAlgorithm(String algorithm) {
         }
     }
 
+
+    public static String getJavaAlgorithmForHash(String algorithm) {
+        switch (algorithm) {
+            case Algorithm.RS256:
+                return SHA256;
+            case Algorithm.RS384:
+                return SHA384;
+            case Algorithm.RS512:
+                return SHA512;
+            case Algorithm.HS256:
+                return SHA256;
+            case Algorithm.HS384:
+                return SHA384;
+            case Algorithm.HS512:
+                return SHA512;
+            case Algorithm.ES256:
+                return SHA256;
+            case Algorithm.ES384:
+                return SHA384;
+            case Algorithm.ES512:
+                return SHA512;
+            case Algorithm.AES:
+                return AES;
+            default:
+                throw new IllegalArgumentException("Unknown algorithm " + algorithm);
+        }
+    }
+
 }
diff --git a/core/src/main/java/org/keycloak/crypto/MacSignatureSignerContext.java b/core/src/main/java/org/keycloak/crypto/MacSignatureSignerContext.java
@@ -36,6 +36,11 @@ public String getAlgorithm() {
         return key.getAlgorithm();
     }
 
+    @Override
+    public String getHashAlgorithm() {
+        return JavaAlgorithm.getJavaAlgorithmForHash(key.getAlgorithm());
+    }
+
     @Override
     public byte[] sign(byte[] data) throws SignatureException {
         try {

diff --git a/core/src/main/java/org/keycloak/crypto/SignatureSignerContext.java b/core/src/main/java/org/keycloak/crypto/SignatureSignerContext.java
@@ -22,6 +22,8 @@ public interface SignatureSignerContext {
 
     String getAlgorithm();
 
+    String getHashAlgorithm();
+
     byte[] sign(byte[] data) throws SignatureException;
 
 }
diff --git a/...eycloak/jose/jws/crypto/HashProvider.java → ...g/keycloak/jose/jws/crypto/HashUtils.java b/...eycloak/jose/jws/crypto/HashProvider.java → ...g/keycloak/jose/jws/crypto/HashUtils.java
@@ -18,51 +18,48 @@
 package org.keycloak.jose.jws.crypto;
 
 import org.keycloak.common.util.Base64Url;
-import org.keycloak.jose.jws.Algorithm;
+import org.keycloak.crypto.HashException;
+import org.keycloak.crypto.JavaAlgorithm;
 
+import java.io.UnsupportedEncodingException;
 import java.security.MessageDigest;
 import java.util.Arrays;
 
 /**
  * @author <a href="mailto:[email protected]">Marek Posolda</a>
  */
-public class HashProvider {
+public class HashUtils {
 
+    // See "at_hash" and "c_hash" in OIDC specification
     public static String oidcHash(String jwtAlgorithmName, String input) {
-        byte[] digest = digest(jwtAlgorithmName, input);
-
-        int hashLength = digest.length / 2;
-        byte[] hashInput = Arrays.copyOf(digest, hashLength);
+        try {
+            byte[] inputBytes = input.getBytes("UTF-8");
+            String javaAlgName = JavaAlgorithm.getJavaAlgorithmForHash(jwtAlgorithmName);
+            byte[] hash = hash(javaAlgName, inputBytes);
 
-        return Base64Url.encode(hashInput);
+            return encodeHashToOIDC(hash);
+        } catch (UnsupportedEncodingException e) {
+            throw new HashException("Error when creating token hash", e);
+        }
     }
-    private static byte[] digest(String algorithm, String input) {
-        String digestAlg = getJavaDigestAlgorithm(algorithm);
 
+
+    public static byte[] hash(String javaAlgorithmName, byte[] inputBytes) {
         try {
-            MessageDigest md = MessageDigest.getInstance(digestAlg);
-            md.update(input.getBytes("UTF-8"));
+            MessageDigest md = MessageDigest.getInstance(javaAlgorithmName);
+            md.update(inputBytes);
             return md.digest();
         } catch (Exception e) {
-            throw new RuntimeException(e);
-        }
-    }
-    private static String getJavaDigestAlgorithm(String alg) {
-        switch (alg) {
-            case "RS256":
-                return "SHA-256";
-            case "RS384":
-                return "SHA-384";
-            case "RS512":
-                return "SHA-512";
-            default:
-                throw new IllegalArgumentException("Not an RSA Algorithm");
+            throw new HashException("Error when creating token hash", e);
         }
     }
 
-    // See "at_hash" and "c_hash" in OIDC specification
-    public static String oidcHash(Algorithm jwtAlgorithm, String input) {
-        return oidcHash(jwtAlgorithm.name(), input);
+
+    public static String encodeHashToOIDC(byte[] hash) {
+        int hashLength = hash.length / 2;
+        byte[] hashInput = Arrays.copyOf(hash, hashLength);
+
+        return Base64Url.encode(hashInput);
     }
 
 }
diff --git a/core/src/test/java/org/keycloak/AtHashTest.java b/core/src/test/java/org/keycloak/AtHashTest.java
@@ -20,8 +20,8 @@
 import org.bouncycastle.jce.provider.BouncyCastleProvider;
 import org.junit.Assert;
 import org.junit.Test;
-import org.keycloak.jose.jws.Algorithm;
-import org.keycloak.jose.jws.crypto.HashProvider;
+import org.keycloak.crypto.Algorithm;
+import org.keycloak.jose.jws.crypto.HashUtils;
 
 import java.security.Security;
 
@@ -37,13 +37,19 @@ public class AtHashTest {
     }
 
     @Test
-    public void testAtHash() throws Exception {
-        verifyHash("jHkWEdUXMU1BwAsC4vtUsZwnNvTIxEl0z9K3vx5KF0Y", "77QmUPtjPfzWtF2AnpK9RQ");
-        verifyHash("ya29.eQETFbFOkAs8nWHcmYXKwEi0Zz46NfsrUU_KuQLOLTwWS40y6Fb99aVzEXC0U14m61lcPMIr1hEIBA", "aUAkJG-u6x4RTWuILWy-CA");
+    public void testAtHashRsa() throws Exception {
+        verifyHash(Algorithm.RS256,"jHkWEdUXMU1BwAsC4vtUsZwnNvTIxEl0z9K3vx5KF0Y", "77QmUPtjPfzWtF2AnpK9RQ");
+        verifyHash(Algorithm.RS256,"ya29.eQETFbFOkAs8nWHcmYXKwEi0Zz46NfsrUU_KuQLOLTwWS40y6Fb99aVzEXC0U14m61lcPMIr1hEIBA", "aUAkJG-u6x4RTWuILWy-CA");
     }
 
-    private void verifyHash(String accessToken, String expectedAtHash) {
-        String atHash = HashProvider.oidcHash(Algorithm.RS256, accessToken);
+    @Test
+    public void testAtHashEs() throws Exception {
+        verifyHash(Algorithm.ES256,"jHkWEdUXMU1BwAsC4vtUsZwnNvTIxEl0z9K3vx5KF0Y", "77QmUPtjPfzWtF2AnpK9RQ");
+        verifyHash(Algorithm.ES256,"ya29.eQETFbFOkAs8nWHcmYXKwEi0Zz46NfsrUU_KuQLOLTwWS40y6Fb99aVzEXC0U14m61lcPMIr1hEIBA", "aUAkJG-u6x4RTWuILWy-CA");
+    }
+
+    private void verifyHash(String jwtAlgorithm, String accessToken, String expectedAtHash) {
+        String atHash = HashUtils.oidcHash(jwtAlgorithm, accessToken);
         Assert.assertEquals(expectedAtHash, atHash);
     }
 }
diff --git a/server-spi-private/src/main/java/org/keycloak/crypto/HashProvider.java b/server-spi-private/src/main/java/org/keycloak/crypto/HashProvider.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2017 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.UnsupportedEncodingException;
+
+import org.keycloak.provider.Provider;
+
+/**
+ * @author <a href="mailto:[email protected]">Marek Posolda</a>
+ */
+public interface HashProvider extends Provider {
+
+
+    default byte[] hash(String input) throws HashException {
+        try {
+            byte[] inputBytes = input.getBytes("UTF-8");
+            return hash(inputBytes);
+        } catch (UnsupportedEncodingException e) {
+            throw new HashException("Unsupported encoding when trying to hash", e);
+        }
+    }
+
+
+    byte[] hash(byte[] input) throws HashException;
+
+
+    @Override
+    default void close() {
+    }
+
+}
diff --git a/server-spi-private/src/main/java/org/keycloak/crypto/HashProviderFactory.java b/server-spi-private/src/main/java/org/keycloak/crypto/HashProviderFactory.java
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2017 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 org.keycloak.Config;
+import org.keycloak.models.KeycloakSessionFactory;
+import org.keycloak.provider.ProviderFactory;
+
+/**
+ * @author <a href="mailto:[email protected]">Marek Posolda</a>
+ */
+public interface HashProviderFactory extends ProviderFactory<HashProvider> {
+
+    @Override
+    default void init(Config.Scope config) {
+    }
+
+    @Override
+    default void postInit(KeycloakSessionFactory factory) {
+    }
+
+    @Override
+    default void close() {
+    }
+}
diff --git a/server-spi-private/src/main/java/org/keycloak/crypto/HashSpi.java b/server-spi-private/src/main/java/org/keycloak/crypto/HashSpi.java
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2017 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 org.keycloak.provider.Provider;
+import org.keycloak.provider.ProviderFactory;
+import org.keycloak.provider.Spi;
+
+/**
+ * @author <a href="mailto:[email protected]">Marek Posolda</a>
+ */
+public class HashSpi implements Spi {
+
+    @Override
+    public boolean isInternal() {
+        return true;
+    }
+
+    @Override
+    public String getName() {
+        return "hash";
+    }
+
+    @Override
+    public Class<? extends Provider> getProviderClass() {
+        return HashProvider.class;
+    }
+
+    @Override
+    public Class<? extends ProviderFactory> getProviderFactoryClass() {
+        return HashProviderFactory.class;
+    }
+}
diff --git a/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi b/server-spi-private/src/main/resources/META-INF/services/org.keycloak.provider.Spi
@@ -73,4 +73,5 @@ org.keycloak.keys.PublicKeyStorageSpi
 org.keycloak.keys.KeySpi
 org.keycloak.storage.client.ClientStorageProviderSpi
 org.keycloak.crypto.SignatureSpi
-org.keycloak.crypto.ClientSignatureVerifierSpi
+org.keycloak.crypto.ClientSignatureVerifierSpi
+org.keycloak.crypto.HashSpi