Added regression tests for #365 (#812)

* Added regression tests for #365
* Removed default doDigest implementation from AbstractSecureDigestAlgorithm and put it in DefaultMacAlgorithm (where it should have been) since SignatureAlgorithms override it already
This commit is contained in:
lhazlewood 2023-09-05 18:29:55 -07:00 committed by GitHub
parent d54be182dc
commit a6792d938f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 207 additions and 27 deletions

View File

@ -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:

View File

@ -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<Key, ?>) 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<Key, ?>) keyAlg;
final KeyAlgorithm<Key, ?> alg = this.keyAlg;
final String cekMsg = "Unable to obtain content encryption key from key management algorithm '%s'.";
this.keyAlgFunction = Functions.wrap(new Function<KeyRequest<Key>, KeyResult>() {
@Override

View File

@ -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<Key> 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);

View File

@ -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;
}

View File

@ -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<S extends Key, V extends Key> extends CryptoAlgorithm implements SecureDigestAlgorithm<S, V> {
@ -75,14 +74,5 @@ abstract class AbstractSecureDigestAlgorithm<S extends Key, V extends Key> exten
}
}
protected boolean messageDigest(VerifySecureDigestRequest<V> 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<V> request) throws Exception {
return messageDigest(request);
}
protected abstract boolean doVerify(VerifySecureDigestRequest<V> request);
}

View File

@ -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<SecretKey,
}
});
}
protected boolean doVerify(VerifySecureDigestRequest<SecretKey> 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);
}
}

View File

@ -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);

View File

@ -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')

View File

@ -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
}
}
}

View File

@ -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<SignatureAlgorithm> sigalgs() {
def algs = Jwts.SIG.get().values()
.findAll({ it -> it instanceof SignatureAlgorithm })
return algs as Collection<SignatureAlgorithm>
}
private static final Collection<KeyAlgorithm<PublicKey, PrivateKey>> asymKeyAlgs() {
def algs = Jwts.KEY.get().values()
.findAll({ it -> it.id.startsWith('R') || it.id.startsWith('E') })
return algs as Collection<KeyAlgorithm<PublicKey, PrivateKey>>
}
private static final Collection<SignatureAlgorithm> sigalgs = sigalgs()
private static final Collection<KeyAlgorithm<PublicKey, PrivateKey>> 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<Key>() {
@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<Key>() {
@Override
Key locate(Header header) {
return pub
}
})
.build().parseClaimsJwe(jwe)
fail()
} catch (InvalidKeyException expected) {
assertEquals DefaultJwtParser.PUB_KEY_DECRYPT_MSG, expected.getMessage()
}
}
}