diff --git a/README.md b/README.md index b95e148e..d550e511 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ enforcement. * Convenient and readable [fluent](http://en.wikipedia.org/wiki/Fluent_interface) interfaces, great for IDE auto-completion to write code quickly * Fully RFC specification compliant on all implemented functionality, tested against RFC-specified test vectors - * Stable implementation with over 1,100+ tests and enforced 100% test code coverage. Every single method, statement + * Stable implementation with over 1,200+ tests and enforced 100% test code coverage. Every single method, statement and conditional branch variant in the entire codebase is tested and required to pass on every build. * Creating, parsing and verifying digitally signed compact JWTs (aka JWSs) with all standard JWS algorithms: diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java index 3f2605d3..ddf71fdd 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java @@ -57,6 +57,7 @@ import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; import java.nio.charset.StandardCharsets; import java.security.Key; +import java.security.PrivateKey; import java.security.Provider; import java.security.PublicKey; import java.security.SecureRandom; @@ -65,9 +66,12 @@ import java.util.Map; public class DefaultJwtBuilder implements JwtBuilder { - public static final String PUB_KEY_SIGN_MSG = "PublicKeys may not be used to create digital signatures. " + - "Only PrivateKeys may be used to create digital signatures, and PublicKeys are used to verify " + - "digital signatures."; + private static final String PUB_KEY_SIGN_MSG = "PublicKeys may not be used to create digital signatures. " + + "PrivateKeys are used to sign, and PublicKeys are used to verify."; + + private static final String PRIV_KEY_ENC_MSG = "PrivateKeys may not be used to encrypt data. PublicKeys are " + + "used to encrypt, and PrivateKeys are used to decrypt."; + protected Provider provider; protected SecureRandom secureRandom; @@ -295,12 +299,18 @@ public class DefaultJwtBuilder implements JwtBuilder { } }, "%s encryption failed.", encId); - this.key = Assert.notNull(key, "Key cannot be null."); - - //noinspection unchecked - this.keyAlg = (KeyAlgorithm) Assert.notNull(keyAlg, "KeyAlgorithm cannot be null."); + Assert.notNull(key, "Encryption key cannot be null."); + if (key instanceof PrivateKey) { + throw new IllegalArgumentException(PRIV_KEY_ENC_MSG); + } + Assert.notNull(keyAlg, "KeyAlgorithm cannot be null."); final String algId = Assert.hasText(keyAlg.getId(), "KeyAlgorithm id cannot be null or empty."); + + this.key = key; + //noinspection unchecked + this.keyAlg = (KeyAlgorithm) keyAlg; final KeyAlgorithm alg = this.keyAlg; + final String cekMsg = "Unable to obtain content encryption key from key management algorithm '%s'."; this.keyAlgFunction = Functions.wrap(new Function, KeyResult>() { @Override diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java index 3b2264a6..af424285 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java @@ -68,7 +68,9 @@ import io.jsonwebtoken.security.WeakKeyException; import javax.crypto.SecretKey; import java.nio.charset.StandardCharsets; import java.security.Key; +import java.security.PrivateKey; import java.security.Provider; +import java.security.PublicKey; import java.util.Collection; import java.util.Date; import java.util.Map; @@ -80,6 +82,12 @@ public class DefaultJwtParser implements JwtParser { private static final JwtTokenizer jwtTokenizer = new JwtTokenizer(); + static final String PRIV_KEY_VERIFY_MSG = "PrivateKeys may not be used to verify digital signatures. " + + "PrivateKeys are used to sign, and PublicKeys are used to verify."; + + static final String PUB_KEY_DECRYPT_MSG = "PublicKeys may not be used to decrypt data. PublicKeys are " + + "used to encrypt, and PrivateKeys are used to decrypt."; + public static final String INCORRECT_EXPECTED_CLAIM_MESSAGE_TEMPLATE = "Expected %s claim to be: %s, but was: %s."; public static final String MISSING_EXPECTED_CLAIM_MESSAGE_TEMPLATE = "Expected %s claim to be: %s, but was not " + "present in the JWT claims."; @@ -303,6 +311,12 @@ public class DefaultJwtParser implements JwtParser { String msg = "Cannot verify JWS signature: unable to locate signature verification key for JWS with header: " + jwsHeader; throw new UnsupportedJwtException(msg); } + Provider provider = ProviderKey.getProvider(key, this.provider); // extract if necessary + key = ProviderKey.getKey(key); // unwrap if necessary, MUST be called after ProviderKey.getProvider + Assert.stateNotNull(key, "ProviderKey cannot be null."); //ProviderKey impl doesn't allow null + if (key instanceof PrivateKey) { + throw new InvalidKeyException(PRIV_KEY_VERIFY_MSG); + } //re-create the jwt part without the signature. This is what is needed for signature verification: String jwtWithoutSignature = tokenized.getProtected() + SEPARATOR_CHAR + tokenized.getPayload(); @@ -311,9 +325,6 @@ public class DefaultJwtParser implements JwtParser { byte[] signature = decode(tokenized.getDigest(), "JWS signature"); try { - Provider provider = ProviderKey.getProvider(key, this.provider); // extract if necessary - key = ProviderKey.getKey(key); // unwrap if necessary, MUST be called after ProviderKey.getProvider - Assert.stateNotNull(key, "ProviderKey cannot be null."); //ProviderKey impl doesn't allow null VerifySecureDigestRequest request = new DefaultVerifySecureDigestRequest<>(data, provider, null, key, signature); if (!algorithm.verify(request)) { @@ -454,6 +465,9 @@ public class DefaultJwtParser implements JwtParser { String msg = "Cannot decrypt JWE payload: unable to locate key for JWE with header: " + jweHeader; throw new UnsupportedJwtException(msg); } + if (key instanceof PublicKey) { + throw new InvalidKeyException(PUB_KEY_DECRYPT_MSG); + } // extract key-specific provider if necessary; Provider provider = ProviderKey.getProvider(key, this.provider); diff --git a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java index b776d235..751c94c6 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java @@ -251,6 +251,9 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder { } private JwtParserBuilder verifyWith(Key key) { + if (key instanceof PrivateKey) { + throw new IllegalArgumentException(DefaultJwtParser.PRIV_KEY_VERIFY_MSG); + } this.signatureVerificationKey = Assert.notNull(key, "signature verification key cannot be null."); return this; } @@ -266,6 +269,9 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder { } private JwtParserBuilder decryptWith(final Key key) { + if (key instanceof PublicKey) { + throw new IllegalArgumentException(DefaultJwtParser.PUB_KEY_DECRYPT_MSG); + } this.decryptionKey = Assert.notNull(key, "decryption key cannot be null."); return this; } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractSecureDigestAlgorithm.java b/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractSecureDigestAlgorithm.java index 6c4d7ef7..65883fa7 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractSecureDigestAlgorithm.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/AbstractSecureDigestAlgorithm.java @@ -24,7 +24,6 @@ import io.jsonwebtoken.security.SignatureException; import io.jsonwebtoken.security.VerifySecureDigestRequest; import java.security.Key; -import java.security.MessageDigest; abstract class AbstractSecureDigestAlgorithm extends CryptoAlgorithm implements SecureDigestAlgorithm { @@ -75,14 +74,5 @@ abstract class AbstractSecureDigestAlgorithm exten } } - protected boolean messageDigest(VerifySecureDigestRequest request) { - byte[] providedSignature = request.getDigest(); - Assert.notEmpty(providedSignature, "Request signature byte array cannot be null or empty."); - @SuppressWarnings({"unchecked", "rawtypes"}) byte[] computedSignature = digest((SecureRequest) request); - return MessageDigest.isEqual(providedSignature, computedSignature); - } - - protected boolean doVerify(VerifySecureDigestRequest request) throws Exception { - return messageDigest(request); - } + protected abstract boolean doVerify(VerifySecureDigestRequest request); } 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 6f07a7c0..169f33b3 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/DefaultMacAlgorithm.java @@ -25,11 +25,13 @@ import io.jsonwebtoken.security.Password; import io.jsonwebtoken.security.SecretKeyBuilder; import io.jsonwebtoken.security.SecureRequest; import io.jsonwebtoken.security.UnsupportedKeyException; +import io.jsonwebtoken.security.VerifySecureDigestRequest; import io.jsonwebtoken.security.WeakKeyException; import javax.crypto.Mac; import javax.crypto.SecretKey; import java.security.Key; +import java.security.MessageDigest; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Locale; @@ -207,4 +209,11 @@ final class DefaultMacAlgorithm extends AbstractSecureDigestAlgorithm request) { + byte[] providedSignature = request.getDigest(); + Assert.notEmpty(providedSignature, "Request signature byte array cannot be null or empty."); + byte[] computedSignature = digest(request); + return MessageDigest.isEqual(providedSignature, computedSignature); + } } diff --git a/impl/src/main/java/io/jsonwebtoken/impl/security/RsaSignatureAlgorithm.java b/impl/src/main/java/io/jsonwebtoken/impl/security/RsaSignatureAlgorithm.java index 6d39215a..356bfeb0 100644 --- a/impl/src/main/java/io/jsonwebtoken/impl/security/RsaSignatureAlgorithm.java +++ b/impl/src/main/java/io/jsonwebtoken/impl/security/RsaSignatureAlgorithm.java @@ -156,6 +156,7 @@ final class RsaSignatureAlgorithm extends AbstractSignatureAlgorithm { return PSS_ALG_NAMES.contains(alg); } + @SuppressWarnings("BooleanMethodIsAlwaysInverted") static boolean isRsaAlgorithmName(Key key) { String alg = KeysBridge.findAlgorithm(key); return KEY_ALG_NAMES.contains(alg); diff --git a/impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy index 8fe83c19..0fa2786f 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy @@ -652,10 +652,8 @@ class JwtsTest { try { testEC(alg, true) fail("EC private keys cannot be used to validate EC signatures.") - } catch (UnsupportedJwtException e) { - String msg = "${alg.getId()} verification keys must be PublicKeys (implement java.security.PublicKey). " + - "Provided key type: sun.security.ec.ECPrivateKeyImpl." - assertEquals msg, e.cause.message + } catch (IllegalArgumentException e) { + assertEquals DefaultJwtParser.PRIV_KEY_VERIFY_MSG, e.getMessage() } } @@ -1211,7 +1209,7 @@ class JwtsTest { // Assert that the server does not recognized the forged token: try { - Jwts.parser().verifyWith(privateKey).build().parse(forged) + Jwts.parser().verifyWith(publicKey).build().parse(forged) fail("Forged token must not be successfully parsed.") } catch (UnsupportedJwtException expected) { assertTrue expected.getMessage().startsWith('The parsed JWT indicates it was signed with the') diff --git a/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractSecureDigestAlgorithmTest.groovy b/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractSecureDigestAlgorithmTest.groovy index 28829777..3e5bec6c 100644 --- a/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractSecureDigestAlgorithmTest.groovy +++ b/impl/src/test/groovy/io/jsonwebtoken/impl/security/AbstractSecureDigestAlgorithmTest.groovy @@ -92,5 +92,10 @@ class AbstractSecureDigestAlgorithmTest { protected byte[] doDigest(SecureRequest request) throws Exception { return new byte[1] } + + @Override + protected boolean doVerify(VerifySecureDigestRequest request) { + return false + } } } diff --git a/impl/src/test/groovy/io/jsonwebtoken/issues/Issue365Test.groovy b/impl/src/test/groovy/io/jsonwebtoken/issues/Issue365Test.groovy new file mode 100644 index 00000000..09e15f50 --- /dev/null +++ b/impl/src/test/groovy/io/jsonwebtoken/issues/Issue365Test.groovy @@ -0,0 +1,147 @@ +/* + * Copyright © 2023 jsonwebtoken.io + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.jsonwebtoken.issues + +import io.jsonwebtoken.Header +import io.jsonwebtoken.Jwts +import io.jsonwebtoken.Locator +import io.jsonwebtoken.impl.DefaultJwtBuilder +import io.jsonwebtoken.impl.DefaultJwtParser +import io.jsonwebtoken.impl.security.TestKeys +import io.jsonwebtoken.impl.security.TestPrivateKey +import io.jsonwebtoken.impl.security.TestPublicKey +import io.jsonwebtoken.security.InvalidKeyException +import io.jsonwebtoken.security.KeyAlgorithm +import io.jsonwebtoken.security.SignatureAlgorithm +import org.junit.Test + +import java.security.Key +import java.security.PrivateKey +import java.security.PublicKey + +import static org.junit.Assert.assertEquals +import static org.junit.Assert.fail + +class Issue365Test { + + + private static final Collection sigalgs() { + def algs = Jwts.SIG.get().values() + .findAll({ it -> it instanceof SignatureAlgorithm }) + return algs as Collection + } + + private static final Collection> asymKeyAlgs() { + def algs = Jwts.KEY.get().values() + .findAll({ it -> it.id.startsWith('R') || it.id.startsWith('E') }) + return algs as Collection> + } + + private static final Collection sigalgs = sigalgs() + + private static final Collection> asymKeyAlgs = asymKeyAlgs() + + @Test + void testSignWithPublicKey() { + + + for (def alg : sigalgs) { + def pair = TestKeys.forAlgorithm(alg).pair + try { + Jwts.builder().issuer('me').signWith(pair.public, alg).compact() + fail() + } catch (IllegalArgumentException expected) { + assertEquals DefaultJwtBuilder.PUB_KEY_SIGN_MSG, expected.getMessage() + } + } + } + + @Test + void testVerifyWithPrivateKey() { + for (def alg : sigalgs) { + def pair = TestKeys.forAlgorithm(alg).pair + String jws = Jwts.builder().issuer('me').signWith(pair.private).compact() + try { + Jwts.parser().verifyWith(pair.private).build().parseClaimsJws(jws) + fail() + } catch (IllegalArgumentException expected) { + assertEquals DefaultJwtParser.PRIV_KEY_VERIFY_MSG, expected.getMessage() + } + } + } + + @Test + void testVerifyWithKeyLocatorPrivateKey() { + for (def alg : sigalgs) { + def pair = TestKeys.forAlgorithm(alg).pair + String jws = Jwts.builder().issuer('me').signWith(pair.private).compact() + try { + Jwts.parser().keyLocator(new Locator() { + @Override + Key locate(Header header) { + return pair.private + } + }) + .build().parseClaimsJws(jws) + fail() + } catch (InvalidKeyException expected) { + assertEquals DefaultJwtParser.PRIV_KEY_VERIFY_MSG, expected.getMessage() + } + } + } + + @Test + void testEncryptWithPrivateKey() { + for (def alg : asymKeyAlgs) { + try { + Jwts.builder().issuer('me').encryptWith(new TestPrivateKey(), alg, Jwts.ENC.A256GCM).compact() + fail() + } catch (IllegalArgumentException expected) { + assertEquals DefaultJwtBuilder.PRIV_KEY_ENC_MSG, expected.getMessage() + } + } + } + + @Test + void testDecryptWithPublicKey() { + def pub = TestKeys.RS256.pair.public + String jwe = Jwts.builder().issuer('me').encryptWith(pub, Jwts.KEY.RSA1_5, Jwts.ENC.A256GCM).compact() + try { + Jwts.parser().decryptWith(new TestPublicKey()).build().parseClaimsJwe(jwe) + fail() + } catch (IllegalArgumentException expected) { + assertEquals DefaultJwtParser.PUB_KEY_DECRYPT_MSG, expected.getMessage() + } + } + + @Test + void testDecryptWithKeyLocatorPublicKey() { + def pub = TestKeys.RS256.pair.public + String jwe = Jwts.builder().issuer('me').encryptWith(pub, Jwts.KEY.RSA1_5, Jwts.ENC.A256GCM).compact() + try { + Jwts.parser().keyLocator(new Locator() { + @Override + Key locate(Header header) { + return pub + } + }) + .build().parseClaimsJwe(jwe) + fail() + } catch (InvalidKeyException expected) { + assertEquals DefaultJwtParser.PUB_KEY_DECRYPT_MSG, expected.getMessage() + } + } +}