Merge branch 'master' into 235-Java8_time_formats

CHANGELOG.md

@@ -1,5 +1,15 @@
 ## Release Notes
 
+### 0.12.6
+
+This patch release:
+
+* Ensures that after successful JWS signature verification, an application-configured Base64Url `Decoder` output is
+  used to construct a `Jws` instance (instead of JJWT's default decoder). See
+  [Issue 947](https://github.com/jwtk/jjwt/issues/947).
+* Fixes a decompression memory leak in concurrent/multi-threaded environments introduced in 0.12.0 when decompressing JWTs with a `zip` header of `GZIP`. See [Issue 949](https://github.com/jwtk/jjwt/issues/949).
+* Upgrades BouncyCastle to 1.78 via [PR 941](https://github.com/jwtk/jjwt/pull/941).
+
 ### 0.12.5
 
 This patch release:

README.adoc

@@ -1,6 +1,6 @@
 :doctype: book
 = Java JWT: JSON Web Token for Java and Android
-:project-version: 0.12.5
+:project-version: 0.12.6
 :toc:
 :toc-title:
 :toc-placement!:

SECURITY.md

@@ -11,8 +11,8 @@ We ask that all users or security researchers upgrade to the latest stable relea
 
 | Version  | Supported          |
 | -------- | ------------------ |
-| 0.11.x   | :white_check_mark: |
-| < 0.11.0 | :x:                |
+| 0.12.x   | :white_check_mark: |
+| < 0.12.0 | :x:                |
 
 ## Reporting Security Issues
 

api/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>io.jsonwebtoken</groupId>
         <artifactId>jjwt-root</artifactId>
-        <version>1.0.0-SNAPSHOT</version>
+        <version>0.12.7-SNAPSHOT</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
 

extensions/gson/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>io.jsonwebtoken</groupId>
         <artifactId>jjwt-root</artifactId>
-        <version>1.0.0-SNAPSHOT</version>
+        <version>0.12.7-SNAPSHOT</version>
         <relativePath>../../pom.xml</relativePath>
     </parent>
 

extensions/jackson/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>io.jsonwebtoken</groupId>
         <artifactId>jjwt-root</artifactId>
-        <version>1.0.0-SNAPSHOT</version>
+        <version>0.12.7-SNAPSHOT</version>
         <relativePath>../../pom.xml</relativePath>
     </parent>
 

extensions/orgjson/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>io.jsonwebtoken</groupId>
         <artifactId>jjwt-root</artifactId>
-        <version>1.0.0-SNAPSHOT</version>
+        <version>0.12.7-SNAPSHOT</version>
         <relativePath>../../pom.xml</relativePath>
     </parent>
 

extensions/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>io.jsonwebtoken</groupId>
         <artifactId>jjwt-root</artifactId>
-        <version>1.0.0-SNAPSHOT</version>
+        <version>0.12.7-SNAPSHOT</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
 

impl/pom.xml

@@ -21,7 +21,7 @@
     <parent>
         <groupId>io.jsonwebtoken</groupId>
         <artifactId>jjwt-root</artifactId>
-        <version>1.0.0-SNAPSHOT</version>
+        <version>0.12.7-SNAPSHOT</version>
         <relativePath>../pom.xml</relativePath>
     </parent>
 

impl/src/main/java/io/jsonwebtoken/impl/DefaultJws.java

@@ -18,17 +18,16 @@
 import io.jsonwebtoken.Jws;
 import io.jsonwebtoken.JwsHeader;
 import io.jsonwebtoken.JwtVisitor;
-import io.jsonwebtoken.io.Decoders;
 
 public class DefaultJws<P> extends DefaultProtectedJwt<JwsHeader, P> implements Jws<P> {
 
     private static final String DIGEST_NAME = "signature";
 
     private final String signature;
 
-    public DefaultJws(JwsHeader header, P payload, String signature) {
-        super(header, payload, Decoders.BASE64URL.decode(signature), DIGEST_NAME);
-        this.signature = signature;
+    public DefaultJws(JwsHeader header, P payload, byte[] signature, String b64UrlSig) {
+        super(header, payload, signature, DIGEST_NAME);
+        this.signature = b64UrlSig;
     }
 
     @Override

impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java

@@ -617,14 +617,8 @@ private String sign(final Payload payload, final Key key, final Provider provide
 
             // 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
             }
@@ -712,14 +706,7 @@ private String encrypt(final Payload content, final Key key, final Provider keyP
         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):
@@ -839,4 +826,15 @@ private void encodeAndWrite(String name, byte[] data, OutputStream out) {
         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.");
+        }
+    }
+
 }

impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java

@@ -266,8 +266,8 @@ private static boolean hasContentType(Header header) {
         return header != null && Strings.hasText(header.getContentType());
     }
 
-    private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHeader, final String alg,
-                                 @SuppressWarnings("deprecation") SigningKeyResolver resolver, Claims claims, Payload payload) {
+    private byte[] verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHeader, final String alg,
+                                   @SuppressWarnings("deprecation") SigningKeyResolver resolver, Claims claims, Payload payload) {
 
         Assert.notNull(resolver, "SigningKeyResolver instance cannot be null.");
 
@@ -355,6 +355,8 @@ private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHe
         } finally {
             Streams.reset(payloadStream);
         }
+
+        return signature;
     }
 
     @Override
@@ -486,7 +488,7 @@ private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHe
         }
 
         byte[] iv = null;
-        byte[] tag = null;
+        byte[] digest = null; // either JWE AEAD tag or JWS signature after Base64Url-decoding
         if (tokenized instanceof TokenizedJwe) {
 
             TokenizedJwe tokenizedJwe = (TokenizedJwe) tokenized;
@@ -522,8 +524,8 @@ private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHe
             base64Url = base64UrlDigest;
             //guaranteed to be non-empty via the `alg` + digest check above:
             Assert.hasText(base64Url, "JWE AAD Authentication Tag cannot be null or empty.");
-            tag = decode(base64Url, "JWE AAD Authentication Tag");
-            if (Bytes.isEmpty(tag)) {
+            digest = decode(base64Url, "JWE AAD Authentication Tag");
+            if (Bytes.isEmpty(digest)) {
                 String msg = "Compact JWE strings must always contain an AAD Authentication Tag.";
                 throw new MalformedJwtException(msg);
             }
@@ -565,7 +567,7 @@ private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHe
             // TODO: add encProvider(Provider) builder method that applies to this request only?
             InputStream ciphertext = payload.toInputStream();
             ByteArrayOutputStream plaintext = new ByteArrayOutputStream(8192);
-            DecryptAeadRequest dreq = new DefaultDecryptAeadRequest(ciphertext, cek, aad, iv, tag);
+            DecryptAeadRequest dreq = new DefaultDecryptAeadRequest(ciphertext, cek, aad, iv, digest);
             encAlg.decrypt(dreq, plaintext);
             payload = new Payload(plaintext.toByteArray(), header.getContentType());
 
@@ -575,7 +577,7 @@ private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHe
             // not using a signing key resolver, so we can verify the signature before reading the payload, which is
             // always safer:
             JwsHeader jwsHeader = Assert.stateIsInstance(JwsHeader.class, header, "Not a JwsHeader. ");
-            verifySignature(tokenized, jwsHeader, alg, new LocatingKeyResolver(this.keyLocator), null, payload);
+            digest = verifySignature(tokenized, jwsHeader, alg, new LocatingKeyResolver(this.keyLocator), null, payload);
             integrityVerified = true; // no exception means signature verified
         }
 
@@ -596,66 +598,73 @@ private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHe
         Claims claims = null;
         byte[] payloadBytes = payload.getBytes();
         if (payload.isConsumable()) {
-
-            InputStream in = payload.toInputStream();
-
-            if (!hasContentType(header)) {   // If there is a content type set, then the application using JJWT is expected
-                //                          to convert the byte payload themselves based on this content type
-                //                          https://www.rfc-editor.org/rfc/rfc7515.html#section-4.1.10 :
-                //
-                //                          "This parameter is ignored by JWS implementations; any processing of this
-                //                          parameter is performed by the JWS application."
-                //
-                Map<String, ?> claimsMap = null;
-                try {
-                    // if deserialization fails, we'll need to rewind to convert to a byte array.  So if
-                    // mark/reset isn't possible, we'll need to buffer:
-                    if (!in.markSupported()) {
-                        in = new BufferedInputStream(in);
-                        in.mark(0);
-                    }
-                    claimsMap = deserialize(new UncloseableInputStream(in) /* Don't close in case we need to rewind */, "claims");
-                } catch (DeserializationException | MalformedJwtException ignored) { // not JSON, treat it as a byte[]
+            InputStream in = null;
+            try {
+                in = payload.toInputStream();
+
+                if (!hasContentType(header)) {   // If there is a content type set, then the application using JJWT is expected
+                    //                          to convert the byte payload themselves based on this content type
+                    //                          https://www.rfc-editor.org/rfc/rfc7515.html#section-4.1.10 :
+                    //
+                    //                          "This parameter is ignored by JWS implementations; any processing of this
+                    //                          parameter is performed by the JWS application."
+                    //
+                    Map<String, ?> claimsMap = null;
+                    try {
+                        // if deserialization fails, we'll need to rewind to convert to a byte array.  So if
+                        // mark/reset isn't possible, we'll need to buffer:
+                        if (!in.markSupported()) {
+                            in = new BufferedInputStream(in);
+                            in.mark(0);
+                        }
+                        claimsMap = deserialize(new UncloseableInputStream(in) /* Don't close in case we need to rewind */, "claims");
+                    } catch (DeserializationException |
+                             MalformedJwtException ignored) { // not JSON, treat it as a byte[]
 //                String msg = "Invalid claims: " + e.getMessage();
 //                throw new MalformedJwtException(msg, e);
-                } finally {
-                    Streams.reset(in);
-                }
-                if (claimsMap != null) {
-                    try {
-                        claims = new DefaultClaims(claimsMap);
-                    } catch (Throwable t) {
-                        String msg = "Invalid claims: " + t.getMessage();
-                        throw new MalformedJwtException(msg);
+                    } finally {
+                        Streams.reset(in);
+                    }
+                    if (claimsMap != null) {
+                        try {
+                            claims = new DefaultClaims(claimsMap);
+                        } catch (Throwable t) {
+                            String msg = "Invalid claims: " + t.getMessage();
+                            throw new MalformedJwtException(msg);
+                        }
                     }
                 }
-            }
-            if (claims == null) {
-                // consumable, but not claims, so convert to byte array:
-                payloadBytes = Streams.bytes(in, "Unable to convert payload to byte array.");
+                if (claims == null) {
+                    // consumable, but not claims, so convert to byte array:
+                    payloadBytes = Streams.bytes(in, "Unable to convert payload to byte array.");
+                }
+            } finally { // always ensure closed per https://github.com/jwtk/jjwt/issues/949
+                Objects.nullSafeClose(in);
             }
         }
 
+        // =============== Post-SKR Signature Check =================
+        if (hasDigest && signingKeyResolver != null) { // TODO: remove for 1.0
+            // A SigningKeyResolver has been configured, and due to it's API, we have to verify the signature after
+            // parsing the body.  This can be a security risk, so it needs to be removed before 1.0
+            JwsHeader jwsHeader = Assert.stateIsInstance(JwsHeader.class, header, "Not a JwsHeader. ");
+            digest = verifySignature(tokenized, jwsHeader, alg, this.signingKeyResolver, claims, payload);
+            //noinspection UnusedAssignment
+            integrityVerified = true; // no exception means verified successfully
+        }
+
         Jwt<?, ?> jwt;
         Object body = claims != null ? claims : payloadBytes;
         if (header instanceof JweHeader) {
-            jwt = new DefaultJwe<>((JweHeader) header, body, iv, tag);
+            jwt = new DefaultJwe<>((JweHeader) header, body, iv, digest);
         } else if (hasDigest) {
             JwsHeader jwsHeader = Assert.isInstanceOf(JwsHeader.class, header, "JwsHeader required.");
-            jwt = new DefaultJws<>(jwsHeader, body, base64UrlDigest.toString());
+            jwt = new DefaultJws<>(jwsHeader, body, digest, base64UrlDigest.toString());
         } else {
             //noinspection rawtypes
             jwt = new DefaultJwt(header, body);
         }
 
-        // =============== Signature =================
-        if (hasDigest && signingKeyResolver != null) { // TODO: remove for 1.0
-            // A SigningKeyResolver has been configured, and due to it's API, we have to verify the signature after
-            // parsing the body.  This can be a security risk, so it needs to be removed before 1.0
-            JwsHeader jwsHeader = Assert.stateIsInstance(JwsHeader.class, header, "Not a JwsHeader. ");
-            verifySignature(tokenized, jwsHeader, alg, this.signingKeyResolver, claims, payload);
-        }
-
         final boolean allowSkew = this.allowedClockSkewMillis > 0;
 
         //since 0.3:

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

@@ -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() + ".");

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

@@ -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() {
@@ -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);
     }
 
     /**