From 1fa8ccbda9d1e41e1570ed8f7c35d41c9bb64eea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mikko=20Nyl=C3=A9n?= <mikko@mikkonylen.fi>
Date: Mon, 22 Apr 2024 04:54:03 +0300
Subject: [PATCH] Review fixes

---
 .../impl/security/DefaultMacAlgorithm.java    |  6 +--
 .../impl/security/KeysBridge.java             | 14 +++----
 .../impl/security/KeysBridgeTest.groovy       | 42 ++++++++++---------
 3 files changed, 32 insertions(+), 30 deletions(-)

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 1ffb74d2f..e92277aed 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java
@@ -136,11 +136,11 @@ private void assertAlgorithmName(SecretKey key, boolean signing) {
         }
 
         // 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 pkcs11Key = KeysBridge.isGenericSecret(key);
+        // 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 d15046c2d..869fab940 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/security/KeysBridge.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/security/KeysBridge.java
@@ -34,9 +34,9 @@
 @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
-    private static final String GENERIC_SECRET_ALGNAME = "GenericSecret"; // AWS CloudHSM JCE provider and possibly other HSMs
+     // 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() {
@@ -99,12 +99,10 @@ public static byte[] findEncoded(Key key) {
     public static boolean isGenericSecret(Key key) {
         if (!(key instanceof SecretKey)) {
             return false;
-        } else if (key.getClass().getName().equals(SUNPKCS11_GENERIC_SECRET_CLASSNAME) &&
-                SUNPKCS11_GENERIC_SECRET_ALGNAME.equals(key.getAlgorithm())) {
-            return true;
-        } else {
-            return GENERIC_SECRET_ALGNAME.equals(key.getAlgorithm());
         }
+
+        String algName = Assert.hasText(key.getAlgorithm(), "Key algorithm cannot be null or empty.");
+        return algName.startsWith(GENERIC_SECRET_ALG_PREFIX);
     }
 
     /**
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 67f41dbb7..9e62ff2c1 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/KeysBridgeTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/KeysBridgeTest.groovy
@@ -63,32 +63,34 @@ class KeysBridgeTest {
 
     @Test
     void testIsGenericSecret() {
-        def genericSecret = new SecretKey() {
-            @Override
-            String getAlgorithm() {
-                return "GenericSecret" ;
-            }
-
-            @Override
-            String getFormat() {
-                return null
+        def secretKeyWithAlg = { alg ->
+            new SecretKey() {
+                @Override
+                String getAlgorithm() {
+                    return alg
+                }
+
+                @Override
+                String getFormat() {
+                    return 'RAW'
+                }
+
+                @Override
+                byte[] getEncoded() {
+                    return new byte[0]
+                }
             }
+        }
 
-            @Override
-            byte[] getEncoded() {
-                return null;
-            }
-        };
-
-        def genericPrivateKey = new PrivateKey() {
+        PrivateKey genericPrivateKey = new PrivateKey() {
             @Override
             String getAlgorithm() {
-                return "GenericSecret";
+                return "Generic"
             }
 
             @Override
             String getFormat() {
-                return null
+                return "RAW"
             }
 
             @Override
@@ -97,7 +99,9 @@ class KeysBridgeTest {
             }
         }
 
-        assertTrue KeysBridge.isGenericSecret(genericSecret)
+        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)