Skip to content

Commit

Permalink
Allow using GenericSecret for HmacSHA* (#935)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
mnylen authored Apr 22, 2024
1 parent c673b76 commit 23d9a33
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ private void assertAlgorithmName(SecretKey key, boolean signing) {
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() + ".");
Expand Down
16 changes: 10 additions & 6 deletions impl/src/main/java/io/jsonwebtoken/impl/security/KeysBridge.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +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
// 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() {
Expand Down Expand Up @@ -95,10 +96,13 @@ public static byte[] findEncoded(Key key) {
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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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)
}
}

0 comments on commit 23d9a33

Please sign in to comment.