diff --git a/CHANGELOG.md b/CHANGELOG.md index 70c53ede..deca6a46 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJws.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJws.java index 0bf0fdf8..dadaa51d 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJws.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJws.java @@ -18,7 +18,6 @@ package io.jsonwebtoken.impl; import io.jsonwebtoken.Jws; import io.jsonwebtoken.JwsHeader; import io.jsonwebtoken.JwtVisitor; -import io.jsonwebtoken.io.Decoders; public class DefaultJws

extends DefaultProtectedJwt implements Jws

{ @@ -26,9 +25,9 @@ public class DefaultJws

extends DefaultProtectedJwt implements 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 diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java index beb36f4b..ad2faad6 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java @@ -265,8 +265,8 @@ public class DefaultJwtParser extends AbstractParser> implements JwtPa 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 @@ public class DefaultJwtParser extends AbstractParser> implements JwtPa } finally { Streams.reset(payloadStream); } + + return signature; } @Override @@ -485,7 +487,7 @@ public class DefaultJwtParser extends AbstractParser> implements JwtPa } 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 @@ public class DefaultJwtParser extends AbstractParser> implements JwtPa 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 @@ public class DefaultJwtParser extends AbstractParser> implements JwtPa // 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 @@ public class DefaultJwtParser extends AbstractParser> implements JwtPa // 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,24 +637,26 @@ public class DefaultJwtParser extends AbstractParser> implements JwtPa } } - Jwt jwt; - Object body = claims != null ? claims : payloadBytes; - if (header instanceof JweHeader) { - jwt = new DefaultJwe<>((JweHeader) header, body, iv, tag); - } else if (hasDigest) { - JwsHeader jwsHeader = Assert.isInstanceOf(JwsHeader.class, header, "JwsHeader required."); - jwt = new DefaultJws<>(jwsHeader, body, base64UrlDigest.toString()); - } else { - //noinspection rawtypes - jwt = new DefaultJwt(header, body); - } - - // =============== Signature ================= + // =============== 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. "); - verifySignature(tokenized, jwsHeader, alg, this.signingKeyResolver, claims, payload); + 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, digest); + } else if (hasDigest) { + JwsHeader jwsHeader = Assert.isInstanceOf(JwsHeader.class, header, "JwsHeader required."); + jwt = new DefaultJws<>(jwsHeader, body, digest, base64UrlDigest.toString()); + } else { + //noinspection rawtypes + jwt = new DefaultJwt(header, body); } final boolean allowSkew = this.allowedClockSkewMillis > 0; diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwsTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwsTest.groovy index a55e550e..1511eb10 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwsTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwsTest.groovy @@ -17,8 +17,12 @@ 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 { @@ -26,10 +30,13 @@ class DefaultJwsTest { @Test void testConstructor() { JwsHeader header = new DefaultJwsHeader([:]) - def jws = new DefaultJws(header, 'foo', 'sig') + byte[] sig = Bytes.random(32) + String b64u = Encoders.BASE64URL.encode(sig) + def jws = new DefaultJws(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