[java-oidc-common] branch dev/JCOMOIDC-62 updated: Fix client information resolver to use new client_secret classes

Phil Smart philip.smart at jisc.ac.uk
Fri Jan 27 11:13:43 UTC 2023


This is an automated email from the git hooks/post-receive script.

philsmart pushed a commit to branch dev/JCOMOIDC-62
in repository java-oidc-common.

View the commit online:
http://git.shibboleth.net/view/?p=java-oidc-common.git;a=commit;h=3387a2dce506e54b92cdcef65f1b66c024064772

The following commit(s) were added to refs/heads/dev/JCOMOIDC-62 by this push:
     new 3387a2d  Fix client information resolver to use new client_secret classes
3387a2d is described below

commit 3387a2dce506e54b92cdcef65f1b66c024064772
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Fri Jan 27 11:13:40 2023 +0000

    Fix client information resolver to use new client_secret classes
    
    And change the way the credential is actually built.
---
 .../credential/ClientSecretCredential.java         |  4 +-
 .../credential/DefaultClientSecretCredential.java  |  1 +
 .../credential/NimbusSecretCredential.java         |  2 +-
 .../impl/BasicJOSEObjectCredentialResolver.java    | 72 ++++++++++++++++++++++
 .../impl/ClientInformationCredentialResolver.java  | 17 +++--
 .../ClientSecretCriterionCredentialResolver.java   | 70 +--------------------
 .../ClientInformationCredentialResolverTest.java   |  3 +
 7 files changed, 95 insertions(+), 74 deletions(-)

diff --git a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/ClientSecretCredential.java b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/ClientSecretCredential.java
index b7f1c0f..15d8b2a 100644
--- a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/ClientSecretCredential.java
+++ b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/ClientSecretCredential.java
@@ -25,6 +25,8 @@ import com.nimbusds.jose.EncryptionMethod;
 import com.nimbusds.jose.JOSEException;
 import com.nimbusds.jose.JWEAlgorithm;
 
+import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
+
 /**
  * Credential wrapping a client_secret. Contains methods to convert the client_secret into suitable keys used for
  * signing and encryption.
@@ -38,7 +40,7 @@ public interface ClientSecretCredential {
      * 
      * @return The client_secret.
      */
-    @Nonnull String getSecret();
+    @Nonnull @NotEmpty String getSecret();
     
     /**
      * Get the client_secret as UTF-8 bytes.
diff --git a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/DefaultClientSecretCredential.java b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/DefaultClientSecretCredential.java
index 5d3a560..1af0841 100644
--- a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/DefaultClientSecretCredential.java
+++ b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/DefaultClientSecretCredential.java
@@ -103,6 +103,7 @@ public class DefaultClientSecretCredential implements ClientSecretCredential {
         final BasicExpiringJWKCredential jwkCredential = new BasicExpiringJWKCredential(); 
         jwkCredential.getKeyNames().add(secretKeyName); 
         jwkCredential.setKid(secretKeyName);
+        jwkCredential.setAlgorithm(alg);
         jwkCredential.setUsageType(UsageType.ENCRYPTION); 
         jwkCredential.setSecretKey(key);
         return  jwkCredential;
diff --git a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/NimbusSecretCredential.java b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/NimbusSecretCredential.java
index 24a72c3..3e92b97 100644
--- a/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/NimbusSecretCredential.java
+++ b/oidc-common-crypto-api/src/main/java/net/shibboleth/oidc/security/credential/NimbusSecretCredential.java
@@ -28,7 +28,7 @@ import com.nimbusds.oauth2.sdk.auth.Secret;
  * 
  * @deprecated use {@link ClientSecretCredential}
  */
- at Deprecated
+ at Deprecated(since = "2.2.0", forRemoval=true)
 public interface NimbusSecretCredential extends Credential {
     
     /**
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 f409a9c..bf90f17 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
@@ -25,13 +25,17 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.opensaml.security.credential.Credential;
+import org.opensaml.security.credential.UsageType;
 import org.opensaml.security.credential.impl.AbstractCriteriaFilteringCredentialResolver;
+import org.opensaml.security.criteria.UsageCriterion;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.nimbusds.jose.EncryptionMethod;
 import com.nimbusds.jose.Header;
 import com.nimbusds.jose.JOSEException;
 import com.nimbusds.jose.JOSEObject;
+import com.nimbusds.jose.JWEAlgorithm;
 import com.nimbusds.jose.JWEHeader;
 import com.nimbusds.jose.JWSHeader;
 import com.nimbusds.jose.jwk.AsymmetricJWK;
@@ -43,6 +47,7 @@ import com.nimbusds.jose.jwk.RSAKey;
 
 import net.shibboleth.oidc.security.CredentialConversionUtil;
 import net.shibboleth.oidc.security.credential.BasicJWKCredential;
+import net.shibboleth.oidc.security.credential.ClientSecretCredential;
 import net.shibboleth.oidc.security.credential.JOSEObjectCredentialResolver;
 import net.shibboleth.oidc.security.jose.criterion.JOSEObjectCriterion;
 import net.shibboleth.oidc.security.jose.criterion.KeyIdCriterion;
@@ -236,4 +241,71 @@ public class BasicJOSEObjectCredentialResolver extends AbstractCriteriaFiltering
             }
         }
     }
+    
+    /**
+     * Use the usage type and information in the criteria to build a suitable signing or encryption credential.
+     * 
+     * <p>Only supports symmetric key encryption algorithms. Request for asymmetric key encryptiopn algorithms are 
+     * ignored.</p>
+     * 
+     * @param usageType are we creating a key suitable for MAC signing or encryption
+     * @param secretCred the raw client_secret credential
+     * @param criteriaSet the criteria set used to find algorithm details for encryption keys
+     * 
+     * @return a suitable credential, or {@code null}. 
+     * 
+     * @throws ResolverException if there is an error deriving the key
+     */
+    @Nullable protected Credential deriveClientSecretCredential(@Nonnull final ClientSecretCredential secretCred, 
+            @Nonnull final CriteriaSet criteriaSet) throws ResolverException {
+        
+        final UsageCriterion usageTypeCriterion = criteriaSet.get(UsageCriterion.class);
+        if (usageTypeCriterion == null) {
+            log.trace("No usage type criterion supplied, unable to derive client_secret credential");
+        }
+        final UsageType usageType = usageTypeCriterion.getUsage();
+        
+        if (usageType == UsageType.SIGNING) {
+            // Create a signing credential
+            final Credential signingCred = secretCred.toSigningCredential();
+            log.debug("Derived signing credential '{}'", signingCred.getKeyNames());            
+            return signingCred;
+            
+        } else if (usageType == UsageType.ENCRYPTION) {
+            // Create an encryption credential suitable for the algorithms specified
+            final KeyManagmentAlgorithmCriterion alg = criteriaSet.get(KeyManagmentAlgorithmCriterion.class);
+                if (alg == null) {
+                throw new ResolverException(
+                        "Credential criteria set did not contain an instance of KeyManagmentAlgorithmCriterion");
+            }
+            // Technically the encryption method is only relevant to key derivation for the Direct Encryption mode
+            final DataEncryptionAlgorithmCriterion enc = criteriaSet.get(DataEncryptionAlgorithmCriterion.class);
+            if (enc == null) {
+                throw new ResolverException(
+                        "Credential criteria set did not contain an instance of DataEncryptionAlgorithmCriterion");
+            }
+            
+            // Can only derive symmetric key credentials, ignore if not
+            if (JWEAlgorithm.Family.SYMMETRIC.contains(JWEAlgorithm.parse(alg.getAlgorithm()))) {                     
+                try {
+                    final Credential derivedCred = secretCred.toEncryptionCredential(JWEAlgorithm.parse(alg.getAlgorithm()), 
+                            EncryptionMethod.parse(enc.getEncAlgorithm()));
+                    
+                    log.debug("Derived encryption credential '{}' from 'alg={}' and 'enc={}'", derivedCred.getKeyNames()
+                            ,alg.getAlgorithm(), enc.getEncAlgorithm());
+                    return derivedCred;
+                    
+                } catch (final JOSEException e) {
+                    log.warn("Unable to derive symmetric encryption key from client_secret using 'alg={}' and 'enc={}'", 
+                            alg.getAlgorithm(), enc.getEncAlgorithm());                    
+                }
+                throw new ResolverException("Unable to create encryption key from client_secret");
+            } else {
+                log.trace("Asymmetric key requested, client_secret not appropriate");                
+            }
+        } else {
+            log.trace("Client secret could not be derived, unknown usage type '{}'", usageType);            
+        }
+        return null;
+    }
 }
diff --git a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/ClientInformationCredentialResolver.java b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/ClientInformationCredentialResolver.java
index cb1d8b8..057bf12 100644
--- a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/ClientInformationCredentialResolver.java
+++ b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/ClientInformationCredentialResolver.java
@@ -24,7 +24,6 @@ import java.util.Collections;
 import java.util.LinkedHashSet;
 
 import javax.annotation.Nonnull;
-import javax.crypto.spec.SecretKeySpec;
 
 import org.opensaml.security.credential.Credential;
 import org.opensaml.security.credential.impl.AbstractCriteriaFilteringCredentialResolver;
@@ -37,8 +36,9 @@ import com.nimbusds.openid.connect.sdk.rp.OIDCClientMetadata;
 
 import net.shibboleth.oidc.jwk.RemoteJwkSetCache;
 import net.shibboleth.oidc.security.credential.BasicJWKCredential;
+import net.shibboleth.oidc.security.credential.ClientSecretCredential;
+import net.shibboleth.oidc.security.credential.DefaultClientSecretCredential;
 import net.shibboleth.oidc.security.credential.JOSEObjectCredentialResolver;
-import net.shibboleth.oidc.security.impl.JWSAssemblyUtils;
 import net.shibboleth.oidc.security.jose.criterion.ClientInformationCriterion;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.Positive;
@@ -96,6 +96,8 @@ public class ClientInformationCredentialResolver extends BasicJOSEObjectCredenti
      * @param interval What to set.
      */
     public void setKeyFetchInterval(@Positive final Duration interval) {
+        ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
+        
         Constraint.isFalse(interval == null || interval.isNegative(), "Remote key refresh must be greater than 0");
         keyFetchInterval = interval;
     }
@@ -143,9 +145,14 @@ public class ClientInformationCredentialResolver extends BasicJOSEObjectCredenti
         final OIDCClientMetadata metadata = information.getOIDCMetadata();
 
         if (information.getSecret() != null) {
-            final BasicJWKCredential jwkCredential = new BasicJWKCredential();
-            jwkCredential.setSecretKey(new SecretKeySpec(JWSAssemblyUtils.getSecretBytes(information.getSecret().getValue()), "AES"));
-            credentials.add(jwkCredential);
+            try {
+                final ClientSecretCredential secretCred = 
+                        new DefaultClientSecretCredential(information.getSecret().getValue());
+                final Credential derivedCredential = deriveClientSecretCredential( secretCred, criteriaSet);
+                credentials.add(derivedCredential);
+            } catch (final ResolverException e) {
+                log.trace("Unable to derive a client_secret based credential", e);
+            }            
         }            
 
         final JWKSet keySet;
diff --git a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/ClientSecretCriterionCredentialResolver.java b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/ClientSecretCriterionCredentialResolver.java
index 9d5dd84..d4a5b7a 100644
--- a/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/ClientSecretCriterionCredentialResolver.java
+++ b/oidc-common-crypto-impl/src/main/java/net/shibboleth/oidc/security/credential/impl/ClientSecretCriterionCredentialResolver.java
@@ -23,17 +23,11 @@ import java.util.List;
 import javax.annotation.Nonnull;
 
 import org.opensaml.security.credential.Credential;
-import org.opensaml.security.credential.UsageType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.nimbusds.jose.EncryptionMethod;
-import com.nimbusds.jose.JOSEException;
-import com.nimbusds.jose.JWEAlgorithm;
-
 import net.shibboleth.oidc.security.credential.ClientSecretCredential;
 import net.shibboleth.oidc.security.jose.criterion.ClientSecretCredentialCriterion;
-import net.shibboleth.utilities.java.support.annotation.ParameterName;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 import net.shibboleth.utilities.java.support.resolver.ResolverException;
@@ -42,8 +36,6 @@ import net.shibboleth.utilities.java.support.resolver.ResolverException;
  * Extracts a credential held inside the {@link ClientSecretCredentialCriterion} from the given criteria set.
  * Supports credential filter via evaluable criterion.
  * 
- * <p>Only supports symmetric key algorithms. Request for assymetric key algorithms are ignored.</p>
- * 
  * <p>A different key credential is derived for different usage types. MAC ('signing') keys are generated directly off 
  * the UTF-8 octets of the client_secret. Encryption/Decryption keys are generated specifically for the key management 
  * mode and encryption algorithm pulled out of the criteria set —and as such, if these criteria do not exist, resolution 
@@ -54,19 +46,6 @@ public class ClientSecretCriterionCredentialResolver extends BasicJOSEObjectCred
     /** Class logger. */
     @Nonnull private final Logger log = LoggerFactory.getLogger(ClientSecretCriterionCredentialResolver.class);
     
-    /** The pre-determined usage type of the credential that is returned from the client_secret.*/
-    @Nonnull private final UsageType forUsageType;
-    
-    /**
-     * 
-     * Constructor.
-     *
-     * @param usage the usage type of the returned credential
-     */
-    public ClientSecretCriterionCredentialResolver(@Nonnull @ParameterName(name="usage") final UsageType usage) {
-        forUsageType = Constraint.isNotNull(usage, "Usage can not be null");
-    }
-    
     @Override
     @Nonnull protected Iterable<Credential> resolveFromSource(@Nonnull final CriteriaSet criteriaSet) 
             throws ResolverException {        
@@ -76,54 +55,11 @@ public class ClientSecretCriterionCredentialResolver extends BasicJOSEObjectCred
             final ClientSecretCredentialCriterion credentialCriterion = criteriaSet.get(ClientSecretCredentialCriterion.class);
             final ClientSecretCredential secretCred = credentialCriterion.getCredential();   
             log.debug("Found client secret credential");           
-            if (forUsageType == UsageType.SIGNING) {
-                // Create a signing credential
-                final Credential signingCred = secretCred.toSigningCredential();
-                log.debug("Derived signing credential '{}'", signingCred.getKeyNames());
-                
-                return List.of(signingCred);
-                
-            } else if (forUsageType == UsageType.ENCRYPTION) {
-                // Create an encryption credential suitable for the algorithms specified
-                final KeyManagmentAlgorithmCriterion alg = criteriaSet.get(KeyManagmentAlgorithmCriterion.class);
-                if (alg == null) {
-                    throw new ResolverException(
-                            "Credential criteria set did not contain an instance of KeyManagmentAlgorithmCriterion");
-                }
-                // Technically the encryption method is only relevant to key derivation for the Direct Encryption mode
-                final DataEncryptionAlgorithmCriterion enc = criteriaSet.get(DataEncryptionAlgorithmCriterion.class);
-                if (enc == null) {
-                    throw new ResolverException(
-                            "Credential criteria set did not contain an instance of DataEncryptionAlgorithmCriterion");
-                }
-                
-                // Can only derive symmetric key credentials, ignore if not
-                if (JWEAlgorithm.Family.SYMMETRIC.contains(JWEAlgorithm.parse(alg.getAlgorithm()))) {                     
-                    try {
-                        final Credential derivedCred = secretCred.toEncryptionCredential(JWEAlgorithm.parse(alg.getAlgorithm()), 
-                                EncryptionMethod.parse(enc.getEncAlgorithm()));
-                        
-                        log.debug("Derived encryption credential '{}' from 'alg={}' and 'enc={}'", derivedCred.getKeyNames()
-                                ,alg.getAlgorithm(), enc.getEncAlgorithm());
-                        return List.of(derivedCred);
-                        
-                    } catch (final JOSEException e) {
-                        log.warn("Unable to derive symmetric encryption key from client_secret using 'alg={}' and 'enc={}'", 
-                                alg.getAlgorithm(), enc.getEncAlgorithm());                    
-                    }
-                    throw new ResolverException("Unable to create encryption key from client_secret");
-                } else {
-                    log.trace("Asymmetric key requested, client_secret not appropriate");
-                    return Collections.emptyList();
-                }
-            } else {
-                throw new ResolverException("Unable to create key from client_secret, incompatible usage type");
-            }
+            final Credential derivedCredential = deriveClientSecretCredential(secretCred, criteriaSet);
+            return derivedCredential != null ? List.of(derivedCredential) : Collections.emptyList();
         } else {
             log.debug("Criteria did not contain a StaticClientSecretCredentialCriterion");
             return Collections.emptyList();
         }
-    }
-    
-    
+    }  
 }
diff --git a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/credential/impl/ClientInformationCredentialResolverTest.java b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/credential/impl/ClientInformationCredentialResolverTest.java
index 79d94ad..a2509de 100644
--- a/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/credential/impl/ClientInformationCredentialResolverTest.java
+++ b/oidc-common-crypto-impl/src/test/java/net/shibboleth/oidc/security/credential/impl/ClientInformationCredentialResolverTest.java
@@ -86,6 +86,7 @@ public class ClientInformationCredentialResolverTest extends BaseMetadataCredent
         assertEquals(credsList.size(), 0);
     }
 
+    @Override
     @Test
     public void testFail_EmptyCriteria() throws Exception {
         ((InitializableComponent) resolver).initialize();
@@ -106,6 +107,8 @@ public class ClientInformationCredentialResolverTest extends BaseMetadataCredent
         
         criteria.add(new ClientInformationCriterion(
                 OIDCClientInformation.parse(JSONObjectUtils.parse(readJsonFromFile(CLIENT_INFORMATION_SECRET)))));
+        
+        criteria.add(new UsageCriterion(UsageType.SIGNING));
 
         final Iterable<Credential> creds = resolver.resolve(criteria);
         

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


More information about the commits mailing list