[java-oidc-common] branch main updated: Add MAC secret key length check to signature signing params resolver

Phil Smart philip.smart at jisc.ac.uk
Mon Feb 20 15:20:45 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=5dc1432d1fe3acee78da5bbbd8e28098f93cd999

The following commit(s) were added to refs/heads/main by this push:
     new 5dc1432  Add MAC secret key length check to signature signing params resolver
5dc1432 is described below

commit 5dc1432d1fe3acee78da5bbbd8e28098f93cd999
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Mon Feb 20 15:20:42 2023 +0000

    Add MAC secret key length check to signature signing params resolver
---
 .../security/credential/JWACredentialSupport.java  | 40 ++++++++++++++++++++++
 .../BasicSignatureSigningParametersResolver.java   |  8 +++--
 ...asicSignatureSigningParametersResolverTest.java | 12 ++++++-
 3 files changed, 56 insertions(+), 4 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
index 56a4d04..39620f3 100644
--- 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
@@ -23,6 +23,9 @@ import java.security.interfaces.ECKey;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.nimbusds.jose.JOSEException;
 import com.nimbusds.jose.JWSAlgorithm;
 import com.nimbusds.jose.crypto.impl.ECDSA;
@@ -33,6 +36,9 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 /** Support class for JSON Web Algorithm credentials.*/
 public final class JWACredentialSupport {
     
+    /** Logger. */
+    private static final Logger log = LoggerFactory.getLogger(JWACredentialSupport.class);
+    
     /** Constructor. */
     private JWACredentialSupport() { }
     
@@ -83,5 +89,39 @@ public final class JWACredentialSupport {
             @Nonnull @NotEmpty final String algorithm) throws JOSEException {
         return keySupportsCurve(key, JWSAlgorithm.parse(algorithm));
     }
+    
+    /**
+     * Check the key length is compatible with the given JWS algorithm.  If not, or the given algorithm is not a MAC
+     * algorithm, return false.
+     * 
+     * @param macAlg the MAC algorithm to check
+     * @param key the key to check the length of
+     * 
+     * @return true if the algorithm is a MAC algorithm and the key length is compatible with the MAC algorithm, 
+     *                  false otherwise.
+     */
+    public static boolean keyLengthSupportsMACAlgorithm(@Nonnull final JWSAlgorithm macAlg, @Nonnull final Key key) {
+        if (!JWSAlgorithm.Family.HMAC_SHA.contains(macAlg)) {
+            return false;
+        }
+        if (key.getEncoded() == null){
+            return false;
+        }
+        final int keyLength = key.getEncoded().length * 8;
+        boolean keyLengthMatch = false;
+        if (JWSAlgorithm.HS256.equals(macAlg)) {
+            keyLengthMatch = keyLength >= 256;
+        } else if (JWSAlgorithm.HS384.equals(macAlg)) {
+            keyLengthMatch = keyLength >= 384;
+        } else if (JWSAlgorithm.HS512.equals(macAlg)) {
+            keyLengthMatch = keyLength >= 512;
+        } 
+        if (!keyLengthMatch) {
+            log.trace("Key length of {} bits does not match minimum required for algorithm '{}'", keyLength, macAlg);
+            return false;
+        } else {
+            return true;
+        }
+    }
 
 }
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 f9ebd62..30f2320 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
@@ -264,7 +264,8 @@ public class BasicSignatureSigningParametersResolver
      * Evaluate whether the specified credential is supported for use with the specified signing algorithm.
      * 
      * <p>First, the key type is checked against the algorithm family, then the algorithm and key length are checked. 
-     * If the key is an EC type, the curve is also checked against the algorithm.</p>
+     * If the key is an EC type, the curve is also checked against the algorithm. If the key is a MAC type, check
+     * the key length matches the MAC signing algorithm used.</p>
      * 
      * @param credential the credential to evaluate
      * @param algorithm the algorithm URI to evaluate
@@ -278,7 +279,8 @@ public class BasicSignatureSigningParametersResolver
             final Key key = CredentialSupport.extractSigningKey(credential);
             
             boolean credSupportsAlgorithm = false;
-            if (JWSAlgorithm.Family.HMAC_SHA.contains(supportedAlgorithm) && credential.getSecretKey() != null) {
+            if (JWSAlgorithm.Family.HMAC_SHA.contains(supportedAlgorithm) && credential.getSecretKey() != null &&
+                    JWACredentialSupport.keyLengthSupportsMACAlgorithm(supportedAlgorithm, credential.getSecretKey())) {
                 credSupportsAlgorithm = true;
             } else if (JWSAlgorithm.Family.RSA.contains(supportedAlgorithm) 
                     && credential.getPrivateKey() instanceof RSAPrivateKey) {
@@ -288,7 +290,7 @@ public class BasicSignatureSigningParametersResolver
                     && JWACredentialSupport.keySupportsCurve((ECKey)credential.getPrivateKey(), supportedAlgorithm)) {
                 credSupportsAlgorithm = true;
             }
-
+            // TODO the final opensaml key check is likely covered for the JWA variant in the tests above
             return credSupportsAlgorithm && AlgorithmSupport.checkKeyAlgorithmAndLength(key, 
                     getAlgorithmRegistry().get(algorithm));
             
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 0484ad7..2ba6c08 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
@@ -69,7 +69,7 @@ public class BasicSignatureSigningParametersResolverTest {
     public void testResolveSuccess_HS256() throws Exception {
 
         final CriteriaSet criteria = buildCriteria(List.of(SignatureConstants.ALGO_ID_SIGNATURE_HS_256),
-                List.of(new DefaultClientSecretCredential("atestsecret").toSigningCredential()));
+                List.of(new DefaultClientSecretCredential("fUjXn2r5u8x/A?D(G+KbPeShVkYp3s6v").toSigningCredential()));
 
         final Iterable<SignatureSigningParameters> params = resolver.resolve(criteria);
         assertNotNull(params);
@@ -77,7 +77,17 @@ public class BasicSignatureSigningParametersResolverTest {
         assertNotNull(params.iterator().next().getSigningCredential());
         assertNotNull(params.iterator().next().getSigningCredential().getSecretKey());
     }
+    
+    @Test
+    public void testResolveFail_HS512_KeySizeToSmall() throws Exception {
+
+        final CriteriaSet criteria = buildCriteria(List.of(SignatureConstants.ALGO_ID_SIGNATURE_HS_512),
+                List.of(new DefaultClientSecretCredential("a").toSigningCredential()));
 
+        final Iterable<SignatureSigningParameters> params = resolver.resolve(criteria);
+        assertNotNull(params);
+        assertFalse(params.iterator().hasNext());   
+    }
     
     @Test
     public void testResolveSuccess_PS256() throws Exception {

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list