Fixes #947 (#948)

Author: lhazlewood

CHANGELOG.md

@@ -1,5 +1,13 @@
 ## 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).
+
 ### 0.12.5
 
 This patch release:

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/DefaultJwtParser.java

@@ -265,8 +265,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.");
 
@@ -354,6 +354,8 @@ private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHe
         } finally {
             Streams.reset(payloadStream);
         }
+
+        return signature;
     }
 
     @Override
@@ -485,7 +487,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;
@@ -521,8 +523,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);
             }
@@ -564,7 +566,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());
 
@@ -574,7 +576,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
         }
 
@@ -635,26 +637,28 @@ private void verifySignature(final TokenizedJwt tokenized, final JwsHeader jwsHe
             }
         }
 
+        // =============== 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/test/groovy/io/jsonwebtoken/impl/DefaultJwsTest.groovy

@@ -17,19 +17,26 @@ package io.jsonwebtoken.impl
 
 import io.jsonwebtoken.JwsHeader
 import io.jsonwebtoken.Jwts
+import io.jsonwebtoken.impl.lang.Bytes
+import io.jsonwebtoken.io.Encoders
 import org.junit.Test
 
+import java.security.MessageDigest
+
 import static org.junit.Assert.*
 
 class DefaultJwsTest {
 
     @Test
     void testConstructor() {
         JwsHeader header = new DefaultJwsHeader([:])
-        def jws = new DefaultJws<String>(header, 'foo', 'sig')
+        byte[] sig = Bytes.random(32)
+        String b64u = Encoders.BASE64URL.encode(sig)
+        def jws = new DefaultJws<String>(header, 'foo', sig, b64u)
         assertSame jws.getHeader(), header
         assertEquals jws.getPayload(), 'foo'
-        assertEquals jws.getSignature(), 'sig'
+        assertTrue MessageDigest.isEqual(sig, jws.getDigest())
+        assertEquals b64u, jws.getSignature()
     }
 
     @Test