[java-oidc-common] branch main updated: Add more JWS Trust Engine verification tests
Phil Smart
philip.smart at jisc.ac.uk
Fri Oct 7 10:00:49 UTC 2022
This is an automated email from the git hooks/post-receive script.
philsmart pushed a commit to branch main
in repository java-oidc-common.
View the commit online:
http://git.shibboleth.net/view/?p=java-oidc-common.git;a=commit;h=ee165f39bac67adfc38436f08a4165dd3199e309
The following commit(s) were added to refs/heads/main by this push:
new ee165f3 Add more JWS Trust Engine verification tests
ee165f3 is described below
commit ee165f39bac67adfc38436f08a4165dd3199e309
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Oct 7 11:00:43 2022 +0100
Add more JWS Trust Engine verification tests
---
.../impl/ExplicitKeySignedJWTTrustEngine.java | 2 +-
.../impl/ExplicitKeySignedJWTTrustEngineTest.java | 252 ++++++++++++++++++++-
2 files changed, 249 insertions(+), 5 deletions(-)
diff --git a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/impl/ExplicitKeySignedJWTTrustEngine.java b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/impl/ExplicitKeySignedJWTTrustEngine.java
index 78ee161..2e25a0a 100644
--- a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/impl/ExplicitKeySignedJWTTrustEngine.java
+++ b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/impl/ExplicitKeySignedJWTTrustEngine.java
@@ -94,7 +94,7 @@ public class ExplicitKeySignedJWTTrustEngine extends BaseSignedJWTTrustEngine<It
criteriaSet.add(new UsageCriterion(UsageType.SIGNING));
}
- // TODO these need matching?
+ // TODO Do we need this?
final String jcaAlgorithm = AlgorithmSupport.getKeyAlgorithm(signedJWT.getHeader().getAlgorithm().getName());
if (!Strings.isNullOrEmpty(jcaAlgorithm)) {
criteriaSet.add(new KeyAlgorithmCriterion(jcaAlgorithm), true);
diff --git a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/impl/ExplicitKeySignedJWTTrustEngineTest.java b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/impl/ExplicitKeySignedJWTTrustEngineTest.java
index 36e7cb2..18847d3 100644
--- a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/impl/ExplicitKeySignedJWTTrustEngineTest.java
+++ b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/impl/ExplicitKeySignedJWTTrustEngineTest.java
@@ -1,3 +1,19 @@
+/*
+ * Licensed to the University Corporation for Advanced Internet Development,
+ * Inc. (UCAID) under one or more contributor license agreements. See the
+ * NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The UCAID licenses this file to You 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 net.shibboleth.oidc.security.impl;
@@ -15,6 +31,7 @@ import org.opensaml.security.credential.Credential;
import org.opensaml.security.credential.CredentialResolver;
import org.opensaml.security.credential.UsageType;
import org.opensaml.security.criteria.UsageCriterion;
+import org.opensaml.security.crypto.KeySupport;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
@@ -23,10 +40,15 @@ import com.nimbusds.jose.JOSEObjectType;
import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jose.JWSHeader;
import com.nimbusds.jose.crypto.ECDSASigner;
+import com.nimbusds.jose.crypto.MACSigner;
+import com.nimbusds.jose.crypto.RSASSASigner;
import com.nimbusds.jose.jwk.AsymmetricJWK;
import com.nimbusds.jose.jwk.Curve;
import com.nimbusds.jose.jwk.ECKey;
+import com.nimbusds.jose.jwk.OctetSequenceKey;
+import com.nimbusds.jose.jwk.RSAKey;
import com.nimbusds.jose.jwk.gen.ECKeyGenerator;
+import com.nimbusds.jose.util.StandardCharset;
import com.nimbusds.jwt.JWTClaimsSet;
import com.nimbusds.jwt.SignedJWT;
@@ -46,6 +68,18 @@ public class ExplicitKeySignedJWTTrustEngineTest {
private JOSEObjectCredentialResolver joseObjectCredResolver;
+ private final String CVE_2016_10555_SIGNING_CERT_PUBLIC_KEY =
+ "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtJSHqzkdHPY62Mvn80Wc\n"
+ + "vriYxHhh6N/x6mK0hd+J/awNqZm8se6LG0Pv35Xx1aftefLGnlgPho49Jf82NMT/\n"
+ + "MczlsHQesAFwotPr1rbqyvmNrbQTqxrAzoN2b5Cpu9ZPcFznOdtUWv+g+M/Xoqdp\n"
+ + "v8ZPXn1SMgfZJiGw+Mhdxyfv6TEAM8TUeqfYzVpuTmg/ns08ZsHmjxJIDlMrOt8o\n"
+ + "7cKDUK/kt0bfDDYZf5kjdFFF6qzZg0uo30L0qxLpJDr/OpLi904MP4H94rnWPXKE\n"
+ + "Ku8Yx+aYhYITnq5yyPiJpyHfgDj6MVlA1vUWqB9MwlvKOywLNCFfDZj6+TCjzCJF\n"
+ + "XQIDAQAB";
+
+ /** The client_secret.*/
+ private static final String CLIENT_SECRET = "Xp2s5v8y/B?E(H+MbQeThWmYq3t6w9z$";
+
@BeforeMethod
public void setup() throws JOSEException {
key = new ECKeyGenerator(Curve.P_256).keyID("123").generate();
@@ -81,12 +115,82 @@ public class ExplicitKeySignedJWTTrustEngineTest {
final CriteriaSet criteria = new CriteriaSet();
criteria.add(new UsageCriterion(UsageType.SIGNING));
+
final var valid = engine.validate(createECSignedJWT(key, key.getKeyID(), JWSAlgorithm.ES256,
"https://op.example.com/", "https://rp.example.com"),
criteria);
assertTrue(valid);
}
+ @Test
+ public void testSuccess_WithTrustedSymmetricKeyCredential() throws JOSEException, SecurityException {
+
+ final CriteriaSet criteria = new CriteriaSet();
+ criteria.add(new UsageCriterion(UsageType.SIGNING));
+
+ final var valid = engine.validate(createMACSignedJWT("SECRET", key.getKeyID(), JWSAlgorithm.HS256,
+ "https://op.example.com/", "https://rp.example.com"),
+ criteria);
+ assertTrue(valid);
+ }
+
+
+ /**
+ * Test for CVE-2016-10555. That is, can a malicious user choose which algorithm to verify the
+ * token with. If so, they could sign the JWT with the servers public key, change the
+ * algorithm to HS256 (or a MAC version) and have the server (if vulnerable) validate
+ * it against the public key it has locally.
+ *
+ * <p>This is only likely in cases where the implementation only expects to verify using
+ * a public key, and has a generic method that accepts the public key, but can actually
+ * use a MAC verification method - with the public key. So a fairly unusual case!.</p>
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void test_CVE_2016_10555() throws Exception {
+
+ credResolver = new CredentialResolver() {
+
+ @Override
+ public Credential resolveSingle(final CriteriaSet criteria) throws ResolverException {
+ final BasicJWKCredential jwkCredential = new BasicJWKCredential();
+ jwkCredential.setAlgorithm(key.getAlgorithm());
+ jwkCredential.setKid(key.getKeyID());
+ try {
+ final var publicRSAKey =
+ KeySupport.buildJavaRSAPublicKey(CVE_2016_10555_SIGNING_CERT_PUBLIC_KEY);
+ jwkCredential.setPublicKey(publicRSAKey);
+ } catch (final Exception e) {
+ fail();
+ }
+ return jwkCredential;
+ }
+
+ @Override
+ public Iterable<Credential> resolve(final CriteriaSet criteria) throws ResolverException {
+ return List.of(resolveSingle(criteria));
+ }
+ };
+
+ engine = new ExplicitKeySignedJWTTrustEngine(credResolver, joseObjectCredResolver);
+
+ final CriteriaSet criteria = new CriteriaSet();
+ criteria.add(new UsageCriterion(UsageType.SIGNING));
+
+ // This is a crafted JWS (from jwt_tool) using the public RSA key to sign as a MAC
+ // The server *might* if vulnerable valid it it using the RSA public
+ // key it has using the MAC validator.
+ final SignedJWT jws =
+ SignedJWT.parse("eyJraWQiOiJtb2NrLWtleSIsInR5cCI6IkpXVCIsImFsZyI6IkhTMjU2In0."
+ + "eyJpc3MiOiJodHRwczovL29wLmV4YW1wbGUuY29tLyIsIm5hbWUiOiJKIERvZSIsIm"
+ + "F1ZCI6Imh0dHBzOi8vcnAuZXhhbXBsZS5jb20iLCJzdWIiOiJqZG9lIiwicHJlZmVy"
+ + "cmVkX3VzZXJuYW1lIjoiamRvZSJ9._7dbXz7UPiY0Hrfz7SffjjKp849-s696mAvlT-ihWB4");
+
+ final var valid = engine.validate(jws,criteria);
+ assertFalse(valid);
+ }
+
@Test
public void testSuccess_WithInlineJWK() throws JOSEException, SecurityException {
@@ -98,6 +202,43 @@ public class ExplicitKeySignedJWTTrustEngineTest {
assertTrue(valid);
}
+ /**
+ * A shared secret would never come via a JWK, but test in case of forgery.
+ * We should not support shared secrets in JWKs. If we did, it should also
+ * not validate against one that is store locally and trusted (as it is the
+ * attackers keys).
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testSuccess_WithInlineSharedSecretJWK() throws Exception {
+
+ final CriteriaSet criteria = new CriteriaSet();
+ criteria.add(new UsageCriterion(UsageType.SIGNING));
+
+ final var jws = createMACSignedJWTWithInlineJWK(CLIENT_SECRET, key.getKeyID(), JWSAlgorithm.HS256,
+ "https://op.example.com/", "https://rp.example.com");
+
+ final var valid = engine.validate(jws, criteria);
+ assertFalse(valid);
+ }
+
+ @Test
+ public void testInvalid_WithUntrustedInlineJWK_CVE_2018_0114() throws JOSEException, SecurityException {
+
+ final CriteriaSet criteria = new CriteriaSet();
+ criteria.add(new UsageCriterion(UsageType.SIGNING));
+
+ // Create a JWK which is different than that pulled back from the resolver. Is not trusted.
+ final ECKey keyToUse = new ECKeyGenerator(Curve.P_256).keyID("123").generate();
+
+ final var valid = engine.validate(createECSignedJWTWithInlineJWK(keyToUse,
+ keyToUse.getKeyID(), JWSAlgorithm.ES256,
+ "https://op.example.com/", "https://rp.example.com"),
+ criteria);
+ assertFalse(valid);
+ }
+
/* JKU resolution not currently supported by the joseObjectCredResolver.*/
@Test(enabled = false)
public void testSuccess_WithInlineJKU() throws JOSEException, SecurityException, URISyntaxException {
@@ -170,7 +311,78 @@ public class ExplicitKeySignedJWTTrustEngineTest {
/**
- * Create a JWS without a JKU or inline JWK.
+ * Create an RSA signed JWS without a JKU or inline JWK.
+ *
+ * @param key the key to sign the JWT.
+ * @param keyId the keyId to describe the key to use in the header.
+ * @param algo the key algorithm.
+ * @param issuer the issuer.
+ * @param audience the audience.
+ * @return the signed JWT
+ * @throws JOSEException on error.
+ */
+ private SignedJWT createRSASignedJWT(final RSAKey key, final String keyId,
+ final JWSAlgorithm algo, final String issuer,
+ final String audience) throws JOSEException {
+
+ final var header = new JWSHeader.Builder(algo)
+ .type(JOSEObjectType.JWT)
+ .keyID(keyId)
+ .build();
+ final var payload = new JWTClaimsSet.Builder()
+ .issuer(issuer)
+ .audience(audience)
+ .subject("jdoe")
+ .claim("preferred_username", "jdoe")
+ .claim("name", "J Doe")
+ .build();
+
+ final var signedJWT = new SignedJWT(header, payload);
+ signedJWT.sign(new RSASSASigner(key.toPrivateKey()));
+ return signedJWT;
+ }
+
+ /**
+ * Create an MAC signed JWS with inline JWK. This should never be a thing
+ * in reality, you should not put a shared secret into a JWK.
+ *
+ * @param secret the secret to use to create the MAC.
+ * @param keyId the keyId to describe the key to use in the header.
+ * @param algo the key algorithm.
+ * @param issuer the issuer.
+ * @param audience the audience.
+ * @return the signed JWT
+ * @throws JOSEException on error.
+ */
+ private SignedJWT createMACSignedJWTWithInlineJWK(final String secret, final String keyId,
+ final JWSAlgorithm algo, final String issuer,
+ final String audience) throws JOSEException {
+
+ final OctetSequenceKey key = new OctetSequenceKey
+ .Builder(secret.getBytes(StandardCharset.UTF_8))
+ .algorithm(JWSAlgorithm.HS256)
+ .keyID(keyId)
+ .build();
+ final var header = new JWSHeader.Builder(algo)
+ .type(JOSEObjectType.JWT)
+ .keyID(keyId)
+ .jwk(key)
+ .build();
+ final var payload = new JWTClaimsSet.Builder()
+ .issuer(issuer)
+ .audience(audience)
+ .subject("jdoe")
+ .claim("preferred_username", "jdoe")
+ .claim("name", "J Doe")
+ .build();
+
+ final var signedJWT = new SignedJWT(header, payload);
+ signedJWT.sign(new MACSigner(secret.getBytes(StandardCharset.UTF_8)));
+ return signedJWT;
+ }
+
+ /**
+ * Create a EC signed JWS without a JKU or inline JWK.
*
* @param key the key to sign the JWT.
* @param keyId the keyId to describe the key to use in the header.
@@ -201,9 +413,41 @@ public class ExplicitKeySignedJWTTrustEngineTest {
return signedJWT;
}
-
+
+ /**
+ * Create a JWS using a MAC without a JKU or inline JWK.
+ *
+ * @param key the shared key to sign the JWT.
+ * @param keyId the keyId to describe the key to use in the header.
+ * @param algo the key algorithm.
+ * @param issuer the issuer.
+ * @param audience the audience.
+ * @return the signed JWT
+ * @throws JOSEException on error.
+ */
+ private SignedJWT createMACSignedJWT(final String key, final String keyId,
+ final JWSAlgorithm algo, final String issuer,
+ final String audience) throws JOSEException {
+
+ final var header = new JWSHeader.Builder(algo)
+ .type(JOSEObjectType.JWT)
+ .keyID(keyId)
+ .build();
+ final var payload = new JWTClaimsSet.Builder()
+ .issuer(issuer)
+ .audience(audience)
+ .subject("jdoe")
+ .claim("preferred_username", "jdoe")
+ .claim("name", "J Doe")
+ .build();
+
+ final var signedJWT = new SignedJWT(header, payload);
+ signedJWT.sign(new MACSigner(key));
+ return signedJWT;
+ }
+
/**
- * Create a JWS with a JKU URL.
+ * Create an EC signed JWS with a JKU URL.
*
* @param key the key to sign the JWT.
* @param keyId the keyId to describe the key to use in the header.
@@ -237,7 +481,7 @@ public class ExplicitKeySignedJWTTrustEngineTest {
}
/**
- * Create a JWS with an inline (in JOSE headers) JWK.
+ * Create an EC signed JWS with an inline (in JOSE headers) JWK.
*
* @param key the key to sign the JWT.
* @param keyId the keyId to describe the key to use in the header.
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list