[java-oidc-common] branch main updated: Add EC curve check when finding signing credentials
Phil Smart
philip.smart at jisc.ac.uk
Mon Feb 20 11:31:05 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=a45bfe35f628fc4854c75142f01da3d1477ccc5a
The following commit(s) were added to refs/heads/main by this push:
new a45bfe3 Add EC curve check when finding signing credentials
a45bfe3 is described below
commit a45bfe35f628fc4854c75142f01da3d1477ccc5a
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Mon Feb 20 11:30:59 2023 +0000
Add EC curve check when finding signing credentials
- If the credential is an EC type, check the curve is compatible with
the ES* signature signing algorithm used.
---
.../security/credential/JWACredentialSupport.java | 74 ++++++++++++++++++++++
.../credential/JWACredentialSupportTest.java | 73 +++++++++++++++++++++
.../BasicSignatureSigningParametersResolver.java | 43 ++++++++++---
...asicSignatureSigningParametersResolverTest.java | 48 ++++++++++++++
4 files changed, 230 insertions(+), 8 deletions(-)
diff --git a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/JWACredentialSupport.java b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/JWACredentialSupport.java
new file mode 100644
index 0000000..afc2d44
--- /dev/null
+++ b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/JWACredentialSupport.java
@@ -0,0 +1,74 @@
+/*
+ * 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.credential;
+
+import java.security.Key;
+import java.security.interfaces.ECKey;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.crypto.impl.ECDSA;
+import com.nimbusds.jose.jwk.Curve;
+
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+
+/** Support class for JSON Web Algorithm credentials.*/
+public final class JWACredentialSupport {
+
+ /** Constructor. */
+ private JWACredentialSupport() { }
+
+ /**
+ * Returns true of the key is not null and is an EC key.
+ *
+ * @param cred the credential to test
+ *
+ * @return true if either a private or public EC key exists
+ */
+ public static boolean isECKeyType(@Nullable final Key key) {
+ if (key == null) {
+ return false;
+ }
+ return key instanceof ECKey;
+ }
+
+ /**
+ * Is the EC key supplied compatible with the EC Curve required by the algorithm given.
+ *
+ * @param key the key to check compatibility for
+ * @param algorithm the algorithm to match compatibility against
+ *
+ * @return true if compatible, false otherwise.
+ *
+ * @throws JOSEException if there is an issue deriving algorithms
+ */
+ public static boolean keySupportsCurve(@Nonnull final ECKey key,
+ @Nonnull @NotEmpty final String algorithm) throws JOSEException {
+
+ final JWSAlgorithm algCompatibleWithCredentialCurve =
+ ECDSA.resolveAlgorithm(Curve.forECParameterSpec(key.getParams()));
+
+ final JWSAlgorithm alg = JWSAlgorithm.parse(algorithm);
+
+ return algCompatibleWithCredentialCurve == alg;
+ }
+
+}
diff --git a/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/security/credential/JWACredentialSupportTest.java b/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/security/credential/JWACredentialSupportTest.java
new file mode 100644
index 0000000..8554846
--- /dev/null
+++ b/oidc-common-crypto-api/src/test/java/net/shibboleth/oidc/security/credential/JWACredentialSupportTest.java
@@ -0,0 +1,73 @@
+
+package net.shibboleth.oidc.security.credential;
+
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.jwk.Curve;
+import com.nimbusds.jose.jwk.ECKey;
+import com.nimbusds.jose.jwk.KeyUse;
+import com.nimbusds.jose.jwk.RSAKey;
+import com.nimbusds.jose.jwk.gen.ECKeyGenerator;
+import com.nimbusds.jose.jwk.gen.RSAKeyGenerator;
+
+/** Tests for {@link JWACredentialSupport}.*/
+public class JWACredentialSupportTest {
+
+ private ECKey key;
+
+ private RSAKey rsaKey;
+
+ @BeforeMethod
+ public void setup() throws JOSEException {
+ key = new ECKeyGenerator(Curve.P_256)
+ .keyID("1")
+ .keyUse(KeyUse.SIGNATURE)
+ .generate();
+
+ rsaKey = new RSAKeyGenerator(2048)
+ .keyID("3")
+ .keyUse(KeyUse.SIGNATURE)
+ .generate();
+ }
+
+ @Test
+ public void testKeySupportsCurve_Success() throws JOSEException {
+ assertTrue(JWACredentialSupport.keySupportsCurve(key.toECPrivateKey(), "ES256"));
+ }
+
+ @Test
+ public void testKeySupportsCurve_Fail() throws JOSEException {
+ assertFalse(JWACredentialSupport.keySupportsCurve(key.toECPrivateKey(), "ES512"));
+ }
+
+ @Test
+ public void testKeySupportsCurve_Fail_BadAlgorithm() throws JOSEException {
+ assertFalse(JWACredentialSupport.keySupportsCurve(key.toECPrivateKey(), "WRONG"));
+ }
+
+ @Test
+ public void testKeySupportsCurve_Fail_JWEAlgorithm() throws JOSEException {
+ assertFalse(JWACredentialSupport.keySupportsCurve(key.toECPrivateKey(), "A192GCMKW"));
+ }
+
+ @Test
+ public void testIsECType_Success() throws JOSEException {
+ assertTrue(JWACredentialSupport.isECKeyType(key.toPrivateKey()));
+ }
+
+ @Test
+ public void testIsECType_Fail_IsRSAType() throws JOSEException {
+ assertFalse(JWACredentialSupport.isECKeyType(rsaKey.toPrivateKey()));
+ }
+
+ @Test
+ public void testIsECType_Fail_IsNull() throws JOSEException {
+ assertFalse(JWACredentialSupport.isECKeyType(null));
+ }
+
+}
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 877cd47..a896ce4 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
@@ -18,6 +18,7 @@
package net.shibboleth.oidc.security.jose.impl;
import java.security.Key;
+import java.security.interfaces.ECKey;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -35,6 +36,10 @@ import org.opensaml.xmlsec.impl.AlgorithmRuntimeSupportedPredicate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.nimbusds.jose.JOSEException;
+
+import net.shibboleth.oidc.security.CredentialConversionUtil;
+import net.shibboleth.oidc.security.credential.JWACredentialSupport;
import net.shibboleth.oidc.security.jose.SignatureSigningConfiguration;
import net.shibboleth.oidc.security.jose.SignatureSigningParameters;
import net.shibboleth.oidc.security.jose.SignatureSigningParametersResolver;
@@ -201,19 +206,26 @@ public class BasicSignatureSigningParametersResolver
log.trace("Resolved effective signature algorithms: {}", algorithms);
for (final Credential credential : credentials) {
- if (log.isTraceEnabled()) {
- final Key key = CredentialSupport.extractSigningKey(credential);
- log.trace("Evaluating credential of type: {}", key != null ? key.getAlgorithm() : "n/a");
- }
for (final String algorithm : algorithms) {
- log.trace("Evaluating credential against algorithm: {}", algorithm);
+ if (log.isTraceEnabled()) {
+ final Key key = CredentialSupport.extractSigningKey(credential);
+ log.trace("Evaluating credential '{}' of type '{}' against algorithm: {}",
+ CredentialConversionUtil.resolveKid(credential), key != null ? key.getAlgorithm() : "n/a",
+ algorithm);
+ }
if (credentialSupportsAlgorithm(credential, algorithm)) {
- log.trace("Credential passed eval against algorithm: {}", algorithm);
+ if (log.isTraceEnabled()) {
+ log.trace("Credential '{}' passed eval against algorithm: {}",
+ CredentialConversionUtil.resolveKid(credential), algorithm);
+ }
params.setSigningCredential(credential);
params.setSignatureAlgorithm(algorithm);
return;
}
- log.trace("Credential failed eval against algorithm: {}", algorithm);
+ if (log.isTraceEnabled()) {
+ log.trace("Credential '{}' failed eval against algorithm: {}",
+ CredentialConversionUtil.resolveKid(credential), algorithm);
+ }
}
}
@@ -232,6 +244,8 @@ public class BasicSignatureSigningParametersResolver
/**
* Evaluate whether the specified credential is supported for use with the specified algorithm URI.
*
+ * <p>If EC key type, the curve is also checked against the algorithm for compatibility.</p>
+ *
* @param credential the credential to evaluate
* @param algorithm the algorithm URI to evaluate
* @return true if credential may be used with the supplied algorithm URI, false otherwise
@@ -239,10 +253,23 @@ public class BasicSignatureSigningParametersResolver
protected boolean credentialSupportsAlgorithm(@Nonnull final Credential credential,
@Nonnull @NotEmpty final String algorithm) {
- return AlgorithmSupport.credentialSupportsAlgorithmForSigning(credential,
+ final boolean credentialSupportAlgorithm = AlgorithmSupport.credentialSupportsAlgorithmForSigning(credential,
getAlgorithmRegistry().get(algorithm));
+
+ // Check private key only as for signing operation. Only check if initial compatibility is established
+ if (credentialSupportAlgorithm && JWACredentialSupport.isECKeyType(credential.getPrivateKey())) {
+ try {
+ return JWACredentialSupport.keySupportsCurve((ECKey)credential.getPrivateKey(), algorithm);
+ } catch (final JOSEException e) {
+ log.trace("Algorithm '{}' and EC credential '{}' threw an error while checking for compatibility, "
+ + "credential can not be used", algorithm, CredentialConversionUtil.resolveKid(credential), e);
+ return false;
+ }
+ }
+ return credentialSupportAlgorithm;
}
+
/**
* Get the effective list of signing credentials to consider.
*
diff --git a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jose/impl/BasicSignatureSigningParametersResolverTest.java b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jose/impl/BasicSignatureSigningParametersResolverTest.java
index 9723de0..0484ad7 100644
--- a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jose/impl/BasicSignatureSigningParametersResolverTest.java
+++ b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/jose/impl/BasicSignatureSigningParametersResolverTest.java
@@ -7,6 +7,7 @@ import static org.testng.Assert.assertNotNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
+import java.security.spec.ECParameterSpec;
import java.util.Collections;
import java.util.List;
@@ -16,6 +17,8 @@ import org.opensaml.xmlsec.config.GlobalAlgorithmRegistryInitializer;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.crypto.impl.ECDSA;
import com.nimbusds.jose.jwk.Curve;
import com.nimbusds.jose.jwk.ECKey;
import com.nimbusds.jose.jwk.KeyUse;
@@ -74,6 +77,7 @@ public class BasicSignatureSigningParametersResolverTest {
assertNotNull(params.iterator().next().getSigningCredential());
assertNotNull(params.iterator().next().getSigningCredential().getSecretKey());
}
+
@Test
public void testResolveSuccess_PS256() throws Exception {
@@ -131,6 +135,50 @@ public class BasicSignatureSigningParametersResolverTest {
assertEquals(params.iterator().next().getSigningCredential().getPrivateKey().getAlgorithm(),"EC");
}
+ /**
+ * Choose the correct ES signing credential when there are two to choose from and the first has the wrong curve.
+ *
+ * @throws Exception on error.
+ */
+ @Test
+ public void testResolveSuccess_Two_ES512_ChooseCorrectCurve() throws Exception {
+
+ final ECKey keyP256 = new ECKeyGenerator(Curve.P_256)
+ .keyID("1")
+ .keyUse(KeyUse.SIGNATURE)
+ .generate();
+
+ final ECKey keyP521 = new ECKeyGenerator(Curve.P_521)
+ .keyID("2")
+ .keyUse(KeyUse.SIGNATURE)
+ .generate();
+
+ final CriteriaSet criteria = buildCriteria(List.of(SignatureConstants.ALGO_ID_SIGNATURE_ES_512),
+ List.of(TestCredentialHelper.createAsymmetricSigningCredential(keyP256),
+ TestCredentialHelper.createAsymmetricSigningCredential(keyP521)));
+
+ final Iterable<SignatureSigningParameters> params = resolver.resolve(criteria);
+ assertNotNull(params);
+ assertTrue(params.iterator().hasNext());
+ final SignatureSigningParameters sigParams = params.iterator().next();
+ assertNotNull(sigParams.getSigningCredential());
+ final Credential cred = sigParams.getSigningCredential();
+ assertNotNull(cred.getPrivateKey());
+ assertTrue(cred.getPrivateKey() instanceof java.security.interfaces.ECKey);
+ final java.security.interfaces.ECKey ecPrivateKeyChosen = (java.security.interfaces.ECKey)cred.getPrivateKey();
+ final ECParameterSpec ecParameterSpec = ecPrivateKeyChosen.getParams();
+
+ assertEquals(cred.getPrivateKey().getAlgorithm(),"EC");
+
+ // Check the curve is compatible with the ES512 alg.
+ final JWSAlgorithm algCompatibleWithCredentialCurve =
+ ECDSA.resolveAlgorithm(Curve.forECParameterSpec(ecParameterSpec));
+
+ assertEquals(algCompatibleWithCredentialCurve, JWSAlgorithm.parse(sigParams.getSignatureAlgorithm()),
+ "Chosen EC key is not compatible with chose signature algorithm");
+
+ }
+
@Test
public void testResolveFail_NoSupportedAlgs() throws Exception {
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list