Repository: jwtk/jjwt
Pull request #935 (2024-04-16 04:17:02)

Allow using GenericSecret for HmacSHA*
Extend the pre-existing check for SUN PKCS11 generic secret to allow all SecretKeys where getAlgorithm() returns "GenericSecret" to bypass the algorithm validation.

This matches at least with AWS CloudHSM JCE provider, but likely others as well.

Relates to discussion https://github.com/jwtk/jjwt/discussions/934

Let me know if more tests are needed. I didn't find a test that would've tested the existing Sun PKCS#11 generic secret case, and not sure how to test that when the class doesn't exist in the classpath.

mnylen: Made the requested changes. Been a bit busy, so it's somewhat slow going. :)

Patch:
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java
index 28ecdfb..e92277a 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java
@@ -135,12 +135,12 @@ final class DefaultMacAlgorithm extends AbstractSecureDigestAlgorithm<SecretKey,
             throw new InvalidKeyException(msg);
         }
 
-        // We can ignore PKCS11 key name assertions because HSM module key algorithm names don't always align with
-        // JCA standard algorithm names:
-        boolean pkcs11Key = KeysBridge.isSunPkcs11GenericSecret(key);
+        // We can ignore key name assertions for generic secrets, because HSM module key algorithm names
+        // don't always align with JCA standard algorithm names
+        boolean generic = KeysBridge.isGenericSecret(key);
 
         //assert key's jca name is valid if it's a JWA standard algorithm:
-        if (!pkcs11Key && isJwaStandard() && !isJwaStandardJcaName(name)) {
+        if (!generic && isJwaStandard() && !isJwaStandardJcaName(name)) {
             throw new InvalidKeyException("The " + keyType(signing) + " key's algorithm '" + name +
                     "' does not equal a valid HmacSHA* algorithm name or PKCS12 OID and cannot be used with " +
                     getId() + ".");
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/KeysBridge.java b/impl/src/main/java/io/jsonwebtoken/impl/security/KeysBridge.java
index a360261..869fab9 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/security/KeysBridge.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/security/KeysBridge.java
@@ -34,8 +34,9 @@ import java.security.interfaces.RSAKey;
 @SuppressWarnings({"unused"}) // reflection bridge class for the io.jsonwebtoken.security.Keys implementation
 public final class KeysBridge {
 
-    private static final String SUNPKCS11_GENERIC_SECRET_CLASSNAME = "sun.security.pkcs11.P11Key$P11SecretKey";
-    private static final String SUNPKCS11_GENERIC_SECRET_ALGNAME = "Generic Secret"; // https://github.com/openjdk/jdk/blob/4f90abaf17716493bad740dcef76d49f16d69379/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11KeyStore.java#L1292
+     // Some HSMs use generic secrets. This prefix matches the generic secret algorithm name
+     // used by SUN PKCS#11 provider, AWS CloudHSM JCE provider and possibly other HSMs
+    private static final String GENERIC_SECRET_ALG_PREFIX = "Generic";
 
     // prevent instantiation
     private KeysBridge() {
@@ -95,10 +96,13 @@ public final class KeysBridge {
         return encoded;
     }
 
-    public static boolean isSunPkcs11GenericSecret(Key key) {
-        return key instanceof SecretKey &&
-                key.getClass().getName().equals(SUNPKCS11_GENERIC_SECRET_CLASSNAME) &&
-                SUNPKCS11_GENERIC_SECRET_ALGNAME.equals(key.getAlgorithm());
+    public static boolean isGenericSecret(Key key) {
+        if (!(key instanceof SecretKey)) {
+            return false;
+        }
+
+        String algName = Assert.hasText(key.getAlgorithm(), "Key algorithm cannot be null or empty.");
+        return algName.startsWith(GENERIC_SECRET_ALG_PREFIX);
     }
 
     /**


Test patch:
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultMacAlgorithmTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultMacAlgorithmTest.groovy
index 184f502..6df8d95 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultMacAlgorithmTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultMacAlgorithmTest.groovy
@@ -19,6 +19,7 @@ import io.jsonwebtoken.impl.io.Streams
 import io.jsonwebtoken.security.*
 import org.junit.Test
 
+import javax.crypto.SecretKey
 import javax.crypto.spec.SecretKeySpec
 import java.nio.charset.StandardCharsets
 import java.security.Key
@@ -232,4 +233,29 @@ class DefaultMacAlgorithmTest {
             assertSame mac, DefaultMacAlgorithm.findByKey(oidKey)
         }
     }
+
+    /**
+     * Asserts that generic secrets are accepted
+     */
+    @Test
+    void testValidateKeyAcceptsGenericSecret() {
+        def genericSecret = new SecretKey() {
+            @Override
+            String getAlgorithm() {
+                return 'GenericSecret'
+            }
+
+            @Override
+            String getFormat() {
+                return "RAW"
+            }
+
+            @Override
+            byte[] getEncoded() {
+                return Randoms.secureRandom().nextBytes(new byte[32])
+            }
+        }
+
+        newAlg().validateKey(genericSecret, true)
+    }
 }
diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/KeysBridgeTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/KeysBridgeTest.groovy
index 89d74be..9e62ff2 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/KeysBridgeTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/KeysBridgeTest.groovy
@@ -17,9 +17,13 @@ package io.jsonwebtoken.impl.security
 
 import org.junit.Test
 
+import javax.crypto.SecretKey
 import java.security.Key
+import java.security.PrivateKey
 
 import static org.junit.Assert.assertEquals
+import static org.junit.Assert.assertFalse
+import static org.junit.Assert.assertTrue
 
 class KeysBridgeTest {
 
@@ -56,4 +60,50 @@ class KeysBridgeTest {
     void testToStringPassword() {
         testFormattedOutput(new PasswordSpec("foo".toCharArray()))
     }
+
+    @Test
+    void testIsGenericSecret() {
+        def secretKeyWithAlg = { alg ->
+            new SecretKey() {
+                @Override
+                String getAlgorithm() {
+                    return alg
+                }
+
+                @Override
+                String getFormat() {
+                    return 'RAW'
+                }
+
+                @Override
+                byte[] getEncoded() {
+                    return new byte[0]
+                }
+            }
+        }
+
+        PrivateKey genericPrivateKey = new PrivateKey() {
+            @Override
+            String getAlgorithm() {
+                return "Generic"
+            }
+
+            @Override
+            String getFormat() {
+                return "RAW"
+            }
+
+            @Override
+            byte[] getEncoded() {
+                return new byte[0]
+            }
+        }
+
+        assertTrue KeysBridge.isGenericSecret(secretKeyWithAlg("GenericSecret"))
+        assertTrue KeysBridge.isGenericSecret(secretKeyWithAlg("Generic Secret"))
+        assertFalse KeysBridge.isGenericSecret(secretKeyWithAlg(" Generic"))
+        assertFalse KeysBridge.isGenericSecret(TestKeys.HS256)
+        assertFalse KeysBridge.isGenericSecret(TestKeys.A256GCM)
+        assertFalse KeysBridge.isGenericSecret(genericPrivateKey)
+    }
 }