From c29c6a2fe825b125d1d9b2f5580b6a32dae8b3bb Mon Sep 17 00:00:00 2001 From: Les Hazlewood Date: Tue, 28 Apr 2015 19:39:59 -0700 Subject: [PATCH] Implemented issue #25 --- .../jsonwebtoken/impl/DefaultJwtBuilder.java | 13 +- .../jsonwebtoken/impl/DefaultJwtParser.java | 22 ++- .../jsonwebtoken/impl/crypto/MacSigner.java | 8 ++ .../impl/crypto/RsaSignatureValidator.java | 28 ++-- .../jsonwebtoken/impl/crypto/RsaSigner.java | 9 +- .../groovy/io/jsonwebtoken/JwtsTest.groovy | 63 ++++++++- .../crypto/RsaSignatureValidatorTest.groovy | 71 ++++++++++ .../impl/crypto/RsaSignerTest.groovy | 130 ++++++++++++++++++ 8 files changed, 322 insertions(+), 22 deletions(-) create mode 100644 src/test/groovy/io/jsonwebtoken/impl/crypto/RsaSignatureValidatorTest.groovy create mode 100644 src/test/groovy/io/jsonwebtoken/impl/crypto/RsaSignerTest.groovy diff --git a/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java b/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java index e1610ab4..e39ca072 100644 --- a/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java +++ b/src/main/java/io/jsonwebtoken/impl/DefaultJwtBuilder.java @@ -19,10 +19,10 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import io.jsonwebtoken.Claims; import io.jsonwebtoken.Header; -import io.jsonwebtoken.Jwts; import io.jsonwebtoken.JwsHeader; import io.jsonwebtoken.JwtBuilder; import io.jsonwebtoken.JwtParser; +import io.jsonwebtoken.Jwts; import io.jsonwebtoken.SignatureAlgorithm; import io.jsonwebtoken.impl.crypto.DefaultJwtSigner; import io.jsonwebtoken.impl.crypto.JwtSigner; @@ -91,7 +91,7 @@ public class DefaultJwtBuilder implements JwtBuilder { public JwtBuilder signWith(SignatureAlgorithm alg, byte[] secretKey) { Assert.notNull(alg, "SignatureAlgorithm cannot be null."); Assert.notEmpty(secretKey, "secret key byte array cannot be null or empty."); - Assert.isTrue(!alg.isRsa(), "Key bytes cannot be specified for RSA signatures. Please specify an RSA PrivateKey instance."); + Assert.isTrue(!alg.isRsa(), "Key bytes cannot be specified for RSA signatures. Please specify an RSAPrivateKey instance."); this.algorithm = alg; this.keyBytes = secretKey; return this; @@ -289,7 +289,7 @@ public class DefaultJwtBuilder implements JwtBuilder { if (key != null) { //jwt must be signed: - JwtSigner signer = new DefaultJwtSigner(algorithm, key); + JwtSigner signer = createSigner(algorithm, key); String base64UrlSignature = signer.sign(jwt); @@ -303,6 +303,13 @@ public class DefaultJwtBuilder implements JwtBuilder { return jwt; } + /* + * @since 0.5 mostly to allow testing overrides + */ + protected JwtSigner createSigner(SignatureAlgorithm alg, Key key) { + return new DefaultJwtSigner(alg, key); + } + public static String base64UrlEncode(Object o, String errMsg) { String s; try { diff --git a/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java b/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java index 5a2626a8..ed757e5d 100644 --- a/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java +++ b/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java @@ -235,7 +235,20 @@ public class DefaultJwtParser implements JwtParser { //re-create the jwt part without the signature. This is what needs to be signed for verification: String jwtWithoutSignature = base64UrlEncodedHeader + SEPARATOR_CHAR + base64UrlEncodedPayload; - JwtSignatureValidator validator = new DefaultJwtSignatureValidator(algorithm, key); + JwtSignatureValidator validator; + try { + validator = createSignatureValidator(algorithm, key); + } catch (IllegalArgumentException e) { + String algName = algorithm.getValue(); + String msg = "The parsed JWT indicates it was signed with the " + algName + " signature " + + "algorithm, but the specified signing key of type " + key.getClass().getName() + + " may not be used to verify " + algName + " signatures. Because the specified " + + "signing key reflects a specific and expected algorithm, and the JWT does not reflect " + + "this algorithm, it is likely that the JWT was not expected and therefore should not be " + + "trusted. Another possibility is that the parser was configured with the incorrect " + + "signing key, but this cannot be assumed for security reasons."; + throw new UnsupportedJwtException(msg, e); + } if (!validator.isValid(jwtWithoutSignature, base64UrlEncodedDigest)) { String msg = "JWT signature does not match locally computed signature. JWT validity cannot be " + @@ -296,6 +309,13 @@ public class DefaultJwtParser implements JwtParser { } } + /* + * @since 0.5 mostly to allow testing overrides + */ + protected JwtSignatureValidator createSignatureValidator(SignatureAlgorithm alg, Key key) { + return new DefaultJwtSignatureValidator(alg, key); + } + @Override public T parse(String compact, JwtHandler handler) throws ExpiredJwtException, MalformedJwtException, SignatureException { diff --git a/src/main/java/io/jsonwebtoken/impl/crypto/MacSigner.java b/src/main/java/io/jsonwebtoken/impl/crypto/MacSigner.java index f5383045..46ad8c43 100644 --- a/src/main/java/io/jsonwebtoken/impl/crypto/MacSigner.java +++ b/src/main/java/io/jsonwebtoken/impl/crypto/MacSigner.java @@ -17,8 +17,10 @@ package io.jsonwebtoken.impl.crypto; import io.jsonwebtoken.SignatureAlgorithm; import io.jsonwebtoken.SignatureException; +import io.jsonwebtoken.lang.Assert; import javax.crypto.Mac; +import javax.crypto.SecretKey; import javax.crypto.spec.SecretKeySpec; import java.security.InvalidKeyException; import java.security.Key; @@ -32,6 +34,12 @@ public class MacSigner extends MacProvider implements Signer { public MacSigner(SignatureAlgorithm alg, Key key) { super(alg, key); + Assert.isTrue(alg.isHmac(), "The MacSigner only supports HMAC signature algorithms."); + if (!(key instanceof SecretKey)) { + String msg = "MAC signatures must be computed and verified using a SecretKey. The specified key of " + + "type " + key.getClass().getName() + " is not a SecretKey."; + throw new IllegalArgumentException(msg); + } } @Override diff --git a/src/main/java/io/jsonwebtoken/impl/crypto/RsaSignatureValidator.java b/src/main/java/io/jsonwebtoken/impl/crypto/RsaSignatureValidator.java index 825c5d31..8f3e2687 100644 --- a/src/main/java/io/jsonwebtoken/impl/crypto/RsaSignatureValidator.java +++ b/src/main/java/io/jsonwebtoken/impl/crypto/RsaSignatureValidator.java @@ -19,38 +19,48 @@ import io.jsonwebtoken.SignatureAlgorithm; import io.jsonwebtoken.SignatureException; import io.jsonwebtoken.lang.Assert; +import java.security.InvalidKeyException; import java.security.Key; -import java.security.PrivateKey; import java.security.PublicKey; import java.security.Signature; +import java.security.interfaces.RSAPrivateKey; +import java.security.interfaces.RSAPublicKey; import java.util.Arrays; public class RsaSignatureValidator extends RsaProvider implements SignatureValidator { + private final RsaSigner SIGNER; + public RsaSignatureValidator(SignatureAlgorithm alg, Key key) { super(alg, key); - Assert.isTrue(key instanceof PrivateKey || key instanceof PublicKey, - "RSA Signature validation requires either a PublicKey or PrivateKey instance."); + Assert.isTrue(key instanceof RSAPrivateKey || key instanceof RSAPublicKey, + "RSA Signature validation requires either a RSAPublicKey or RSAPrivateKey instance."); + this.SIGNER = key instanceof RSAPrivateKey ? new RsaSigner(alg, key) : null; } @Override public boolean isValid(byte[] data, byte[] signature) { - if (key instanceof PublicKey) { Signature sig = createSignatureInstance(); PublicKey publicKey = (PublicKey) key; try { - sig.initVerify(publicKey); - sig.update(data); - return sig.verify(signature); + return doVerify(sig, publicKey, data, signature); } catch (Exception e) { - String msg = "Unable to verify RSA signature using configured PublicKey. " + e.getMessage(); + String msg = "Unable to verify RSA signature using configured PublicKey. " + e.getMessage(); throw new SignatureException(msg, e); } } else { - byte[] computed = new RsaSigner(alg, key).sign(data); + assert this.SIGNER != null; + byte[] computed = this.SIGNER.sign(data); return Arrays.equals(computed, signature); } } + protected boolean doVerify(Signature sig, PublicKey publicKey, byte[] data, byte[] signature) + throws InvalidKeyException, java.security.SignatureException { + sig.initVerify(publicKey); + sig.update(data); + return sig.verify(signature); + } + } diff --git a/src/main/java/io/jsonwebtoken/impl/crypto/RsaSigner.java b/src/main/java/io/jsonwebtoken/impl/crypto/RsaSigner.java index 73d5a8fa..3a3849df 100644 --- a/src/main/java/io/jsonwebtoken/impl/crypto/RsaSigner.java +++ b/src/main/java/io/jsonwebtoken/impl/crypto/RsaSigner.java @@ -17,18 +17,22 @@ package io.jsonwebtoken.impl.crypto; import io.jsonwebtoken.SignatureAlgorithm; import io.jsonwebtoken.SignatureException; -import io.jsonwebtoken.lang.Assert; import java.security.InvalidKeyException; import java.security.Key; import java.security.PrivateKey; import java.security.Signature; +import java.security.interfaces.RSAPrivateKey; public class RsaSigner extends RsaProvider implements Signer { public RsaSigner(SignatureAlgorithm alg, Key key) { super(alg, key); - Assert.isInstanceOf(PrivateKey.class, key, "RSA signatures be computed using a PrivateKey."); + if (!(key instanceof RSAPrivateKey)) { + String msg = "RSA signatures must be computed using an RSAPrivateKey. The specified key of type " + + key.getClass().getName() + " is not an RSAPrivateKey."; + throw new IllegalArgumentException(msg); + } } @Override @@ -43,7 +47,6 @@ public class RsaSigner extends RsaProvider implements Signer { } protected byte[] doSign(byte[] data) throws InvalidKeyException, java.security.SignatureException { - Assert.isInstanceOf(PrivateKey.class, key, "RSA signatures be computed using a PrivateKey."); PrivateKey privateKey = (PrivateKey)key; Signature sig = createSignatureInstance(); sig.initSign(privateKey); diff --git a/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy b/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy index 8a1ea437..7ab09336 100644 --- a/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy +++ b/src/test/groovy/io/jsonwebtoken/JwtsTest.groovy @@ -15,10 +15,15 @@ */ package io.jsonwebtoken +import com.fasterxml.jackson.databind.ObjectMapper import io.jsonwebtoken.impl.DefaultHeader import io.jsonwebtoken.impl.DefaultJwsHeader +import io.jsonwebtoken.impl.TextCodec import org.testng.annotations.Test +import javax.crypto.Mac +import javax.crypto.spec.SecretKeySpec +import java.nio.charset.Charset import java.security.KeyPair import java.security.KeyPairGenerator import java.security.PrivateKey @@ -422,9 +427,9 @@ class JwtsTest { } } - //Asserts correct/expected behavior discussed in https://github.com/jwtk/jjwt/issues/20 + //Asserts correct/expected behavior discussed in https://github.com/jwtk/jjwt/issues/20 and https://github.com/jwtk/jjwt/issues/25 @Test - void testForgedTokenWhenUsingRsaPublicKeyAsHmacSigningKey() { + void testParseForgedRsaPublicKeyAsHmacTokenVerifiedWithTheRsaPrivateKey() { //Create a legitimate RSA public and private key pair: KeyPairGenerator keyGenerator = KeyPairGenerator.getInstance("RSA"); @@ -433,16 +438,62 @@ class JwtsTest { PublicKey publicKey = kp.getPublic(); PrivateKey privateKey = kp.getPrivate(); - // Now for the forgery: simulate a client using the RSA public key to sign a token, but + 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 RSA public key to sign a token, but // using it as an HMAC signing key instead of RSA: - String forged = Jwts.builder().setPayload('foo').signWith(SignatureAlgorithm.HS256, publicKey).compact(); + 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 server (that should always use the private key) does not recognized the forged token: try { Jwts.parser().setSigningKey(privateKey).parse(forged); fail("Forged token must not be successfully parsed.") - } catch (SignatureException expected) { - assertEquals expected.getMessage(), 'JWT signature does not match locally computed signature. JWT validity cannot be asserted and should not be trusted.' + } catch (UnsupportedJwtException expected) { + assertTrue expected.getMessage().startsWith('The parsed JWT indicates it was signed with the') + } + } + + //Asserts correct behavior for https://github.com/jwtk/jjwt/issues/25 + @Test + void testParseForgedRsaPublicKeyAsHmacTokenVerifiedWithTheRsaPublicKey() { + + //Create a legitimate RSA public and private key pair: + KeyPairGenerator keyGenerator = KeyPairGenerator.getInstance("RSA"); + keyGenerator.initialize(1024); + KeyPair kp = keyGenerator.genKeyPair(); + 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 RSA public key to sign a token, but + // using it as an HMAC signing key instead of RSA: + 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') } } diff --git a/src/test/groovy/io/jsonwebtoken/impl/crypto/RsaSignatureValidatorTest.groovy b/src/test/groovy/io/jsonwebtoken/impl/crypto/RsaSignatureValidatorTest.groovy new file mode 100644 index 00000000..bb17fdd4 --- /dev/null +++ b/src/test/groovy/io/jsonwebtoken/impl/crypto/RsaSignatureValidatorTest.groovy @@ -0,0 +1,71 @@ +/* + * 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.impl.crypto + +import io.jsonwebtoken.SignatureAlgorithm +import io.jsonwebtoken.SignatureException +import org.testng.annotations.Test + +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 static org.testng.Assert.assertEquals +import static org.testng.Assert.assertSame +import static org.testng.Assert.fail + + +class RsaSignatureValidatorTest { + + private static final Random rng = new Random(); //doesn't need to be secure - we're just testing + + @Test + void testDoVerifyWithInvalidKeyException() { + + KeyPairGenerator keyGenerator = KeyPairGenerator.getInstance("RSA"); + keyGenerator.initialize(1024); + + KeyPair kp = keyGenerator.genKeyPair(); + PublicKey publicKey = kp.getPublic(); + PrivateKey privateKey = kp.getPrivate(); + + String msg = 'foo' + final InvalidKeyException ex = new InvalidKeyException(msg) + + RsaSignatureValidator v = new RsaSignatureValidator(SignatureAlgorithm.RS256, publicKey) { + @Override + protected boolean doVerify(Signature sig, PublicKey pk, byte[] data, byte[] signature) throws InvalidKeyException, java.security.SignatureException { + throw ex; + } + } + + byte[] bytes = new byte[16] + byte[] signature = new byte[16] + rng.nextBytes(bytes) + rng.nextBytes(signature) + + try { + v.isValid(bytes, signature) + fail(); + } catch (SignatureException se) { + assertEquals se.message, 'Unable to verify RSA signature using configured PublicKey. ' + msg + assertSame se.cause, ex + } + } +} diff --git a/src/test/groovy/io/jsonwebtoken/impl/crypto/RsaSignerTest.groovy b/src/test/groovy/io/jsonwebtoken/impl/crypto/RsaSignerTest.groovy new file mode 100644 index 00000000..89401324 --- /dev/null +++ b/src/test/groovy/io/jsonwebtoken/impl/crypto/RsaSignerTest.groovy @@ -0,0 +1,130 @@ +/* + * 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.impl.crypto + +import io.jsonwebtoken.SignatureAlgorithm +import io.jsonwebtoken.SignatureException +import org.testng.annotations.Test + +import javax.crypto.spec.SecretKeySpec +import java.security.InvalidKeyException +import java.security.KeyPair +import java.security.KeyPairGenerator +import java.security.PrivateKey +import java.security.PublicKey + +import static org.testng.Assert.* + + +class RsaSignerTest { + + private static final Random rng = new Random(); //doesn't need to be secure - we're just testing + + @Test + void testConstructorWithoutRsaAlg() { + + byte[] bytes = new byte[16] + rng.nextBytes(bytes) + SecretKeySpec key = new SecretKeySpec(bytes, 'HmacSHA256') + + try { + new RsaSigner(SignatureAlgorithm.HS256, key); + fail('RsaSigner should reject non RSA algorithms.') + } catch (IllegalArgumentException expected) { + assertEquals expected.message, 'SignatureAlgorithm must be an RSASSA or RSASSA-PSS algorithm.'; + } + } + + @Test + void testConstructorWithoutRsaPrivateKey() { + + byte[] bytes = new byte[16] + rng.nextBytes(bytes) + SecretKeySpec key = new SecretKeySpec(bytes, 'HmacSHA256') + + try { + new RsaSigner(SignatureAlgorithm.RS256, key); + fail('RsaSigner should reject non RSAPrivateKey instances.') + } catch (IllegalArgumentException expected) { + assertEquals expected.message, "RSA signatures must be computed using an RSAPrivateKey. The specified key of type " + + key.getClass().getName() + " is not an RSAPrivateKey."; + } + } + + @Test + void testDoSignWithInvalidKeyException() { + + KeyPairGenerator keyGenerator = KeyPairGenerator.getInstance("RSA"); + keyGenerator.initialize(1024); + + KeyPair kp = keyGenerator.genKeyPair(); + PublicKey publicKey = kp.getPublic(); + PrivateKey privateKey = kp.getPrivate(); + + String msg = 'foo' + final InvalidKeyException ex = new InvalidKeyException(msg) + + RsaSigner signer = new RsaSigner(SignatureAlgorithm.RS256, privateKey) { + @Override + protected byte[] doSign(byte[] data) throws InvalidKeyException, java.security.SignatureException { + throw ex + } + } + + byte[] bytes = new byte[16] + rng.nextBytes(bytes) + + try { + signer.sign(bytes) + fail(); + } catch (SignatureException se) { + assertEquals se.message, 'Invalid RSA PrivateKey. ' + msg + assertSame se.cause, ex + } + } + + @Test + void testDoSignWithJdkSignatureException() { + + KeyPairGenerator keyGenerator = KeyPairGenerator.getInstance("RSA"); + keyGenerator.initialize(1024); + + KeyPair kp = keyGenerator.genKeyPair(); + PublicKey publicKey = kp.getPublic(); + PrivateKey privateKey = kp.getPrivate(); + + String msg = 'foo' + final java.security.SignatureException ex = new java.security.SignatureException(msg) + + RsaSigner signer = new RsaSigner(SignatureAlgorithm.RS256, privateKey) { + @Override + protected byte[] doSign(byte[] data) throws InvalidKeyException, java.security.SignatureException { + throw ex + } + } + + byte[] bytes = new byte[16] + rng.nextBytes(bytes) + + try { + signer.sign(bytes) + fail(); + } catch (SignatureException se) { + assertEquals se.message, 'Unable to calculate signature using RSA PrivateKey. ' + msg + assertSame se.cause, ex + } + } +}