Repository: jwtk/jjwt
Pull request #937 (2024-04-22 05:22:20)

JWE arbitrary content compression
The JWE content was compressed only when claims was used as payload, but when using arbitrary content string/bytes/input stream, the compression was omitted, while keeping the "zip" header field, leading to un-parseable JWE as the decompression failed on non-compressed content.

I couldn't find any evidence that this was intended behavior from the documentation or the JWE spec, so made a fix for this.

See https://github.com/jwtk/jjwt/discussions/936

Note that people might have used a similar workaround as I presented in the discussion to manually compress the payload, so fixing this might result in some code breaking somewhere. Might be good to mention in the changelog.

lhazlewood: Excellent find!!! I can't believe this was missed, thank you so much 🙏 

Patch:
diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java
index ef41c7a..afa6a48 100644
--- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java
+++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java
@@ -595,14 +595,8 @@ public class DefaultJwtBuilder implements JwtBuilder {
 
             // Next, b64 extension requires the raw (non-encoded) payload to be included directly in the signing input,
             // so we ensure we have an input stream for that:
-            if (payload.isClaims() || payload.isCompressed()) {
-                ByteArrayOutputStream claimsOut = new ByteArrayOutputStream(8192);
-                writeAndClose("JWS Unencoded Payload", payload, claimsOut);
-                payloadStream = Streams.of(claimsOut.toByteArray());
-            } else {
-                // No claims and not compressed, so just get the direct InputStream:
-                payloadStream = Assert.stateNotNull(payload.toInputStream(), "Payload InputStream cannot be null.");
-            }
+            payloadStream = toInputStream("JWS Unencoded Payload", payload);
+
             if (!payload.isClaims()) {
                 payloadStream = new CountingInputStream(payloadStream); // we'll need to assert if it's empty later
             }
@@ -693,14 +687,7 @@ public class DefaultJwtBuilder implements JwtBuilder {
         Assert.stateNotNull(keyAlgFunction, "KeyAlgorithm function cannot be null.");
         assertPayloadEncoding("JWE");
 
-        InputStream plaintext;
-        if (content.isClaims()) {
-            ByteArrayOutputStream out = new ByteArrayOutputStream(4096);
-            writeAndClose("JWE Claims", content, out);
-            plaintext = Streams.of(out.toByteArray());
-        } else {
-            plaintext = content.toInputStream();
-        }
+        InputStream plaintext = toInputStream("JWE Payload", content);
 
         //only expose (mutable) JweHeader functionality to KeyAlgorithm instances, not the full headerBuilder
         // (which exposes this JwtBuilder and shouldn't be referenced by KeyAlgorithms):
@@ -820,4 +807,15 @@ public class DefaultJwtBuilder implements JwtBuilder {
         Streams.writeAndClose(out, data, "Unable to write bytes");
     }
 
+    private InputStream toInputStream(final String name, Payload payload) {
+        if (payload.isClaims() || payload.isCompressed()) {
+            ByteArrayOutputStream claimsOut = new ByteArrayOutputStream(8192);
+            writeAndClose(name, payload, claimsOut);
+            return Streams.of(claimsOut.toByteArray());
+        } else {
+            // No claims and not compressed, so just get the direct InputStream:
+            return Assert.stateNotNull(payload.toInputStream(), "Payload InputStream cannot be null.");
+        }
+    }
+
 }
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/JwtsTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy
index 2f1c432..5f6b822 100644
--- a/impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy
+++ b/impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy
@@ -22,6 +22,7 @@ import io.jsonwebtoken.impl.io.Streams
 import io.jsonwebtoken.impl.lang.Bytes
 import io.jsonwebtoken.impl.lang.Services
 import io.jsonwebtoken.impl.security.*
+import io.jsonwebtoken.io.CompressionAlgorithm
 import io.jsonwebtoken.io.Decoders
 import io.jsonwebtoken.io.Deserializer
 import io.jsonwebtoken.io.Encoders
@@ -1398,6 +1399,97 @@ class JwtsTest {
         }
     }
 
+    @Test
+    void testJweCompressionWithArbitraryContentString() {
+        def codecs = [Jwts.ZIP.DEF, Jwts.ZIP.GZIP]
+
+        for (CompressionAlgorithm zip : codecs) {
+
+            for (AeadAlgorithm enc : Jwts.ENC.get().values()) {
+
+                SecretKey key = enc.key().build()
+
+                String payload = 'hello, world!'
+
+                // encrypt and compress:
+                String jwe = Jwts.builder()
+                    .content(payload)
+                    .compressWith(zip)
+                    .encryptWith(key, enc)
+                    .compact()
+
+                //decompress and decrypt:
+                def jwt = Jwts.parser()
+                    .decryptWith(key)
+                    .build()
+                    .parseEncryptedContent(jwe)
+                assertEquals payload, new String(jwt.getPayload(), StandardCharsets.UTF_8)
+            }
+        }
+    }
+
+    @Test
+    void testJweCompressionWithArbitraryContentByteArray() {
+        def codecs = [Jwts.ZIP.DEF, Jwts.ZIP.GZIP]
+
+        for (CompressionAlgorithm zip : codecs) {
+
+            for (AeadAlgorithm enc : Jwts.ENC.get().values()) {
+
+                SecretKey key = enc.key().build()
+
+                byte[] payload = new byte[14];
+                Randoms.secureRandom().nextBytes(payload)
+
+                // encrypt and compress:
+                String jwe = Jwts.builder()
+                        .content(payload)
+                        .compressWith(zip)
+                        .encryptWith(key, enc)
+                        .compact()
+
+                //decompress and decrypt:
+                def jwt = Jwts.parser()
+                        .decryptWith(key)
+                        .build()
+                        .parseEncryptedContent(jwe)
+                assertArrayEquals payload, jwt.getPayload()
+            }
+        }
+    }
+
+    @Test
+    void testJweCompressionWithArbitraryContentInputStream() {
+        def codecs = [Jwts.ZIP.DEF, Jwts.ZIP.GZIP]
+
+        for (CompressionAlgorithm zip : codecs) {
+
+            for (AeadAlgorithm enc : Jwts.ENC.get().values()) {
+
+                SecretKey key = enc.key().build()
+
+                byte[] payloadBytes = new byte[14];
+                Randoms.secureRandom().nextBytes(payloadBytes)
+
+                ByteArrayInputStream payload = new ByteArrayInputStream(payloadBytes)
+
+                // encrypt and compress:
+                String jwe = Jwts.builder()
+                        .content(payload)
+                        .compressWith(zip)
+                        .encryptWith(key, enc)
+                        .compact()
+
+                //decompress and decrypt:
+                def jwt = Jwts.parser()
+                        .decryptWith(key)
+                        .build()
+                        .parseEncryptedContent(jwe)
+                assertArrayEquals payloadBytes, jwt.getPayload()
+            }
+        }
+    }
+
     @Test
     void testPasswordJwes() {
 
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)
+    }
 }