#18: enhanced code coverage. Added cobertura for code enforcement and reporting.

This commit is contained in:
Les Hazlewood 2015-05-11 13:21:22 -07:00
parent 66b30e2e10
commit 9f51760472
11 changed files with 199 additions and 34 deletions

67
pom.xml
View File

@ -276,6 +276,73 @@
</execution> </execution>
</executions> </executions>
</plugin> </plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>cobertura-maven-plugin</artifactId>
<version>2.7</version>
<configuration>
<instrumentation>
<excludes>
<exclude>io/jsonwebtoken/lang/*.class</exclude>
</excludes>
<ignores>
<ignore>io.jsonwebtoken.lang.*</ignore>
</ignores>
</instrumentation>
<check>
<lineRate>100</lineRate>
<branchRate>100</branchRate>
<packageLineRate>100</packageLineRate>
<packageBranchRate>100</packageBranchRate>
<haltOnFailure>true</haltOnFailure>
<regexes>
<regex>
<!-- Cannot get to 100% on DefaultClaims because of Cobertura bug w/ generics:
https://github.com/cobertura/cobertura/issues/207 -->
<pattern>io.jsonwebtoken.impl.DefaultClaims</pattern>
<lineRate>96</lineRate>
<branchRate>100</branchRate>
</regex>
<regex>
<pattern>io.jsonwebtoken.impl.Base64UrlCodec</pattern>
<lineRate>100</lineRate>
<branchRate>95</branchRate>
</regex>
<regex>
<pattern>io.jsonwebtoken.impl.DefaultJwtBuilder</pattern>
<lineRate>91</lineRate>
<branchRate>85</branchRate>
</regex>
<regex>
<pattern>io.jsonwebtoken.impl.DefaultJwtParser</pattern>
<lineRate>97</lineRate>
<branchRate>88</branchRate>
</regex>
<regex>
<pattern>io.jsonwebtoken.impl.crypto.EllipticCurveSignatureValidator</pattern>
<lineRate>95</lineRate>
<branchRate>100</branchRate>
</regex>
<regex>
<pattern>io.jsonwebtoken.impl.crypto.RsaSignatureValidator</pattern>
<lineRate>93</lineRate>
<branchRate>100</branchRate>
</regex>
</regexes>
</check>
<formats>
<format>html</format>
</formats>
</configuration>
<executions>
<execution>
<goals>
<goal>clean</goal>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin> <plugin>
<groupId>org.apache.maven.plugins</groupId> <groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-release-plugin</artifactId> <artifactId>maven-release-plugin</artifactId>

View File

@ -43,20 +43,22 @@ public class SigningKeyResolverAdapter implements SigningKeyResolver {
@Override @Override
public Key resolveSigningKey(JwsHeader header, Claims claims) { public Key resolveSigningKey(JwsHeader header, Claims claims) {
SignatureAlgorithm alg = SignatureAlgorithm.forName(header.getAlgorithm()); SignatureAlgorithm alg = SignatureAlgorithm.forName(header.getAlgorithm());
byte[] keyBytes = resolveSigningKeyBytes(header, claims); Assert.isTrue(alg.isHmac(), "The default resolveSigningKey(JwsHeader, Claims) implementation cannot be " +
Assert.isTrue(!alg.isRsa(), "resolveSigningKeyBytes(JwsHeader, Claims) cannot be used for RSA signatures. " + "used for asymmetric key algorithms (RSA, Elliptic Curve). " +
"Override the resolveSigningKey(JwsHeader, Claims) method instead and return a " + "Override the resolveSigningKey(JwsHeader, Claims) method instead and return a " +
"PublicKey or PrivateKey instance."); "Key instance appropriate for the " + alg.name() + " algorithm.");
byte[] keyBytes = resolveSigningKeyBytes(header, claims);
return new SecretKeySpec(keyBytes, alg.getJcaName()); return new SecretKeySpec(keyBytes, alg.getJcaName());
} }
@Override @Override
public Key resolveSigningKey(JwsHeader header, String plaintext) { public Key resolveSigningKey(JwsHeader header, String plaintext) {
SignatureAlgorithm alg = SignatureAlgorithm.forName(header.getAlgorithm()); SignatureAlgorithm alg = SignatureAlgorithm.forName(header.getAlgorithm());
byte[] keyBytes = resolveSigningKeyBytes(header, plaintext); Assert.isTrue(alg.isHmac(), "The default resolveSigningKey(JwsHeader, String) implementation cannot be " +
Assert.isTrue(!alg.isRsa(), "resolveSigningKeyBytes(JwsHeader, String) cannot be used for RSA signatures. " + "used for asymmetric key algorithms (RSA, Elliptic Curve). " +
"Override the resolveSigningKey(JwsHeader, String) method instead and return a " + "Override the resolveSigningKey(JwsHeader, String) method instead and return a " +
"PublicKey or PrivateKey instance."); "Key instance appropriate for the " + alg.name() + " algorithm.");
byte[] keyBytes = resolveSigningKeyBytes(header, plaintext);
return new SecretKeySpec(keyBytes, alg.getJcaName()); return new SecretKeySpec(keyBytes, alg.getJcaName());
} }
@ -65,7 +67,8 @@ public class SigningKeyResolverAdapter implements SigningKeyResolver {
* key bytes. This implementation simply throws an exception: if the JWS parsed is a Claims JWS, you must * key bytes. This implementation simply throws an exception: if the JWS parsed is a Claims JWS, you must
* override this method or the {@link #resolveSigningKey(JwsHeader, Claims)} method instead. * override this method or the {@link #resolveSigningKey(JwsHeader, Claims)} method instead.
* *
* <p><b>NOTE:</b> You cannot override this method when validating RSA signatures. If you expect RSA signatures, </p> * <p><b>NOTE:</b> You cannot override this method when validating RSA signatures. If you expect RSA signatures,
* you must override the {@link #resolveSigningKey(JwsHeader, Claims)} method instead.</p>
* *
* @param header the parsed {@link JwsHeader} * @param header the parsed {@link JwsHeader}
* @param claims the parsed {@link Claims} * @param claims the parsed {@link Claims}
@ -74,7 +77,7 @@ public class SigningKeyResolverAdapter implements SigningKeyResolver {
public byte[] resolveSigningKeyBytes(JwsHeader header, Claims claims) { public byte[] resolveSigningKeyBytes(JwsHeader header, Claims claims) {
throw new UnsupportedJwtException("The specified SigningKeyResolver implementation does not support " + throw new UnsupportedJwtException("The specified SigningKeyResolver implementation does not support " +
"Claims JWS signing key resolution. Consider overriding either the " + "Claims JWS signing key resolution. Consider overriding either the " +
"resolveSigningKey(JwsHeader, Claims) or " + "resolveSigningKey(JwsHeader, Claims) method or, for HMAC algorithms, the " +
"resolveSigningKeyBytes(JwsHeader, Claims) method."); "resolveSigningKeyBytes(JwsHeader, Claims) method.");
} }
@ -90,7 +93,7 @@ public class SigningKeyResolverAdapter implements SigningKeyResolver {
public byte[] resolveSigningKeyBytes(JwsHeader header, String payload) { public byte[] resolveSigningKeyBytes(JwsHeader header, String payload) {
throw new UnsupportedJwtException("The specified SigningKeyResolver implementation does not support " + throw new UnsupportedJwtException("The specified SigningKeyResolver implementation does not support " +
"plaintext JWS signing key resolution. Consider overriding either the " + "plaintext JWS signing key resolution. Consider overriding either the " +
"resolveSigningKey(JwsHeader, String) or " + "resolveSigningKey(JwsHeader, String) method or, for HMAC algorithms, the " +
"resolveSigningKeyBytes(JwsHeader, String) method."); "resolveSigningKeyBytes(JwsHeader, String) method.");
} }
} }

View File

@ -36,7 +36,6 @@ import java.security.Key;
import java.util.Date; import java.util.Date;
import java.util.Map; import java.util.Map;
@SuppressWarnings("unchecked")
public class DefaultJwtBuilder implements JwtBuilder { public class DefaultJwtBuilder implements JwtBuilder {
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

View File

@ -55,7 +55,8 @@ public abstract class EllipticCurveProvider extends SignatureProvider {
} }
public static KeyPair generateKeyPair(String jcaAlgorithmName, String jcaProviderName, SignatureAlgorithm alg, SecureRandom random) { public static KeyPair generateKeyPair(String jcaAlgorithmName, String jcaProviderName, SignatureAlgorithm alg, SecureRandom random) {
Assert.isTrue(alg != null && alg.isEllipticCurve(), "SignatureAlgorithm argument must represent an Elliptic Curve algorithm."); Assert.notNull(alg, "SignatureAlgorithm argument cannot be null.");
Assert.isTrue(alg.isEllipticCurve(), "SignatureAlgorithm argument must represent an Elliptic Curve algorithm.");
try { try {
KeyPairGenerator g = KeyPairGenerator.getInstance(jcaAlgorithmName, jcaProviderName); KeyPairGenerator g = KeyPairGenerator.getInstance(jcaAlgorithmName, jcaProviderName);
String paramSpecCurveName = EC_CURVE_NAMES.get(alg); String paramSpecCurveName = EC_CURVE_NAMES.get(alg);

View File

@ -23,20 +23,15 @@ import java.security.InvalidKeyException;
import java.security.Key; import java.security.Key;
import java.security.PublicKey; import java.security.PublicKey;
import java.security.Signature; import java.security.Signature;
import java.security.interfaces.ECPrivateKey;
import java.security.interfaces.ECPublicKey; import java.security.interfaces.ECPublicKey;
public class EllipticCurveSignatureValidator extends EllipticCurveProvider implements SignatureValidator { public class EllipticCurveSignatureValidator extends EllipticCurveProvider implements SignatureValidator {
private static final String NO_EC_PRIVATE_KEY_MSG =
"Elliptic Curve signature validation requires an ECPublicKey. ECPrivateKeys may not be used.";
private static final String EC_PUBLIC_KEY_REQD_MSG = private static final String EC_PUBLIC_KEY_REQD_MSG =
"Elliptic Curve Signature validation requires either an ECPublicKey instance."; "Elliptic Curve signature validation requires an ECPublicKey instance.";
public EllipticCurveSignatureValidator(SignatureAlgorithm alg, Key key) { public EllipticCurveSignatureValidator(SignatureAlgorithm alg, Key key) {
super(alg, key); super(alg, key);
Assert.isTrue(!(key instanceof ECPrivateKey), NO_EC_PRIVATE_KEY_MSG);
Assert.isTrue(key instanceof ECPublicKey, EC_PUBLIC_KEY_REQD_MSG); Assert.isTrue(key instanceof ECPublicKey, EC_PUBLIC_KEY_REQD_MSG);
} }

View File

@ -50,7 +50,7 @@ public class RsaSignatureValidator extends RsaProvider implements SignatureValid
throw new SignatureException(msg, e); throw new SignatureException(msg, e);
} }
} else { } else {
assert this.SIGNER != null; Assert.notNull(this.SIGNER, "RSA Signer instance cannot be null. This is a bug. Please report it.");
byte[] computed = this.SIGNER.sign(data); byte[] computed = this.SIGNER.sign(data);
return Arrays.equals(computed, signature); return Arrays.equals(computed, signature);
} }

View File

@ -16,11 +16,11 @@
package io.jsonwebtoken package io.jsonwebtoken
import io.jsonwebtoken.impl.TextCodec import io.jsonwebtoken.impl.TextCodec
import org.junit.Test
import javax.crypto.spec.SecretKeySpec import javax.crypto.spec.SecretKeySpec
import java.security.SecureRandom import java.security.SecureRandom
import org.junit.Test
import static org.junit.Assert.* import static org.junit.Assert.*
class JwtParserTest { class JwtParserTest {
@ -638,8 +638,8 @@ class JwtParserTest {
fail() fail()
} catch (UnsupportedJwtException ex) { } catch (UnsupportedJwtException ex) {
assertEquals ex.getMessage(), 'The specified SigningKeyResolver implementation does not support ' + assertEquals ex.getMessage(), 'The specified SigningKeyResolver implementation does not support ' +
'Claims JWS signing key resolution. Consider overriding either the ' + 'Claims JWS signing key resolution. Consider overriding either the resolveSigningKey(JwsHeader, Claims) method ' +
'resolveSigningKey(JwsHeader, Claims) or resolveSigningKeyBytes(JwsHeader, Claims) method.' 'or, for HMAC algorithms, the resolveSigningKeyBytes(JwsHeader, Claims) method.'
} }
} }
@ -708,8 +708,8 @@ class JwtParserTest {
fail() fail()
} catch (UnsupportedJwtException ex) { } catch (UnsupportedJwtException ex) {
assertEquals ex.getMessage(), 'The specified SigningKeyResolver implementation does not support plaintext ' + assertEquals ex.getMessage(), 'The specified SigningKeyResolver implementation does not support plaintext ' +
'JWS signing key resolution. Consider overriding either the ' + 'JWS signing key resolution. Consider overriding either the resolveSigningKey(JwsHeader, String) ' +
'resolveSigningKey(JwsHeader, String) or resolveSigningKeyBytes(JwsHeader, String) method.' 'method or, for HMAC algorithms, the resolveSigningKeyBytes(JwsHeader, String) method.'
} }
} }
} }

View File

@ -22,6 +22,7 @@ import io.jsonwebtoken.impl.TextCodec
import io.jsonwebtoken.impl.crypto.EllipticCurveProvider import io.jsonwebtoken.impl.crypto.EllipticCurveProvider
import io.jsonwebtoken.impl.crypto.MacProvider import io.jsonwebtoken.impl.crypto.MacProvider
import io.jsonwebtoken.impl.crypto.RsaProvider import io.jsonwebtoken.impl.crypto.RsaProvider
import org.junit.Test
import javax.crypto.Mac import javax.crypto.Mac
import javax.crypto.spec.SecretKeySpec import javax.crypto.spec.SecretKeySpec
@ -30,7 +31,6 @@ import java.security.KeyPair
import java.security.PrivateKey import java.security.PrivateKey
import java.security.PublicKey import java.security.PublicKey
import org.junit.Test
import static org.junit.Assert.* import static org.junit.Assert.*
class JwtsTest { class JwtsTest {
@ -401,7 +401,7 @@ class JwtsTest {
testEC(SignatureAlgorithm.ES256, true) testEC(SignatureAlgorithm.ES256, true)
fail("EC private keys cannot be used to validate EC signatures.") fail("EC private keys cannot be used to validate EC signatures.")
} catch (UnsupportedJwtException e) { } catch (UnsupportedJwtException e) {
assertEquals e.cause.message, "Elliptic Curve signature validation requires an ECPublicKey. ECPrivateKeys may not be used." assertEquals e.cause.message, "Elliptic Curve signature validation requires an ECPublicKey instance."
} }
} }
@ -520,6 +520,39 @@ class JwtsTest {
} }
} }
//Asserts correct behavior for https://github.com/jwtk/jjwt/issues/25
@Test
void testParseForgedEllipticCurvePublicKeyAsHmacToken() {
//Create a legitimate RSA public and private key pair:
KeyPair kp = EllipticCurveProvider.generateKeyPair()
PublicKey publicKey = kp.getPublic();
//PrivateKey privateKey = kp.getPrivate();
ObjectMapper om = new ObjectMapper()
String header = TextCodec.BASE64URL.encode(om.writeValueAsString(['alg': 'HS256']))
String body = TextCodec.BASE64URL.encode(om.writeValueAsString('foo'))
String compact = header + '.' + body + '.'
// Now for the forgery: simulate an attacker using the Elliptic Curve public key to sign a token, but
// using it as an HMAC signing key instead of Elliptic Curve:
Mac mac = Mac.getInstance('HmacSHA256');
mac.init(new SecretKeySpec(publicKey.getEncoded(), 'HmacSHA256'));
byte[] signatureBytes = mac.doFinal(compact.getBytes(Charset.forName('US-ASCII')))
String encodedSignature = TextCodec.BASE64URL.encode(signatureBytes);
//Finally, the forged token is the header + body + forged signature:
String forged = compact + encodedSignature;
// Assert that the parser does not recognized the forged token:
try {
Jwts.parser().setSigningKey(publicKey).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')
}
}
static void testRsa(SignatureAlgorithm alg, int keySize=1024, boolean verifyWithPrivateKey=false) { static void testRsa(SignatureAlgorithm alg, int keySize=1024, boolean verifyWithPrivateKey=false) {
KeyPair kp = RsaProvider.generateKeyPair(keySize) KeyPair kp = RsaProvider.generateKeyPair(keySize)

View File

@ -0,0 +1,42 @@
/*
* Copyright (C) 2015 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
import io.jsonwebtoken.impl.crypto.RsaProvider
import org.junit.Test
import static org.junit.Assert.assertEquals
import static org.junit.Assert.fail
class SigningKeyResolverAdapterTest {
@Test
void testResolveClaimsSigningKeyWithRsaKey() {
def pair = RsaProvider.generateKeyPair(1024) //real apps should use 4096 or better. We're only reducing the size here so the tests are fast
def compact = Jwts.builder().claim('foo', 'bar').signWith(SignatureAlgorithm.RS256, pair.private).compact()
Jws<Claims> jws = Jwts.parser().setSigningKey(pair.public).parseClaimsJws(compact)
try {
new SigningKeyResolverAdapter().resolveSigningKey(jws.header, jws.body)
fail()
} catch (IllegalArgumentException iae) {
assertEquals "The default resolveSigningKey(JwsHeader, Claims) implementation cannot be used for asymmetric key algorithms (RSA, Elliptic Curve). Override the resolveSigningKey(JwsHeader, Claims) method instead and return a Key instance appropriate for the RS256 algorithm.", iae.message
}
}
}

View File

@ -16,13 +16,13 @@
package io.jsonwebtoken.impl.crypto package io.jsonwebtoken.impl.crypto
import io.jsonwebtoken.SignatureAlgorithm import io.jsonwebtoken.SignatureAlgorithm
import org.junit.Test
import java.security.KeyPair import java.security.KeyPair
import java.security.NoSuchProviderException import java.security.NoSuchProviderException
import java.security.interfaces.ECPrivateKey import java.security.interfaces.ECPrivateKey
import java.security.interfaces.ECPublicKey import java.security.interfaces.ECPublicKey
import org.junit.Test
import static org.junit.Assert.* import static org.junit.Assert.*
class EllipticCurveProviderTest { class EllipticCurveProviderTest {
@ -45,4 +45,24 @@ class EllipticCurveProviderTest {
assertTrue ise.cause instanceof NoSuchProviderException assertTrue ise.cause instanceof NoSuchProviderException
} }
} }
@Test
void testGenerateKeyPairWithNullAlgorithm() {
try {
EllipticCurveProvider.generateKeyPair("ECDSA", "Foo", null, null)
fail()
} catch (IllegalArgumentException ise) {
assertEquals ise.message, "SignatureAlgorithm argument cannot be null."
}
}
@Test
void testGenerateKeyPairWithNonEllipticCurveAlgorithm() {
try {
EllipticCurveProvider.generateKeyPair("ECDSA", "Foo", SignatureAlgorithm.HS256, null)
fail()
} catch (IllegalArgumentException ise) {
assertEquals ise.message, "SignatureAlgorithm argument must represent an Elliptic Curve algorithm."
}
}
} }

View File

@ -17,21 +17,26 @@ package io.jsonwebtoken.impl.crypto
import io.jsonwebtoken.SignatureAlgorithm import io.jsonwebtoken.SignatureAlgorithm
import io.jsonwebtoken.SignatureException import io.jsonwebtoken.SignatureException
import java.security.InvalidKeyException
import java.security.KeyPair
import java.security.KeyPairGenerator
import java.security.PrivateKey
import java.security.PublicKey
import java.security.Signature
import org.junit.Test import org.junit.Test
import java.security.*
import static org.junit.Assert.* import static org.junit.Assert.*
class RsaSignatureValidatorTest { class RsaSignatureValidatorTest {
private static final Random rng = new Random(); //doesn't need to be secure - we're just testing private static final Random rng = new Random(); //doesn't need to be secure - we're just testing
@Test
void testConstructorWithNonRsaKey() {
try {
new RsaSignatureValidator(SignatureAlgorithm.RS256, MacProvider.generateKey());
fail()
} catch (IllegalArgumentException iae) {
assertEquals "RSA Signature validation requires either a RSAPublicKey or RSAPrivateKey instance.", iae.message
}
}
@Test @Test
void testDoVerifyWithInvalidKeyException() { void testDoVerifyWithInvalidKeyException() {