AWS CloudHSM JCE provider & JJWT

[mnylen]

Hello!

I've been exploring the use of AWS CloudHSM for signing and verifying JWTs. This works pretty easily with RS256 and ES256 algorithms, but HS256 is proving to be a bit difficult, because the SecretKey instance returned from HSM just has GenericSecret as algorithm, which JJWT rejects:

io.jsonwebtoken.security.SignatureException: Unable to compute HS256 signature. Cause: The signing key's algorithm 'GenericSecret' does not equal a valid HmacSHA* algorithm name or PKCS12 OID and cannot be used with HS256.

The same key however works well when used directly with javax.crypto.Mac manually.

I'm wondering if there's a way to tell JJWT that the key has HmacSHA256 algorithm, or somehow force it to accept the key? As far as I know, as long as the key is >= 32 bytes, it should be suitable, as long as the provider can calculate the mac using the key?

var provider = new CloudHsmProvider();
Security.addProvider(provider);

var keystore = KeyStore.getInstance(CloudHsmProvider.PROVIDER_NAME);
keystore.load(null, null);

var key = (SecretKey)keystore.getKey("hmac-key", null);

// this works
var mac = Mac.getInstance("HmacSHA256", CloudHsmProvider.PROVIDER_NAME);
mac.init(key);
mac.update("hello".getBytes());
System.out.println("digest: " + Base64.getEncoder().encodeToString(mac.doFinal()));

// this throws io.jsonwebtoken.security.SignatureException: Unable to compute HS256 signature
var token = Jwts.builder().subject("hello").signWith(key, Jwts.SIG.HS256).compact();

Edit: My own replies to this thread deviated slightly to also include problems encountered with AES-GCM encryption, so renamed the discussion from being specific to the HMAC keys to just be generally AWS CloudHSM & JJWT related. :)

[mnylen]

It seems this error is coming from

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

Line 143 in c673b76

if (!pkcs11Key && isJwaStandard() && !isJwaStandardJcaName(name)) {

where there seems to be some support for generic secrets, but I guess the CloudHSM GenericSecretKey variant doesn't get matched.

The actual key class is com.amazonaws.cloudhsm.jce.provider.GenericSecretKey and the getAlgorithm() returns GenericSecret

Would JJWT implementation need to be extended to support this, or is there a way to convert so that the algorithm would match the expected one?

[mnylen]

The only workaround I was able to find was to "monkey-patch" the whole DefaultMacAlgorithm to not validate the algorithm name. But after that, the HS256 sign/verify works as expected with CloudHSM keys.

First, I tried to create a custom ProviderKey implementation that overrides the getAlgorithm() method, but unfortunately it seems the ProviderKey is unwrapped by the time DefaultMacAlgorithmis used. Although still hacky, it would've isolated addition instead of replacing an existing class.

[mnylen]

I created a PR to allow bypassing the algorithm name validation for any SecretKey where getAlgorithm() returns "GenericSecret". #935

[mnylen]

Not exactly related to HMAC keys, but JWEs with A256GCM encryption can't use AES keys retrieved via CloudHSM JCE Provider, as they report key length to be 0.

Trying to supply com.amazonaws.cloudhsm.jce.provider.AesKey to encryptWith gives

io.jsonwebtoken.security.WeakKeyException: The 'A256GCM' algorithm requires keys with a length of 256 bits (32 bytes).  The provided key has a length of 0 bits (0 bytes).

which seems to be thrown from here https://github.com/jwtk/jjwt/blob/master/impl/src/main/java/io/jsonwebtoken/impl/security/AesAlgorithm.java#L98

The problem is that getEncoded() for com.amazonaws.cloudhsm.jce.provider.AesKey returns null (as the key is not extractable), which Bytes.bitLength assumes to mean 0 bits. 🤔 Is this how null values are intended to be interpreted?

[mnylen]

Would something like this be acceptable for GcmAesAeadAlgorithm#encrypt():

    @Override
    public void encrypt(final AeadRequest req, AeadResult res) throws SecurityException {

        Assert.notNull(req, "Request cannot be null.");
        Assert.notNull(res, "Result cannot be null.");

        final SecretKey key = assertKey(req.getKey());
        final InputStream plaintext = Assert.notNull(req.getPayload(),
                "Request content (plaintext) InputStream cannot be null.");
        final OutputStream out = Assert.notNull(res.getOutputStream(),
                "Result ciphertext OutputStream cannot be null.");
        final InputStream aad = req.getAssociatedData();
        final byte[] ivSpecIv = ensureInitializationVector(req);
        final AlgorithmParameterSpec ivSpec = getIvSpec(ivSpecIv);

        final TagAndIv result = jca(req).withCipher(new CheckedFunction<Cipher, TagAndIv>() {
            @Override
            public TagAndIv apply(Cipher cipher) throws Exception {
                cipher.init(Cipher.ENCRYPT_MODE, key, ivSpec);
                byte[] taggedCiphertext = withCipher(cipher, plaintext, aad, out);
                // When using GCM mode, the JDK appends the authentication tag to the ciphertext, so let's extract it:
                // (tag has a length of BLOCK_BYTE_SIZE):
                int ciphertextLength = Bytes.length(taggedCiphertext) - BLOCK_BYTE_SIZE;
                Streams.write(out, taggedCiphertext, 0, ciphertextLength, "Ciphertext write failure.");
                byte[] tag = new byte[BLOCK_BYTE_SIZE];
                System.arraycopy(taggedCiphertext, ciphertextLength, tag, 0, BLOCK_BYTE_SIZE);

                return new TagAndIv(tag, cipher.getIV());
            }
        });
        Streams.flush(out);
        Streams.reset(plaintext);

        res.setTag(result.getTag()).setIv(result.getIv());
    }

The modifications to the original would be:

1. Return a new value class TagAndIv from withCipher(...) block
2. The actual IV is read from Cipher

There probably needs to be assertion that cipher.getIV() doesn't return null.

And the idea here would be that a custom enc algorithm could pass IvSupplier with the zero-IV, so the cipher would be initialized with that, but the actual IV will be read from the cipher.

Similar changes should be done to other algorithms as well, but before I start the work, would like some direction. :)

[lhazlewood]

And FWIW, the DefaultMacAlgorithm (that the original question was related to) uses KeysBridge.findBitLength to determine the key length (

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

Line 110 in c673b76

public static int findBitLength(Key key) {

). This method seems to handle the case where getFormat() returns null. Is there something specific about AesAlgorithm that it can't use this method that would already correctly handle this case, and instead has it's own length validation?

Nope, nothing specific about AesAlgorithm other than it was implemented before KeysBridge#findBitLength was written 😄. It definitely makes sense to refactor AesAlgorithm to use the same logic for the same reasons.

[lhazlewood]

A few things here about the GcmAesAeadAlgorithm implementation and initialization vectors:

The GcmAesAeadAlgorithm is in the impl package which is the only JJWT .jar that does not adhere to semantic versioning per https://github.com/jwtk/jjwt?tab=readme-ov-file#understanding-jjwt-dependencies. When JDK 9 modules are supported via module-info.java files, internal implementations won't be accessible at all, so it's best not to depend on anything in that .jar. (this #929 (comment) might be helpful as well).

So the 'safer' immediate fix here is implement the AeadAlgorithm interface directly to do whatever you need and override the default implementation by using JwtParserBuilder#enc() (see that method's JavaDoc).

Regarding a zeroed-out IV, I have to admit that feels really wrong and a very poor design decision on the part of the AWS team. Proper IVs are never deterministic - forcing an all-zero IV could have catastrophic affects on an application where the wrong IV is used (e.g. IV is zeroed out but CloudHSM is not used in that context, and now there's no random initialization destroying the application's security). That AWS would require this is irresponsible IMO, it'd be better to NOT provide an IV at all when initializing the Cipher to clearly indicate that the Cipher is expected to generate one automatically.

So it seems that some thought needs to be put into how IVs are generated within the AesAlgorithm implementation and subclasses. The current implementation works well but it's not extensible for things like AWS's oddity. If we can guarantee that an IV will be securely generated at all times by the Cipher, there's no need for JJWT to generate one itself, and we can remove that code. Testing/verification and non-null/non-empty assertions would need to be put in the code to guarantee this of course.

There probably needs to be assertion that cipher.getIV() doesn't return null.

Yep, non-null but also has the exact required AES IV length (96 bits for GCM mode, 128 bits otherwise).

In the meantime of course, a custom AeadAlgorithm implementation avoids any delay waiting for a JJWT release.

[mnylen]

Yeah, I agree that it'd be quite bad if the IV then isn't generated. So perhaps the right way for all of these issues would be to just "implement" (copy-paste with some surgery) our own algorithms and assert that CloudHSM provider is used in them.

It's probably hard to guarantee that all Cipher implementations would generate the IV securely. It would need to be some special interface that the request implements, like ProviderGeneratedIv. But then this would also need to provide the IV that the Cipher gets initialized with, as it's unlikely that other HSMs would similarly except zero-IV. But with that approach, one could implement their own algorithm that just delegates a modified request to Jwts.ENC.A256GCM.

[mnylen]

Having done some further testing, it seems with CloudHSM, if you call Cipher#init() without the third params object, it also generates the IV for you. So no need for the zero-IV.

null or empty byte array won't be accepted by GCMParametersSpec

[lhazlewood]

Hi @mnylen!

I was out over the weekend and yesterday was as super busy day, so we haven't been able to review these comments in detail until now. Please see below:

I've been exploring the use of AWS CloudHSM for signing and verifying JWTs. This works pretty easily with RS256 and ES256 algorithms, but HS256 is proving to be a bit difficult, because the SecretKey instance returned from HSM just has GenericSecret as algorithm, which JJWT rejects:

io.jsonwebtoken.security.SignatureException: Unable to compute HS256 signature. Cause: The signing key's algorithm 'GenericSecret' does not equal a valid HmacSHA* algorithm name or PKCS12 OID and cannot be used with HS256.

The same key however works well when used directly with javax.crypto.Mac manually.

The HmacSHA* name and OID checks are in place to prevent people from accidentally using keys that they shouldn't be using for Mac calculations. This happens all the time in applications: where one key is used for one cryptographic operation and then used again for an entirely different operation (e.g. Mac and AES encryption, or using the same RSA key pair for both encryption and signatures), and this is a well-documented poor practice that compromises key security and most application developers are never aware of the problem. So the name check helps ensure that someone doesn't accidentally use an AES key (for example) as a Mac key.

The JCA doesn't do any key algorithm name checks (as you've shown in your code example), but the JCA has many problems like this that are poor practices that I won't get into now 😓 . So JJWT tries to 'do better' and avoid these scenarios entirely.

It's unfortunate that Sun's own PKCS11 keys, and now evidently AWS's, don't make this distinction. But I think we can make an exception for Cloud HSM as well. For example, we have this check:

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

Lines 98 to 102 in c673b76

	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());
	}

We could probably add something similar and then wrap both checks in a utility method that covers both cases for use in various algorithms.

var provider = new CloudHsmProvider();
Security.addProvider(provider);

JJWT has been tested extensively when using Sun's PKCS11 provider, which acts as a bridge to the native HSM implementation. Have you tried using Cloud HSM with the SunPKCS11 provider instead of directly as shown in the code snippet above? If this works, it may likely solve the issues you're seeing.

[mnylen]

I could maybe try that. Just wonder if the SUNPKCS11 implements all the loadbalancing bits over multiple HSMs that the CloudHSM one does.

But AES-GCM will require the IV to be generated on the HSM, see the other thread in this discussion. Ah, nevermind - saw that you answered that already.