[java-oidc-common] branch main updated: Add MAC key length criterion to Explicit Key Trust engine
Phil Smart
philip.smart at jisc.ac.uk
Mon Feb 20 17:09:46 UTC 2023
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=3e5051d15980a630b1ee116f23f50ecbb8ca6fec
The following commit(s) were added to refs/heads/main by this push:
new 3e5051d Add MAC key length criterion to Explicit Key Trust engine
3e5051d is described below
commit 3e5051d15980a630b1ee116f23f50ecbb8ca6fec
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Mon Feb 20 17:09:44 2023 +0000
Add MAC key length criterion to Explicit Key Trust engine
- Inc. tests
---
.../impl/BasicJOSEObjectCredentialResolver.java | 2 +-
.../EvaluableMACKeyLengthCredentialCriterion.java | 95 ++++++++++++++++++
.../impl/ExplicitKeySignedJWTTrustEngine.java | 13 ++-
.../BasicSignatureSigningParametersResolver.java | 1 +
...aluableMACKeyLengthCredentialCriterionTest.java | 109 +++++++++++++++++++++
.../impl/ExplicitKeySignedJWTTrustEngineTest.java | 86 +++++++++++++++-
.../MockAbstractFunctionalCredentialResolver.java | 26 +++++
...tionSignatureSigningParametersResolverTest.java | 4 +-
8 files changed, 327 insertions(+), 9 deletions(-)
diff --git a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/BasicJOSEObjectCredentialResolver.java b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/BasicJOSEObjectCredentialResolver.java
index 08a4da1..11d8385 100644
--- a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/BasicJOSEObjectCredentialResolver.java
+++ b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/BasicJOSEObjectCredentialResolver.java
@@ -99,7 +99,7 @@ public class BasicJOSEObjectCredentialResolver extends AbstractCriteriaFiltering
// Extension point for subclasses
postProcess(criteriaSet, joseObject, credentials);
- log.debug("A total of {} credentials were resolved", credentials.size());
+ log.debug("A total of {} credentials were resolved from the JOSE headers", credentials.size());
return credentials;
diff --git a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/EvaluableMACKeyLengthCredentialCriterion.java b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/EvaluableMACKeyLengthCredentialCriterion.java
new file mode 100644
index 0000000..801335f
--- /dev/null
+++ b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/EvaluableMACKeyLengthCredentialCriterion.java
@@ -0,0 +1,95 @@
+
+package net.shibboleth.oidc.security.credential.impl;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.opensaml.security.credential.Credential;
+import org.opensaml.security.credential.criteria.impl.EvaluableCredentialCriterion;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.nimbusds.jose.JWSAlgorithm;
+
+import net.shibboleth.oidc.security.credential.JWACredentialSupport;
+import net.shibboleth.utilities.java.support.logic.AbstractTriStatePredicate;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
+
+/**
+ * Instance of evaluable credential criteria for evaluating if the key length of the secret key inside the credential
+ * is compatible with the MAC algorithm given.
+ *
+ * @since 2.2.0
+ */
+public class EvaluableMACKeyLengthCredentialCriterion extends AbstractTriStatePredicate<Credential>
+ implements EvaluableCredentialCriterion {
+
+ /** Logger. */
+ private final Logger log = LoggerFactory.getLogger(EvaluableMACKeyLengthCredentialCriterion.class);
+
+ /** Base criteria. */
+ private final JWSAlgorithm alg;
+
+ /**
+ * Constructor.
+ *
+ * @param criteria the criteria which is the basis for evaluation. MUST be a MAC algorithm.
+ */
+ public EvaluableMACKeyLengthCredentialCriterion(@Nonnull final JWSAlgorithm algorithm) {
+ alg = Constraint.isNotNull(algorithm, "MAC Algorithm can not be null");
+ if (!JWSAlgorithm.Family.HMAC_SHA.contains(algorithm)) {
+ throw new ConstraintViolationException(
+ "EvaluableMACKeyLengthCredentialCriterion only usable for MAC algorithms");
+ }
+ }
+
+ @Override
+ public boolean test(@Nullable final Credential target) {
+ if (target == null) {
+ log.error("Credential target was null");
+ return isNullInputSatisfies();
+ }
+ if (target.getSecretKey() == null) {
+ log.error("Credential does not contain a secret key, can not be evaluated for key length");
+ return isUnevaluableSatisfies();
+ }
+ return JWACredentialSupport.keyLengthSupportsMACAlgorithm(alg, target.getSecretKey());
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public String toString() {
+ final StringBuilder builder = new StringBuilder();
+ builder.append("EvaluableMACKeyLengthCredentialCriterion [alg =");
+ builder.append(alg);
+ builder.append("]");
+ return builder.toString();
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public int hashCode() {
+ return alg.hashCode();
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public boolean equals(final Object obj) {
+ if (this == obj) {
+ return true;
+ }
+
+ if (obj == null) {
+ return false;
+ }
+
+ if (obj instanceof EvaluableMACKeyLengthCredentialCriterion) {
+ return alg.equals(((EvaluableMACKeyLengthCredentialCriterion) obj).alg);
+ }
+
+ return false;
+ }
+
+}
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 ac7595d..2f4f71c 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
@@ -32,10 +32,12 @@ import org.slf4j.LoggerFactory;
import com.google.common.base.Strings;
import com.nimbusds.jose.JOSEObject;
+import com.nimbusds.jose.JWSAlgorithm;
import com.nimbusds.jwt.SignedJWT;
import net.shibboleth.oidc.security.credential.JOSEObjectCredentialResolver;
import net.shibboleth.oidc.security.credential.impl.EvaluableKeyIDCredentialCriterion;
+import net.shibboleth.oidc.security.credential.impl.EvaluableMACKeyLengthCredentialCriterion;
import net.shibboleth.oidc.security.jose.criterion.KeyIdCriterion;
import net.shibboleth.utilities.java.support.annotation.ParameterName;
import net.shibboleth.utilities.java.support.logic.Constraint;
@@ -94,10 +96,10 @@ public class ExplicitKeySignedJWTTrustEngine extends BaseSignedJWTTrustEngine<It
criteriaSet.addAll(trustBasisCriteria);
if (!criteriaSet.contains(UsageCriterion.class)) {
criteriaSet.add(new UsageCriterion(UsageType.SIGNING));
- }
+ }
+ final JWSAlgorithm sigAlg = signedJWT.getHeader().getAlgorithm();
- // TODO Do we need this?
- final String jcaAlgorithm = AlgorithmSupport.getKeyAlgorithm(signedJWT.getHeader().getAlgorithm().getName());
+ final String jcaAlgorithm = AlgorithmSupport.getKeyAlgorithm(sigAlg.getName());
if (!Strings.isNullOrEmpty(jcaAlgorithm)) {
criteriaSet.add(new KeyAlgorithmCriterion(jcaAlgorithm), true);
}
@@ -107,6 +109,11 @@ public class ExplicitKeySignedJWTTrustEngine extends BaseSignedJWTTrustEngine<It
if (!Strings.isNullOrEmpty(kid)) {
criteriaSet.add(new EvaluableKeyIDCredentialCriterion(new KeyIdCriterion(kid)));
}
+
+ if (JWSAlgorithm.Family.HMAC_SHA.contains(sigAlg)) {
+ // If MAC type, minimum key length applies
+ criteriaSet.add(new EvaluableMACKeyLengthCredentialCriterion(sigAlg));
+ }
final Iterable<Credential> trustedCredentials;
try {
diff --git a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/jose/impl/BasicSignatureSigningParametersResolver.java b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/jose/impl/BasicSignatureSigningParametersResolver.java
index 30f2320..ba4f4dc 100644
--- a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/jose/impl/BasicSignatureSigningParametersResolver.java
+++ b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/jose/impl/BasicSignatureSigningParametersResolver.java
@@ -69,6 +69,7 @@ import net.shibboleth.utilities.java.support.resolver.ResolverException;
*
* @since 2.2.0
*/
+// TODO this could use a similar strategy based approach as used by the DefaultEncryptionParametersResolver
public class BasicSignatureSigningParametersResolver
extends AbstractSecurityParametersResolver<SignatureSigningParameters>
implements SignatureSigningParametersResolver {
diff --git a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/credential/impl/EvaluableMACKeyLengthCredentialCriterionTest.java b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/credential/impl/EvaluableMACKeyLengthCredentialCriterionTest.java
new file mode 100644
index 0000000..16b748d
--- /dev/null
+++ b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/credential/impl/EvaluableMACKeyLengthCredentialCriterionTest.java
@@ -0,0 +1,109 @@
+
+package net.shibboleth.oidc.security.credential.impl;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import java.security.PrivateKey;
+import java.security.PublicKey;
+import java.util.Collection;
+
+import javax.crypto.SecretKey;
+import javax.crypto.spec.SecretKeySpec;
+
+import org.opensaml.security.credential.Credential;
+import org.opensaml.security.credential.CredentialContextSet;
+import org.opensaml.security.credential.UsageType;
+import org.testng.annotations.Test;
+
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.util.StandardCharset;
+
+import net.shibboleth.utilities.java.support.logic.ConstraintViolationException;
+
+public class EvaluableMACKeyLengthCredentialCriterionTest {
+
+ /** A 256 bit client_secret.*/
+ private static final String CLIENT_SECRET_256 = "!A%D*G-KaPdSgVkYp3s6v8y/B?E(H+Mb";
+
+
+ @Test
+ public void testSucccess() {
+ final var criterion = new EvaluableMACKeyLengthCredentialCriterion(JWSAlgorithm.HS256);
+ assertTrue(criterion.test(new MockCredential(CLIENT_SECRET_256)));
+ }
+
+ /**
+ * The key is too small for the HS512 alg.
+ */
+ @Test
+ public void testFail_KeyToSmall() {
+ final var criterion = new EvaluableMACKeyLengthCredentialCriterion(JWSAlgorithm.HS512);
+ assertFalse(criterion.test(new MockCredential(CLIENT_SECRET_256)));
+
+ }
+
+ @Test(expectedExceptions = ConstraintViolationException.class)
+ public void testConstraintViolationException_WrongAlgType() {
+ new EvaluableMACKeyLengthCredentialCriterion(JWSAlgorithm.RS256);
+ }
+
+ @Test
+ public void testConstraintViolationException_NullKey() {
+ final var criterion = new EvaluableMACKeyLengthCredentialCriterion(JWSAlgorithm.HS256);
+ criterion.setNullInputSatisfies(false);
+ assertFalse(criterion.test(null));
+ }
+
+ /** A no-op mock credential.*/
+ private static class MockCredential implements Credential {
+
+ private final String secret;
+
+ public MockCredential(final String sec) {
+ secret = sec;
+ }
+
+ @Override
+ public String getEntityId() {
+ return null;
+ }
+
+ @Override
+ public UsageType getUsageType() {
+ return null;
+ }
+
+ @Override
+ public Collection<String> getKeyNames() {
+ return null;
+ }
+
+ @Override
+ public PublicKey getPublicKey() {
+ return null;
+ }
+
+ @Override
+ public PrivateKey getPrivateKey() {
+ return null;
+ }
+
+ @Override
+ public SecretKey getSecretKey() {
+ return new SecretKeySpec(secret.getBytes(StandardCharset.UTF_8), "NONE");
+ }
+
+ @Override
+ public CredentialContextSet getCredentialContextSet() {
+ return null;
+ }
+
+ @Override
+ public Class<? extends Credential> getCredentialType() {
+ return null;
+ }
+
+ }
+
+}
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 3fca776..be656e9 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
@@ -27,7 +27,10 @@ import java.security.KeyException;
import java.text.ParseException;
import java.util.List;
+import javax.annotation.Nullable;
+
import org.opensaml.security.SecurityException;
+import org.opensaml.security.credential.Credential;
import org.opensaml.security.credential.UsageType;
import org.opensaml.security.criteria.UsageCriterion;
import org.opensaml.security.crypto.KeySupport;
@@ -54,6 +57,7 @@ import net.shibboleth.oidc.security.credential.impl.BasicJOSEObjectCredentialRes
import net.shibboleth.oidc.security.jose.SignatureValidationParameters;
import net.shibboleth.oidc.security.jose.criterion.SignatureValidationParametersCriterion;
import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
+import net.shibboleth.utilities.java.support.resolver.ResolverException;
/**
* Tests for the {@link ExplicitKeySignedJWTTrustEngine}.
@@ -96,9 +100,12 @@ public class ExplicitKeySignedJWTTrustEngineTest {
+ "cC5leGFtcGxlLmNvbSIsInN1YiI6Impkb2UiLCJwcmVmZXJyZWRfdXNlcm5hbWUiOiJq"
+ "ZG9lIn0.q0XGQTDjL2RPVY1DUswmBh7Q8D-vkJw0KruoUJbSU9c";
- /** The client_secret.*/
+ /** The client_secret. 256 bit*/
private static final String CLIENT_SECRET = "Xp2s5v8y/B?E(H+MbQeThWmYq3t6w9z$";
+ /** The client_secret. 256 bit*/
+ private static final String CLIENT_SECRET_512 = "$B&E)H at McQfTjWnZr4u7w!z%C*F-JaNdRgUkXp2s5v8y/A?D(G+KbPeShVmYq3t6";
+
/** Signature params.*/
private SignatureValidationParameters params;
@@ -167,6 +174,79 @@ public class ExplicitKeySignedJWTTrustEngineTest {
assertTrue(valid);
}
+ /**
+ * Test symmetric key using a mock filtering resolver.
+ *
+ * @throws JOSEException on error
+ * @throws SecurityException on error
+ */
+ @Test
+ public void testValid_WithSymmetricKeyCredential_Filtered() throws JOSEException, SecurityException {
+
+ credResolver = new MockAbstractFunctionalCredentialResolver() {
+
+ @Override
+ public Credential resolveSingle(final CriteriaSet criteriaSet) throws ResolverException {
+ try {
+ return TestCredentialHelper.createClientSecretCredential(CLIENT_SECRET).toSigningCredential();
+ } catch (final KeyException e) {
+ fail();
+ return null;
+ }
+ }
+
+ @Override
+ protected Iterable<Credential> resolveFromSource(final CriteriaSet criteriaSet) throws ResolverException {
+ return List.of(resolveSingle(criteriaSet));
+ }
+
+ };
+
+ engine = new ExplicitKeySignedJWTTrustEngine(credResolver, joseObjectCredResolver);
+
+ final var valid = engine.validate(createMACSignedJWT(CLIENT_SECRET, null, JWSAlgorithm.HS256,
+ "https://op.example.com/", "https://rp.example.com"),
+ criteria);
+ assertTrue(valid);
+ }
+
+ /**
+ * Test symmetric key using a mock filtering resolver. The key length is too small for the HS512 MAC algorithm used
+ * and so no credentials should be supplied for validation.
+ *
+ * @throws JOSEException on error
+ * @throws SecurityException on error
+ */
+ @Test
+ public void testValid_WithSymmetricKeyCredential_Filtered_WrongKeySize() throws JOSEException, SecurityException {
+
+ credResolver = new MockAbstractFunctionalCredentialResolver() {
+
+ @Override
+ public Credential resolveSingle(final CriteriaSet criteriaSet) throws ResolverException {
+ try {
+ return TestCredentialHelper.createClientSecretCredential(CLIENT_SECRET).toSigningCredential();
+ } catch (final KeyException e) {
+ fail();
+ return null;
+ }
+ }
+
+ @Override
+ protected Iterable<Credential> resolveFromSource(final CriteriaSet criteriaSet) throws ResolverException {
+ return List.of(resolveSingle(criteriaSet));
+ }
+
+ };
+
+ engine = new ExplicitKeySignedJWTTrustEngine(credResolver, joseObjectCredResolver);
+
+ final var valid = engine.validate(createMACSignedJWT(CLIENT_SECRET_512, null, JWSAlgorithm.HS512,
+ "https://op.example.com/", "https://rp.example.com"),
+ criteria);
+ assertFalse(valid);
+ }
+
@Test
public void testValid_WithSymmetricKeyCredential_JWSAlgorithm_Excluded() throws JOSEException, SecurityException {
@@ -382,14 +462,14 @@ public class ExplicitKeySignedJWTTrustEngineTest {
* 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 keyId the keyId to describe the key to use in the header. If null, will not be added
* @param algo the key algorithm.
* @param issuer the issuer.
* @param audience the audience.
* @return the signed JWT
* @throws JOSEException on error.
*/
- protected static SignedJWT createMACSignedJWT(final String key, final String keyId,
+ protected static SignedJWT createMACSignedJWT(final String key, @Nullable final String keyId,
final JWSAlgorithm algo, final String issuer,
final String audience) throws JOSEException {
diff --git a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/impl/MockAbstractFunctionalCredentialResolver.java b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/impl/MockAbstractFunctionalCredentialResolver.java
new file mode 100644
index 0000000..24bab86
--- /dev/null
+++ b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/impl/MockAbstractFunctionalCredentialResolver.java
@@ -0,0 +1,26 @@
+/*
+ * 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;
+
+import org.opensaml.security.credential.impl.AbstractCriteriaFilteringCredentialResolver;
+
+/** Abstract base class for test filterable credential resolvers.*/
+public abstract class MockAbstractFunctionalCredentialResolver extends AbstractCriteriaFilteringCredentialResolver
+ implements MockFunctionalCredentialResolver{
+
+}
diff --git a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jose/impl/ClientInformationSignatureSigningParametersResolverTest.java b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jose/impl/ClientInformationSignatureSigningParametersResolverTest.java
index c5f486c..f185b4a 100644
--- a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jose/impl/ClientInformationSignatureSigningParametersResolverTest.java
+++ b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jose/impl/ClientInformationSignatureSigningParametersResolverTest.java
@@ -35,7 +35,7 @@ import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
public class ClientInformationSignatureSigningParametersResolverTest {
/** A 256 bit client_secret.*/
- private static final String TWO_FIVE_SIX_BIT_CLIENT_SECRET = "!A%D*G-KaPdSgVkYp3s6v8y/B?E(H+Mb";
+ private static final String CLIENT_SECRET_256 = "!A%D*G-KaPdSgVkYp3s6v8y/B?E(H+Mb";
private static final ClassPathResource CLIENT_INFORMATION_SECRET =
new ClassPathResource("/metadata/test-resolver-client-information-secret.json");
@@ -91,7 +91,7 @@ public class ClientInformationSignatureSigningParametersResolverTest {
SignatureConstants.ALGO_ID_SIGNATURE_HS_256.toString());
final CriteriaSet criteria = buildCriteria(List.of(SignatureConstants.ALGO_ID_SIGNATURE_HS_256),
- List.of(new DefaultClientSecretCredential(TWO_FIVE_SIX_BIT_CLIENT_SECRET).toSigningCredential()));
+ List.of(new DefaultClientSecretCredential(CLIENT_SECRET_256).toSigningCredential()));
final Iterable<SignatureSigningParameters> params = resolver.resolve(criteria);
assertNotNull(params);
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list