Skip to content

Commit 23d9a33

Browse files
authored
Allow using GenericSecret for HmacSHA* (#935)
* Extended the pre-existing check for SUN PKCS11 generic secret to allow `SecretKey`s with `getAlgorithm()` names starting with `Generic` (e.g. `GenericSecret` and `Generic Secret`) as valid keys for `HmacSHA*` algorithm name checks in `DefaultMacAlgorithm`. This matches at least with the Sun PKCS11 and AWS CloudHSM JCE providers, but likely others as well.
1 parent c673b76 commit 23d9a33

File tree

4 files changed

+90
-10
lines changed

4 files changed

+90
-10
lines changed

impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,12 @@ private void assertAlgorithmName(SecretKey key, boolean signing) {
135135
throw new InvalidKeyException(msg);
136136
}
137137

138-
// We can ignore PKCS11 key name assertions because HSM module key algorithm names don't always align with
139-
// JCA standard algorithm names:
140-
boolean pkcs11Key = KeysBridge.isSunPkcs11GenericSecret(key);
138+
// We can ignore key name assertions for generic secrets, because HSM module key algorithm names
139+
// don't always align with JCA standard algorithm names
140+
boolean generic = KeysBridge.isGenericSecret(key);
141141

142142
//assert key's jca name is valid if it's a JWA standard algorithm:
143-
if (!pkcs11Key && isJwaStandard() && !isJwaStandardJcaName(name)) {
143+
if (!generic && isJwaStandard() && !isJwaStandardJcaName(name)) {
144144
throw new InvalidKeyException("The " + keyType(signing) + " key's algorithm '" + name +
145145
"' does not equal a valid HmacSHA* algorithm name or PKCS12 OID and cannot be used with " +
146146
getId() + ".");

impl/src/main/java/io/jsonwebtoken/impl/security/KeysBridge.java

+10-6
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@
3434
@SuppressWarnings({"unused"}) // reflection bridge class for the io.jsonwebtoken.security.Keys implementation
3535
public final class KeysBridge {
3636

37-
private static final String SUNPKCS11_GENERIC_SECRET_CLASSNAME = "sun.security.pkcs11.P11Key$P11SecretKey";
38-
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
37+
// Some HSMs use generic secrets. This prefix matches the generic secret algorithm name
38+
// used by SUN PKCS#11 provider, AWS CloudHSM JCE provider and possibly other HSMs
39+
private static final String GENERIC_SECRET_ALG_PREFIX = "Generic";
3940

4041
// prevent instantiation
4142
private KeysBridge() {
@@ -95,10 +96,13 @@ public static byte[] findEncoded(Key key) {
9596
return encoded;
9697
}
9798

98-
public static boolean isSunPkcs11GenericSecret(Key key) {
99-
return key instanceof SecretKey &&
100-
key.getClass().getName().equals(SUNPKCS11_GENERIC_SECRET_CLASSNAME) &&
101-
SUNPKCS11_GENERIC_SECRET_ALGNAME.equals(key.getAlgorithm());
99+
public static boolean isGenericSecret(Key key) {
100+
if (!(key instanceof SecretKey)) {
101+
return false;
102+
}
103+
104+
String algName = Assert.hasText(key.getAlgorithm(), "Key algorithm cannot be null or empty.");
105+
return algName.startsWith(GENERIC_SECRET_ALG_PREFIX);
102106
}
103107

104108
/**

impl/src/test/groovy/io/jsonwebtoken/impl/security/DefaultMacAlgorithmTest.groovy

+26
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import io.jsonwebtoken.impl.io.Streams
1919
import io.jsonwebtoken.security.*
2020
import org.junit.Test
2121

22+
import javax.crypto.SecretKey
2223
import javax.crypto.spec.SecretKeySpec
2324
import java.nio.charset.StandardCharsets
2425
import java.security.Key
@@ -232,4 +233,29 @@ class DefaultMacAlgorithmTest {
232233
assertSame mac, DefaultMacAlgorithm.findByKey(oidKey)
233234
}
234235
}
236+
237+
/**
238+
* Asserts that generic secrets are accepted
239+
*/
240+
@Test
241+
void testValidateKeyAcceptsGenericSecret() {
242+
def genericSecret = new SecretKey() {
243+
@Override
244+
String getAlgorithm() {
245+
return 'GenericSecret'
246+
}
247+
248+
@Override
249+
String getFormat() {
250+
return "RAW"
251+
}
252+
253+
@Override
254+
byte[] getEncoded() {
255+
return Randoms.secureRandom().nextBytes(new byte[32])
256+
}
257+
}
258+
259+
newAlg().validateKey(genericSecret, true)
260+
}
235261
}

impl/src/test/groovy/io/jsonwebtoken/impl/security/KeysBridgeTest.groovy

+50
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@ package io.jsonwebtoken.impl.security
1717

1818
import org.junit.Test
1919

20+
import javax.crypto.SecretKey
2021
import java.security.Key
22+
import java.security.PrivateKey
2123

2224
import static org.junit.Assert.assertEquals
25+
import static org.junit.Assert.assertFalse
26+
import static org.junit.Assert.assertTrue
2327

2428
class KeysBridgeTest {
2529

@@ -56,4 +60,50 @@ class KeysBridgeTest {
5660
void testToStringPassword() {
5761
testFormattedOutput(new PasswordSpec("foo".toCharArray()))
5862
}
63+
64+
@Test
65+
void testIsGenericSecret() {
66+
def secretKeyWithAlg = { alg ->
67+
new SecretKey() {
68+
@Override
69+
String getAlgorithm() {
70+
return alg
71+
}
72+
73+
@Override
74+
String getFormat() {
75+
return 'RAW'
76+
}
77+
78+
@Override
79+
byte[] getEncoded() {
80+
return new byte[0]
81+
}
82+
}
83+
}
84+
85+
PrivateKey genericPrivateKey = new PrivateKey() {
86+
@Override
87+
String getAlgorithm() {
88+
return "Generic"
89+
}
90+
91+
@Override
92+
String getFormat() {
93+
return "RAW"
94+
}
95+
96+
@Override
97+
byte[] getEncoded() {
98+
return new byte[0]
99+
}
100+
}
101+
102+
assertTrue KeysBridge.isGenericSecret(secretKeyWithAlg("GenericSecret"))
103+
assertTrue KeysBridge.isGenericSecret(secretKeyWithAlg("Generic Secret"))
104+
assertFalse KeysBridge.isGenericSecret(secretKeyWithAlg(" Generic"))
105+
assertFalse KeysBridge.isGenericSecret(TestKeys.HS256)
106+
assertFalse KeysBridge.isGenericSecret(TestKeys.A256GCM)
107+
assertFalse KeysBridge.isGenericSecret(genericPrivateKey)
108+
}
59109
}

0 commit comments

Comments
 (0)