[java-idp-plugin-oidc-rp] branch main updated: Improve provider metadata encryption parameter resolver

Phil Smart philip.smart at jisc.ac.uk
Tue Aug 2 15:31:43 UTC 2022


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

philsmart pushed a commit to branch main
in repository java-idp-plugin-oidc-rp.

View the commit online:
http://git.shibboleth.net/view/?p=java-idp-plugin-oidc-rp.git;a=commit;h=5b86a6a55c10ba1666f7c875cef81155b3a0fd08

The following commit(s) were added to refs/heads/main by this push:
     new 5b86a6a  Improve provider metadata encryption parameter resolver
5b86a6a is described below

commit 5b86a6a55c10ba1666f7c875cef81155b3a0fd08
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Tue Aug 2 16:31:37 2022 +0100

    Improve provider metadata encryption parameter resolver
    
    Supports key wrapping and key encryption credentials/modes.
    Fix tests
---
 .gitignore                                         |   1 +
 .../logic/RequestObjectRequiredAndSupported.java   |   2 +-
 .../plugin/authn/oidc/rp/support/JWKSupport.java   |  49 +++++
 ...oviderMetadataEncryptionParametersResolver.java | 214 ++++++++++++---------
 ...RelyingPartyProxySigningParametersResolver.java |  22 +--
 .../authn/oidc/rp/messaging/impl/EncryptJWT.java   |   2 +-
 .../oidc-relying-party-authn-beans.xml             |   1 -
 .../oidc-relying-party-authn-flow.xml              |  24 +--
 .../idp/service/relying-party/postconfig.xml       |  11 +-
 .../plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java  |   5 +
 ...erMetadataEncryptionParametersResolverTest.java |  62 +++++-
 .../authn/oidc/rp/impl/TestCredentialHelper.java   |  21 +-
 .../plugin/authn/oidc/rp/impl/TestJsonHelper.java  |   1 -
 .../test/resources/conf/authn/rp-credentials.xml   |   7 +-
 14 files changed, 286 insertions(+), 136 deletions(-)

diff --git a/.gitignore b/.gitignore
index 0ca7416..10de8c9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,3 +8,4 @@
 /.project
 /.settings
 target
+/bin/
diff --git a/idp-oidc-rp-api/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/context/logic/RequestObjectRequiredAndSupported.java b/idp-oidc-rp-api/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/context/logic/RequestObjectRequiredAndSupported.java
index 49e2f59..5b7fc35 100644
--- a/idp-oidc-rp-api/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/context/logic/RequestObjectRequiredAndSupported.java
+++ b/idp-oidc-rp-api/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/context/logic/RequestObjectRequiredAndSupported.java
@@ -88,7 +88,7 @@ public class RequestObjectRequiredAndSupported extends AbstractRelyingPartyPredi
         final boolean requestedAndSupport =
                 requestObjectRequestedFromConfig && isSupportedByOP; 
         
-        log.debug("Authentication RequestObject was requested '{}', is supported by the OP '{}', "
+        log.debug("Authentication RequestObject was enabled '{}', is supported by the OP '{}', "
                 + "will be used '{}'", requestObjectRequestedFromConfig, isSupportedByOP, requestedAndSupport);
        
         return requestedAndSupport;
diff --git a/idp-oidc-rp-api/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/support/JWKSupport.java b/idp-oidc-rp-api/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/support/JWKSupport.java
new file mode 100644
index 0000000..259a464
--- /dev/null
+++ b/idp-oidc-rp-api/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/support/JWKSupport.java
@@ -0,0 +1,49 @@
+/*
+ * 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.idp.plugin.authn.oidc.rp.support;
+
+import java.util.stream.Collectors;
+
+import org.opensaml.security.credential.Credential;
+
+import net.shibboleth.oidc.security.credential.JWKCredential;
+import net.shibboleth.utilities.java.support.primitive.StringSupport;
+
+/** Support class for JSON Web Key credentials.*/
+public final class JWKSupport {
+    
+    /** Constructor. */
+    private JWKSupport() {
+    }
+    
+    /**
+     * Extract the credentials name for display. Favouring the keyId if there is one, over the keyNames.
+     * 
+     * @param credential the credential to extract the name from
+     * 
+     * @return the key name. 
+     */
+    public static String extractKeyName(final Credential credential) {
+        if (credential instanceof JWKCredential && 
+                StringSupport.trimOrNull(((JWKCredential)credential).getKid()) != null) {
+            return ((JWKCredential)credential).getKid();
+        } 
+        return credential.getKeyNames().stream().collect(Collectors.joining(","));     
+    }
+
+}
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProviderMetadataEncryptionParametersResolver.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProviderMetadataEncryptionParametersResolver.java
index 54d52e2..23a5cb5 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProviderMetadataEncryptionParametersResolver.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProviderMetadataEncryptionParametersResolver.java
@@ -17,7 +17,6 @@
 
 package net.shibboleth.idp.plugin.authn.oidc.rp.impl;
 
-import java.security.NoSuchAlgorithmException;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.Collections;
@@ -30,7 +29,9 @@ import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.opensaml.security.credential.Credential;
+import org.opensaml.xmlsec.EncryptionConfiguration;
 import org.opensaml.xmlsec.EncryptionParameters;
+import org.opensaml.xmlsec.criterion.EncryptionConfigurationCriterion;
 import org.opensaml.xmlsec.impl.BasicEncryptionParametersResolver;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -39,7 +40,6 @@ import com.nimbusds.jose.Algorithm;
 import com.nimbusds.jose.EncryptionMethod;
 import com.nimbusds.jose.JOSEException;
 import com.nimbusds.jose.JWEAlgorithm;
-import com.nimbusds.jose.jwk.ECKey;
 import com.nimbusds.jose.jwk.JWK;
 import com.nimbusds.jose.jwk.JWKSet;
 import com.nimbusds.jose.jwk.KeyType;
@@ -47,22 +47,33 @@ import com.nimbusds.jose.jwk.KeyUse;
 import com.nimbusds.jose.jwk.RSAKey;
 import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata;
 
+import net.shibboleth.idp.plugin.authn.oidc.rp.support.JWKSupport;
 import net.shibboleth.oidc.jwk.RemoteJwkSetCache;
 import net.shibboleth.oidc.security.credential.BasicJWKCredential;
+import net.shibboleth.oidc.security.credential.JWKCredential;
 import net.shibboleth.oidc.security.criterion.ProviderMetadataCriterion;
-import net.shibboleth.oidc.security.criterion.StaticCredentialCriterion;
-import net.shibboleth.oidc.security.impl.OIDCDecryptionParameters;
-import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.annotation.constraint.Positive;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.logic.FunctionSupport;
-import net.shibboleth.utilities.java.support.logic.PredicateSupport;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 
 /**
- * 
- * <p>Does not support Direct Encryption.</p>
+ * A specialization of {@link BasicEncryptionParametersResolver} which resolves encryption credentials from 
+ * various sources based on what is supported by both the IdP and the downstream OpenID Provider. 
+ * Broadly this supports:
+ * <ul>
+ * <li>Private KeyWrapping credentials found in the {@link EncryptionConfiguration}.</li>
+ * <li>Public KeyEncryption credentials found in the Providers remote keyset. The Provider's metadata
+ * must be contained inside a ProviderMetadataCriterion.</li>
+ * </ul>
+ * Failure to resolve credentials from the above sources delegates resolution back to the 
+ * {@link BasicEncryptionParametersResolver}. 
+ * It does not support:
+ * <ul>
+ * <li>Direct encryption credentials</li>
+ * </ul>
  */
+//TODO check key length for algorithm e.g AES key wrap credential size?
 public class ProviderMetadataEncryptionParametersResolver extends BasicEncryptionParametersResolver {
     
     /** Logger. */
@@ -145,10 +156,18 @@ public class ProviderMetadataEncryptionParametersResolver extends BasicEncryptio
             log.debug("No provider metadata criterion, falling back to local configuration");
             super.resolveAndPopulateCredentialsAndAlgorithms(params, criteria, includeExcludePredicate);
             return;
-        }     
-        
+        }    
         final OIDCProviderMetadata metadata = criteria.get(ProviderMetadataCriterion.class).getMetadata();
         
+        final List<EncryptionConfiguration> encryptionConfigurations =
+                criteria.get(EncryptionConfigurationCriterion.class).getConfigurations();
+        if (encryptionConfigurations == null || encryptionConfigurations.isEmpty()) {
+            log.debug("No encryption configuration, falling back to default configuration");
+            super.resolveAndPopulateCredentialsAndAlgorithms(params, criteria, includeExcludePredicate);
+            return;
+        }
+        
+
         // We populate the parameters for the algorithm the provider has registered
         final List<JWEAlgorithm> keyTransportAlgorithms =  
                 providerKeyTransportAlgorithmsLookupStrategy.apply(metadata);      
@@ -207,86 +226,117 @@ public class ProviderMetadataEncryptionParametersResolver extends BasicEncryptio
             super.resolveAndPopulateCredentialsAndAlgorithms(params, criteria, includeExcludePredicate);
             return;
         } 
+         
+        // Find key wrapping credentials. Find these first, as faster to resolve
+        resolveKeyWrappingCredential(supportedJWEKeyTransportAlgorithms, encryptionConfigurations, 
+                params, encryptionMethod);
         
-        //Check if all supported and configured 'alg's are key wrapping algorithms. If so, use
-        //Key wrapping, otherwise chose a key encryption/agreement algorithm.
-        if (allKeyWrappingAlgorithms(supportedJWEKeyTransportAlgorithms)) {
-            //Do key wrapping
-            log.debug("All configured and supported algorithms are for the key wrapping mode, generating"
-                    + " key wrapping credential");
-            // Choose the first algorithm
-            final JWEAlgorithm algorithm = supportedJWEKeyTransportAlgorithms.get(0);
-            // Pull out the shared secret from the config? Perhaps pull out a shared 'OCT' secret from the
-            // config?
-            //TODO check this logic
-            if (criteria.contains(StaticCredentialCriterion.class)) {
-                final Credential sharedSecret = criteria.get(StaticCredentialCriterion.class).getCredential();
-                //Do more here.
-                params.setKeyTransportEncryptionCredential(sharedSecret);
-                params.setKeyTransportEncryptionAlgorithm(algorithm.getName());
-                params.setDataEncryptionAlgorithm(encryptionMethod.getName());
-                return;
-            } else {
-                log.warn("No shared secret criterion found, can not generate key wrapping credential");
-                super.resolveAndPopulateCredentialsAndAlgorithms(params, criteria, includeExcludePredicate);
-                return;
-            }
-            
+        // Short circuit if found to avoid keyset retrieval
+        if (params.getKeyTransportEncryptionCredential() != null) {
+            return;
         }
         
-        // Find and process key encryption or key agreement modes
-        JWKSet providerKeySet = getProviderKeys(metadata);
+        // Find and process key encryption mode
+        resolveKeyEncryptionCredential(getProviderKeys(metadata), supportedJWEKeyTransportAlgorithms, 
+                params, encryptionMethod);
+        
+        // TODO Key agreement?    
+        
+        
+        if (params.getKeyTransportEncryptionCredential() == null) {
+            log.debug("Not able to locate encryption credentials based on provider metadata or encryption "
+                    + "configuration");
+            super.resolveAndPopulateCredentialsAndAlgorithms(params, criteria, includeExcludePredicate);
+        }      
+        
+
+    }
+    
+    /**
+     * Resolve a suitable key encryption credential from those fetched from the OP's remote key set.
+     * The first compatible key and algorithm are chosen. 
+     * 
+     * @param keySet the providers keyset
+     * @param keyTransportAlgorithms the set of supported key transport algorithms
+     * @param params the encryption parameters to add the credential to
+     * @param encryptionMethod the encryption method to use
+     */
+    private void resolveKeyEncryptionCredential(final JWKSet keySet, 
+            final List<JWEAlgorithm> keyTransportAlgorithms,
+            final EncryptionParameters params, final EncryptionMethod encryptionMethod) {
+        
+        JWKSet providerKeySet = keySet;
         if (providerKeySet == null) {
             providerKeySet = new JWKSet();
-        }
+        }        
         log.trace("Has '{}' keys from provider's JWKSet", providerKeySet.getKeys().size());
         
-        // Keys in the remote keys file are key encryption and agreement algorithms?
         for (final JWK key : providerKeySet.getKeys()) {
             if (KeyUse.SIGNATURE.equals(key.getKeyUse())) {
                 continue;
-            }
-            final JWEAlgorithm keyTransportAlgorithm = 
-                    findSupportedKeyTransportAlgorithm(key, supportedJWEKeyTransportAlgorithms);
-            if (keyTransportAlgorithm != null) {
-                final BasicJWKCredential jwkCredential = new BasicJWKCredential();
-                jwkCredential.setAlgorithm(keyTransportAlgorithm);
-                jwkCredential.setKid(key.getKeyID());
-                try {
-                    if (key.getKeyType().equals(KeyType.RSA)) {
-                        jwkCredential.setPublicKey(((RSAKey) key).toPublicKey());
-                    } else {
-                        jwkCredential.setPublicKey(((ECKey) key).toPublicKey());
+            }            
+            for (final JWEAlgorithm algorithm : keyTransportAlgorithms) {
+                
+                if (key.getAlgorithm().equals(algorithm)) {
+                    final BasicJWKCredential jwkCredential = new BasicJWKCredential();
+                    jwkCredential.setAlgorithm(algorithm);
+                    jwkCredential.setKid(key.getKeyID());
+                    try {
+                        if (key.getKeyType().equals(KeyType.RSA)) {
+                            jwkCredential.setPublicKey(((RSAKey) key).toPublicKey());
+                        } 
+                    } catch (final JOSEException e) {
+                        log.warn("Unable to parse keyset", e);
+                        continue;
                     }
-                } catch (final JOSEException e) {
-                    log.warn("Unable to parse keyset", e);
-                    continue;
-                }
-                log.debug("Selected key {} for alg {} and enc {}", key.getKeyID(), keyTransportAlgorithm.getName(),
-                        encryptionMethod.getName());
-                params.setKeyTransportEncryptionCredential(jwkCredential);
-                params.setKeyTransportEncryptionAlgorithm(keyTransportAlgorithm.getName());
-                params.setDataEncryptionAlgorithm(encryptionMethod.getName());
-                return;
+                    log.debug("Selected key '{}' for alg {} and enc {}", key.getKeyID(), 
+                            algorithm.getName(), encryptionMethod.getName());
+                    params.setKeyTransportEncryptionCredential(jwkCredential);
+                    params.setKeyTransportEncryptionAlgorithm(algorithm.getName());
+                    params.setDataEncryptionAlgorithm(encryptionMethod.getName());
+                    return;                
+                }            
             }
-            
         }
-        //TODO THIS IS NOT CORRECT, needs much review, e.g. key wrapping,  key aggreement.
-
+        
     }
     
     /**
-     * If all the supported algorithms are symmetric key wrapping algorithms, return true. Return false otherwise.
+     * Resolve a suitable key wrapping credential from those stored in the encryption configurations.   
+     * The first compatible key and algorithm are chosen.
      * 
-     * @param algorithms the alogorithms to check
-     * 
-     * @return true if all algorithms are symmetric key wrapping algorithms, false otherwise.
+     * @param keyTransportAlgorithms the set of supported key transport algorithms
+     * @param encryptionConfigurations the encryption configurations where the credentials are stored
+     * @param params the encryption parameters to add the credential to
+     * @param encryptionMethod the encryption method to use
      */
-    private boolean allKeyWrappingAlgorithms(@Nonnull @NotEmpty final List<JWEAlgorithm> algorithms) {
-        return algorithms.stream().allMatch(
-                PredicateSupport.or(JWEAlgorithm.Family.AES_GCM_KW::contains, JWEAlgorithm.Family.AES_KW::contains));
+    private void resolveKeyWrappingCredential(final List<JWEAlgorithm> keyTransportAlgorithms,
+            final List<EncryptionConfiguration> encryptionConfigurations,
+            final EncryptionParameters params, final EncryptionMethod encryptionMethod) {
+        
+        for (final EncryptionConfiguration config : encryptionConfigurations) {
+            for (final Credential credential : config.getKeyTransportEncryptionCredentials()) {  
+                
+                for (final JWEAlgorithm algorithm : keyTransportAlgorithms) {                
+                    if (credential instanceof JWKCredential && credential.getSecretKey() != null && 
+                            algorithm.equals(((JWKCredential)credential).getAlgorithm())) {
+                        
+                            if (log.isDebugEnabled()) {
+                                log.debug("Selected key '{}' for alg {} and enc {}", 
+                                        JWKSupport.extractKeyName(credential),
+                                        algorithm.getName(), encryptionMethod.getName());
+                            }
+                            params.setKeyTransportEncryptionCredential(credential);
+                            params.setKeyTransportEncryptionAlgorithm(algorithm.getName());
+                            params.setDataEncryptionAlgorithm(encryptionMethod.getName());
+                            return;
+                        }                    
+                }
+            }
+        }
     }
 
+
     /**
      * Return the first encryption method in the supported list, or null otherwise.
      * 
@@ -312,30 +362,6 @@ public class ProviderMetadataEncryptionParametersResolver extends BasicEncryptio
         return algos.stream().map(JWEAlgorithm::parse).collect(Collectors.toList());
     }
     
-    /**
-     * Does the key support any one of the given algorithms. The key has to be either an RSA, or EC type.
-     * 
-     * @param key the key to check
-     * @param algorithms the algorithms to check against
-     * 
-     * @return the supported algorithm, or null if none are supported
-     */
-    @Nullable private JWEAlgorithm findSupportedKeyTransportAlgorithm(@Nonnull final JWK key, 
-            @Nonnull final List<JWEAlgorithm> algorithms) {  
-        
-        final JWEAlgorithm algorithm =
-                algorithms.stream().filter(alg -> alg.equals(key.getAlgorithm())).findFirst().orElse(null);
-        
-        if ((JWEAlgorithm.Family.RSA.contains(algorithm) && 
-                key.getKeyType().equals(KeyType.RSA))
-                || (JWEAlgorithm.Family.ECDH_ES.contains(algorithm) && 
-                        key.getKeyType().equals(KeyType.EC))) {
-            return algorithm;
-        }         
-        // No support
-        return null;
-    }
-    
     /**
      * Fetch the OpenID Provider's remote JWKSet.
      * 
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/RelyingPartyProxySigningParametersResolver.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/RelyingPartyProxySigningParametersResolver.java
index 5c02095..40c9081 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/RelyingPartyProxySigningParametersResolver.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/RelyingPartyProxySigningParametersResolver.java
@@ -21,7 +21,6 @@ import java.security.interfaces.ECPrivateKey;
 import java.security.interfaces.RSAPrivateKey;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.StringJoiner;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -40,12 +39,11 @@ import com.nimbusds.jose.JWSAlgorithm;
 import com.nimbusds.jose.jwk.Curve;
 import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata;
 
-import net.shibboleth.oidc.security.credential.JWKCredential;
+import net.shibboleth.idp.plugin.authn.oidc.rp.support.JWKSupport;
 import net.shibboleth.oidc.security.criterion.ProviderMetadataCriterion;
 import net.shibboleth.oidc.security.criterion.StaticCredentialCriterion;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 import net.shibboleth.utilities.java.support.logic.FunctionSupport;
-import net.shibboleth.utilities.java.support.primitive.StringSupport;
 import net.shibboleth.utilities.java.support.resolver.CriteriaSet;
 
 /**
@@ -124,7 +122,7 @@ public class RelyingPartyProxySigningParametersResolver extends BasicSignatureSi
         // Pick the first credential that matches one of the supported algorithms
         for (final Credential credential : allCredentials) {   
             if (log.isTraceEnabled()) {
-                log.trace("Evaluating signing credential '{}'", extractKeyName(credential));
+                log.trace("Evaluating signing credential '{}'", JWKSupport.extractKeyName(credential));
             }
             final JWSAlgorithm foundSupportedAlgorithm = 
                     credentialSupportsSigningAlgorithm(credential, supportedAlgorithms);
@@ -138,21 +136,7 @@ public class RelyingPartyProxySigningParametersResolver extends BasicSignatureSi
             
         }
     }
-    
-    /**
-     * Extract the credentials name for display. Favouring the keyId if there is one, over the keyNames.
-     * 
-     * @param credential the credential to extract the name from
-     * 
-     * @return the key name. 
-     */
-    private String extractKeyName(final Credential credential) {
-        if (credential instanceof JWKCredential && 
-                StringSupport.trimOrNull(((JWKCredential)credential).getKid()) != null) {
-            return ((JWKCredential)credential).getKid();
-        } 
-        return credential.getKeyNames().stream().collect(Collectors.joining(","));     
-    }
+   
     
     /** {@inheritDoc} 
      * 
diff --git a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/EncryptJWT.java b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/EncryptJWT.java
index 60616a4..f5bb11c 100644
--- a/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/EncryptJWT.java
+++ b/idp-oidc-rp-impl/src/main/java/net/shibboleth/idp/plugin/authn/oidc/rp/messaging/impl/EncryptJWT.java
@@ -202,7 +202,7 @@ public class EncryptJWT extends AbstractMessageHandler {
             }
             
         } catch (final JOSEException | ParseException e) {
-            log.error("{} Encryption failed {}", getLogPrefix(), e);           
+            log.error("{} Encryption failed", getLogPrefix(), e);           
             throw new MessageHandlerException("Encryption failed", e);
         }
         
diff --git a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
index 29e010f..1a2a8a0 100644
--- a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
+++ b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-beans.xml
@@ -169,7 +169,6 @@
         class="net.shibboleth.idp.plugin.authn.oidc.rp.messaging.context.logic.RequestObjectRequiredAndSupported" />
 
 
-
     <bean id="SignRequestObjectProxyCondition" 
                 class="net.shibboleth.idp.plugin.authn.oidc.rp.config.logic.SignRequestObjectPredicate"
                 p:relyingPartyContextLookupStrategy-ref="shibboleth.ChildLookup.Proxy.RelyingPartyContext"/>
diff --git a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-flow.xml b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-flow.xml
index a43ae1d..917e5fd 100644
--- a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-flow.xml
+++ b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/flows/authn/OIDCRelyingParty/oidc-relying-party-authn-flow.xml
@@ -1,5 +1,7 @@
-<flow xmlns="http://www.springframework.org/schema/webflow" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-    xsi:schemaLocation="http://www.springframework.org/schema/webflow http://www.springframework.org/schema/webflow/spring-webflow.xsd"
+<flow xmlns="http://www.springframework.org/schema/webflow" 
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://www.springframework.org/schema/webflow 
+    http://www.springframework.org/schema/webflow/spring-webflow.xsd"
     parent="authn.abstract, authn/conditions">
 
     <!-- This is a login flow for proxied authentication implemented via OIDC -->
@@ -42,19 +44,19 @@
         <evaluate expression="AddRequestedClaims" />
         <evaluate expression="AddRedirectURI"/>
         <evaluate expression="AddAuthenticationContextClassReferences" />
-        <!-- <evaluate expression="PostRequestPopulateAuditContext" /> <evaluate expression="WriteAuditLog" /> -->
-
+        <!-- <evaluate expression="PostRequestPopulateAuditContext" /> -->
+        <!-- <evaluate expression="WriteAuditLog" /> -->
         <!-- <evaluate expression="InitializeMessageChannelSecurityContext" /> -->
         <evaluate expression="'proceed'" />
 
         <transition on="proceed" to="RequestObjectRequiredAndSupported" />
     </action-state>
 
-    <!-- Is a request object requested by the config, and does the OP support it? -->
+    <!-- Is a request object required by the config, and does the OP support it? -->
     <decision-state id="RequestObjectRequiredAndSupported">
-        <if
-            test="RequestObjectRequiredAndSupportedPredicate.test(opensamlProfileRequestContext.getSubcontext('net.shibboleth.idp.authn.context.AuthenticationContext').getSubcontext('org.opensaml.profile.context.ProfileRequestContext'))"
-            then="BuildRequestObject" else="AuthnRequest" />
+        <if test="RequestObjectRequiredAndSupportedPredicate.test(ProxyProfileRequestContextLookup.apply(opensamlProfileRequestContext))"
+            then="BuildRequestObject" 
+            else="AuthnRequest" />
     </decision-state>
 
     <action-state id="BuildRequestObject">
@@ -95,7 +97,7 @@
         <transition on="proceed" to="SwitchOnGrantType" />
     </action-state>
 
-    <!-- Switch flow path based on OIDC grant_type used -->
+    <!-- Switch flow path based on OIDC grant_type used. Only code flow is actually supported currently. -->
     <decision-state id="SwitchOnGrantType">
         <if test="IsCodeFlow.test(OutboundMessageContextFromRootPRC.apply(opensamlProfileRequestContext))" 
             then="CodeFlow" />
@@ -137,7 +139,7 @@
     <!-- A plain JWT will skip token validation and go straight to claims validation -->
     <decision-state id="CheckUserInfoResponseType">
         <if test="CheckUserInfoPlainResponseTypeCondition.test(ProxyProfileRequestContextLookup.apply(opensamlProfileRequestContext))"
-            then="ValidateUserInfoPlaimClaimsSet" else="ValidateUserInfoJWT" />
+            then="ValidateUserInfoPlainClaimsSet" else="ValidateUserInfoJWT" />
     </decision-state>
 
     <!-- Actions to perform if the UserInfo response is a JWT type -->
@@ -152,7 +154,7 @@
     </action-state>
 
     <!-- Plain UserInfo response types will skip straight to this stage -->
-    <action-state id="ValidateUserInfoPlaimClaimsSet">
+    <action-state id="ValidateUserInfoPlainClaimsSet">
         <evaluate expression="ValidateUserInfoPlainResponseClaims" />
         <evaluate expression="'proceed'" />
         <transition on="proceed" to="FinalizeResponse" />
diff --git a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/service/relying-party/postconfig.xml b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/service/relying-party/postconfig.xml
index 10ef5fe..6c9d816 100644
--- a/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/service/relying-party/postconfig.xml
+++ b/idp-oidc-rp-impl/src/main/resources/META-INF/net/shibboleth/idp/service/relying-party/postconfig.xml
@@ -94,7 +94,8 @@
     </bean>
     
     <!-- Configuration for supported algorithms for Request Object encryption. -->
-    <bean id="shibboleth.authn.oidc.rp.EncryptionConfiguration" parent="shibboleth.BasicEncryptionConfiguration">
+    <bean id="shibboleth.authn.oidc.rp.EncryptionConfiguration" parent="shibboleth.BasicEncryptionConfiguration"
+        p:keyTransportEncryptionCredentials="#{getObject('shibboleth.authn.oidc.rp.RequestObjectEncryptionCredentials')}">
         <property name="keyTransportEncryptionAlgorithms">
             <list>
                 <util:constant
@@ -112,7 +113,7 @@
                 <util:constant
                     static-field="net.shibboleth.oidc.jwa.support.KeyManagementConstants.ALGO_ID_ALG_AES_128_GCM_KW" />
                 <util:constant
-                    static-field="net.shibboleth.oidc.jwa.support.KeyManagementConstants.ALGO_ID_ALG_AES_192_GCM_KW" />
+                    static-field="net.shibboleth.oidc.jwa.support.KeyManagementConstants.ALGO_ID_ALG_AES_192_GCM_KW" /> 
                 <util:constant
                     static-field="net.shibboleth.oidc.jwa.support.KeyManagementConstants.ALGO_ID_ALG_AES_256_GCM_KW" />
             </list>
@@ -128,7 +129,7 @@
                 <util:constant
                     static-field="net.shibboleth.oidc.jwa.support.EncryptionConstants.ALGO_ID_ENC_ALG_A128GCM" />
                 <util:constant
-                    static-field="net.shibboleth.oidc.jwa.support.EncryptionConstants.ALGO_ID_ENC_ALG_A192GCM" />
+                    static-field="net.shibboleth.oidc.jwa.support.EncryptionConstants.ALGO_ID_ENC_ALG_A192GCM" /> 
                 <util:constant
                     static-field="net.shibboleth.oidc.jwa.support.EncryptionConstants.ALGO_ID_ENC_ALG_A256GCM" />
             </list>
@@ -163,6 +164,10 @@
     <bean id="shibboleth.authn.oidc.rp.SigningCredentials" 
         class="net.shibboleth.idp.plugin.authn.oidc.rp.config.CredentialsListFactory"
         c:_0-ref="shibboleth.authn.oidc.rp.DefaultSigningCredentials"/>
+        
+    <bean id="shibboleth.authn.oidc.rp.RequestObjectEncryptionCredentials" 
+        class="net.shibboleth.idp.plugin.authn.oidc.rp.config.CredentialsListFactory"
+        c:_0-ref="shibboleth.authn.oidc.rp.DefaultRequestObjectEncryptionCredentials"/>
 
     
 
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java
index 9f47937..76e042a 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/OIDCRPFlowTest.java
@@ -17,6 +17,11 @@
 
 package net.shibboleth.idp.plugin.authn.oidc.rp.impl;
 
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.fail;
+
 import java.net.InetAddress;
 import java.net.URI;
 import java.net.UnknownHostException;
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProviderMetadataEncryptionParametersResolverTest.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProviderMetadataEncryptionParametersResolverTest.java
index 21a653f..44e431d 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProviderMetadataEncryptionParametersResolverTest.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/ProviderMetadataEncryptionParametersResolverTest.java
@@ -44,6 +44,7 @@ import org.springframework.core.io.ClassPathResource;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import com.nimbusds.jose.JWEAlgorithm;
 import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata;
 
 import net.shibboleth.oidc.jwa.support.EncryptionConstants;
@@ -70,6 +71,9 @@ public class ProviderMetadataEncryptionParametersResolverTest {
     /** The client_secret.*/
     private static final String CLIENT_SECRET = "Xp2s5v8y/B?E(H+MbQeThWmYq3t6w9z$";
     
+    /** The mock symmetric key e.g. for keywrap.*/
+    private static final String SYMMETRIC_KEY = "/A?D(G+KbPdSgVkYp3s6v9y$B&E)H at Mc";
+    
     /** The resolver to test.*/
     private ProviderMetadataEncryptionParametersResolver resolver;
     
@@ -143,7 +147,7 @@ public class ProviderMetadataEncryptionParametersResolverTest {
     
     /* Algorithms are know because they are limited by config.*/
     @Test
-    public void testSuccessfulResolution_WithKnownAlgorithms() throws Exception {
+    public void testSuccessfulResolution_ForKeyEncryption() throws Exception {
         final CriteriaSet criteria =  buildBasicCriteriaSet();
         config.setKeyTransportEncryptionAlgorithms(List.of(KeyManagementConstants.ALGO_ID_ALG_RSA_OAEP));
         config.setDataEncryptionAlgorithms(List.of(EncryptionConstants.ALGO_ID_ENC_ALG_A256CBC_HS512));
@@ -160,11 +164,55 @@ public class ProviderMetadataEncryptionParametersResolverTest {
         assertNotNull(param.getKeyTransportEncryptionCredential().getPublicKey());
     }
     
+    /* Should chose key encryption creds as they are the only ones configured.*/
+    @Test
+    public void testSuccessfulResolution_ForKeyEncryption_WhenKeyWrapPossible() throws Exception {
+        final CriteriaSet criteria =  buildBasicCriteriaSet();
+        config.setKeyTransportEncryptionAlgorithms(List.of(KeyManagementConstants.ALGO_ID_ALG_RSA_OAEP, 
+                KeyManagementConstants.ALGO_ID_ALG_AES_128_KW));
+        config.setDataEncryptionAlgorithms(List.of(EncryptionConstants.ALGO_ID_ENC_ALG_A256CBC_HS512));
+        
+        final Iterable<EncryptionParameters> params = resolver.resolve(criteria);
+        assertNotNull(params);
+        assertTrue(params.iterator().hasNext());
+        final EncryptionParameters param = params.iterator().next();
+        assertNotNull(param.getDataEncryptionAlgorithm());
+        assertNotNull(param.getKeyTransportEncryptionAlgorithm());  
+        assertEquals(param.getDataEncryptionAlgorithm(),EncryptionConstants.ALGO_ID_ENC_ALG_A256CBC_HS512);
+        assertEquals(param.getKeyTransportEncryptionAlgorithm(),KeyManagementConstants.ALGO_ID_ALG_RSA_OAEP);
+        assertNotNull(param.getKeyTransportEncryptionCredential());
+        assertNotNull(param.getKeyTransportEncryptionCredential().getPublicKey());
+    }
+    
+    /* Should chose key wrap creds first.*/
+    @Test
+    public void testSuccessfulResolution_ForKeyWrap_WhenKeyEncryptionPossible() throws Exception {
+        final CriteriaSet criteria =  buildBasicCriteriaSet();
+        config.setKeyTransportEncryptionAlgorithms(List.of(KeyManagementConstants.ALGO_ID_ALG_RSA_OAEP, 
+                KeyManagementConstants.ALGO_ID_ALG_AES_256_KW));
+        config.setDataEncryptionAlgorithms(List.of(EncryptionConstants.ALGO_ID_ENC_ALG_A256CBC_HS512));
+        config.setKeyTransportEncryptionCredentials(
+                List.of(TestCredentialHelper.createClientSecretCredential(SYMMETRIC_KEY, JWEAlgorithm.A256KW)));
+        
+        final Iterable<EncryptionParameters> params = resolver.resolve(criteria);
+        assertNotNull(params);
+        assertTrue(params.iterator().hasNext());
+        final EncryptionParameters param = params.iterator().next();
+        assertNotNull(param.getDataEncryptionAlgorithm());
+        assertNotNull(param.getKeyTransportEncryptionAlgorithm());  
+        assertEquals(param.getDataEncryptionAlgorithm(),EncryptionConstants.ALGO_ID_ENC_ALG_A256CBC_HS512);
+        assertEquals(param.getKeyTransportEncryptionAlgorithm(),KeyManagementConstants.ALGO_ID_ALG_AES_256_KW);
+        assertNotNull(param.getKeyTransportEncryptionCredential());
+        assertNotNull(param.getKeyTransportEncryptionCredential().getSecretKey());
+    }
+    
     @Test
     public void testSuccessfulResolution_ForKeyWrap() throws Exception {
         final CriteriaSet criteria =  buildBasicCriteriaSet();
         config.setKeyTransportEncryptionAlgorithms(List.of(KeyManagementConstants.ALGO_ID_ALG_AES_256_KW));
         config.setDataEncryptionAlgorithms(List.of(EncryptionConstants.ALGO_ID_ENC_ALG_A128GCM));
+        config.setKeyTransportEncryptionCredentials(
+                List.of(TestCredentialHelper.createClientSecretCredential(SYMMETRIC_KEY, JWEAlgorithm.A256KW)));
         
         final Iterable<EncryptionParameters> params = resolver.resolve(criteria);
         assertNotNull(params);
@@ -178,6 +226,18 @@ public class ProviderMetadataEncryptionParametersResolverTest {
         assertNotNull(param.getKeyTransportEncryptionCredential().getSecretKey());
     }
     
+    /* Do not provide a symmetric key in the params.*/
+    @Test
+    public void testUnSuccessfulResolution_ForKeyWrap() throws Exception {
+        final CriteriaSet criteria =  buildBasicCriteriaSet();
+        config.setKeyTransportEncryptionAlgorithms(List.of(KeyManagementConstants.ALGO_ID_ALG_AES_256_KW));
+        config.setDataEncryptionAlgorithms(List.of(EncryptionConstants.ALGO_ID_ENC_ALG_A128GCM));
+        
+        final Iterable<EncryptionParameters> params = resolver.resolve(criteria);
+        assertNotNull(params);
+        assertFalse(params.iterator().hasNext());    
+    }
+    
     /* 
      * Can not test ECDH-ES as the net.shibboleth.oidc.jwa.algorithm.descriptors.KeyAgreementECDHES is not
      * supported by the runtime!
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/TestCredentialHelper.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/TestCredentialHelper.java
index 8f8460e..62f4821 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/TestCredentialHelper.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/TestCredentialHelper.java
@@ -29,6 +29,7 @@ import com.nimbusds.jose.JWEAlgorithm;
 import com.nimbusds.jose.jwk.AsymmetricJWK;
 import com.nimbusds.jose.jwk.JWK;
 import com.nimbusds.jose.jwk.RSAKey;
+import com.nimbusds.jose.Algorithm;
 
 import net.shibboleth.oidc.security.credential.BasicExpiringJWKCredential;
 import net.shibboleth.oidc.security.credential.JWKCredential;
@@ -42,20 +43,34 @@ public final class TestCredentialHelper {
     }
     
     /**
-     * Create a simple client credential from from the given shared secret.
+     * Create a simple symmetric key client credential from from the given shared secret.
      * 
      * @param secret the secret to convert to a {@link JWKCredential}.
      * 
      * @return the credential
      * @throws KeyException on error creating the key
      */
-    //TODO used for both signing and encryption, so alg needs to reflect this
-    public static JWKCredential createClientSecretCredential(final String secret) throws KeyException {
+    public static JWKCredential createClientSecretCredential(final String secret) throws KeyException {        
+        return createClientSecretCredential(secret, null);
+    }
+    
+    /**
+     * Create a simple symmetric key client credential from from the given shared secret.
+     * 
+     * @param secret the secret to convert to a {@link JWKCredential}.
+     * @param algorithm the JWA algorithm to set on the credential.
+     * 
+     * @return the credential
+     * @throws KeyException on error creating the key
+     */
+    public static JWKCredential createClientSecretCredential(final String secret, final Algorithm algorithm) 
+            throws KeyException {
         final BasicExpiringJWKCredential jwkCredential = new BasicExpiringJWKCredential();
         jwkCredential.setSecretKey(KeySupport.decodeSecretKey(JWSAssemblyUtils.getSecretBytes(secret), "AES"));
         jwkCredential.setCredentialExpiresAt(Duration.ZERO);
         jwkCredential.setUsageType(UsageType.UNSPECIFIED);
         jwkCredential.setKid("mockKey");
+        jwkCredential.setAlgorithm(algorithm);
         jwkCredential.getKeyNames().add("mockKey");
         return jwkCredential;
     }
diff --git a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/TestJsonHelper.java b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/TestJsonHelper.java
index 157250a..557926d 100644
--- a/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/TestJsonHelper.java
+++ b/idp-oidc-rp-impl/src/test/java/net/shibboleth/idp/plugin/authn/oidc/rp/impl/TestJsonHelper.java
@@ -15,7 +15,6 @@
  * limitations under the License.
  */
 
-
 package net.shibboleth.idp.plugin.authn.oidc.rp.impl;
 
 import static org.testng.Assert.fail;
diff --git a/idp-oidc-rp-impl/src/test/resources/conf/authn/rp-credentials.xml b/idp-oidc-rp-impl/src/test/resources/conf/authn/rp-credentials.xml
index d660e13..e45060a 100644
--- a/idp-oidc-rp-impl/src/test/resources/conf/authn/rp-credentials.xml
+++ b/idp-oidc-rp-impl/src/test/resources/conf/authn/rp-credentials.xml
@@ -15,8 +15,13 @@
         p:resource="%{idp.authn.oidc.rp.client.enc.key:#{null}}" />
     </util:list>
     
+    <util:list id="shibboleth.authn.oidc.rp.DefaultRequestObjectEncryptionCredentials">
+        <bean parent="shibboleth.authn.oidc.rp.JWKCredential" p:failIfResourceIsNull="false" 
+        p:resource="%{idp.authn.oidc.rp.client.requestobject.enc.key:#{null}}" />
+    </util:list>
+    
      <!-- Default signing credentials -->
-     <util:list id="shibboleth.authn.oidc.rp.SigningCredentials">
+     <util:list id="shibboleth.authn.oidc.rp.DefaultSigningCredentials">
         <bean parent="shibboleth.authn.oidc.rp.JWKCredential" p:failIfResourceIsNull="false" 
         p:resource="%{idp.authn.oidc.rp.client.sig.key:#{null}}" />
     </util:list>

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


More information about the commits mailing list