[java-shib-attribute] branch main updated: OSJ-391: Default supported TLS protocols appears too broad

Brent Putman putmanb at georgetown.edu
Fri Feb 9 16:00:35 UTC 2024


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

putmanb pushed a commit to branch main
in repository java-shib-attribute.

View the commit online:
http://git.shibboleth.net/view/?p=java-shib-attribute.git;a=commit;h=3228e69d02798a3a6d8ce2a1aa83b73274fecdec

The following commit(s) were added to refs/heads/main by this push:
     new 3228e69d0 OSJ-391: Default supported TLS protocols appears too broad
3228e69d0 is described below

commit 3228e69d02798a3a6d8ce2a1aa83b73274fecdec
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Fri Feb 9 06:59:22 2024 -0500

    OSJ-391: Default supported TLS protocols appears too broad
    
    Refactor HTTP data connector support for HttpClientSecurityParams to use
    new factory bean which merges the inline attribute and security params
    ref inputs.
---
 .../dc/http/impl/HTTPDataConnectorParser.java      | 70 ++++++++++++++--------
 .../dc/http/impl/HTTPDataConnectorParserTest.java  | 31 ++++++++++
 ...ttp-attribute-resolver-v2-goodprotocols-ref.xml | 23 +++++++
 .../resolver/spring/dc/http/spring-beans.xml       |  4 ++
 4 files changed, 102 insertions(+), 26 deletions(-)

diff --git a/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/dc/http/impl/HTTPDataConnectorParser.java b/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/dc/http/impl/HTTPDataConnectorParser.java
index 42d7b3322..dd73f2b31 100644
--- a/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/dc/http/impl/HTTPDataConnectorParser.java
+++ b/shib-attribute-resolver-spring/src/main/java/net/shibboleth/idp/attribute/resolver/spring/dc/http/impl/HTTPDataConnectorParser.java
@@ -22,12 +22,16 @@ import javax.xml.namespace.QName;
 
 import org.opensaml.security.httpclient.HttpClientSecurityParameters;
 import org.opensaml.spring.credential.BasicX509CredentialFactoryBean;
+import org.opensaml.spring.httpclient.HttpClientSecurityParametersMergingFactoryBean;
 import org.opensaml.spring.trust.StaticExplicitKeyFactoryBean;
 import org.opensaml.spring.trust.StaticPKIXFactoryBean;
 import org.slf4j.Logger;
+import org.springframework.beans.BeanMetadataElement;
 import org.springframework.beans.factory.BeanCreationException;
 import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
+import org.springframework.beans.factory.support.ManagedList;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.w3c.dom.Attr;
 import org.w3c.dom.Element;
@@ -87,28 +91,21 @@ public class HTTPDataConnectorParser extends AbstractDataConnectorParser {
             builder.addPropertyReference("httpClient", httpClientID);
         }
         
-        final String securityParams =
-                StringSupport.trimOrNull(config.getAttributeNS(null, "httpClientSecurityParametersRef"));
-        final BeanDefinition httpClientSecurityBean =
-                v2Parser.buildHttpClientSecurityParams(config.getAttributeNS(null, "id"));
-        if (httpClientSecurityBean != null) {
-            if (securityParams != null) {
-                log.warn("Ignoring httpClientSecurityParametersRef setting in favor of manual settings");
-            }
-            builder.addPropertyValue("httpClientSecurityParameters", httpClientSecurityBean);
-        } else if (securityParams != null) {
-            builder.addPropertyReference("httpClientSecurityParameters", securityParams);
-        }
+        final BeanDefinition httpClientSecurityParameters = buildHttpClientSecurityParameters(
+                v2Parser.buildHttpClientSecurityParams(config.getAttributeNS(null, "id")),
+                StringSupport.trimOrNull(config.getAttributeNS(null, "httpClientSecurityParametersRef")),
+                parserContext);
+        builder.addPropertyValue("httpClientSecurityParameters", httpClientSecurityParameters);
 
         final String searchBuilderID = v2Parser.getBeanSearchBuilderID();
         if (searchBuilderID != null) {
             builder.addPropertyReference("executableSearchBuilder", searchBuilderID);
         } else {
-            BeanDefinition def = v2Parser.createBodyTemplateBuilder(httpClientSecurityBean);
+            BeanDefinition def = v2Parser.createBodyTemplateBuilder(httpClientSecurityParameters);
             if (def != null) {
                 builder.addPropertyValue("executableSearchBuilder", def);
             } else {
-                def = v2Parser.createURLTemplateBuilder(httpClientSecurityBean);
+                def = v2Parser.createURLTemplateBuilder(httpClientSecurityParameters);
                 if (def != null) {
                     builder.addPropertyValue("executableSearchBuilder", def);
                 }
@@ -142,6 +139,39 @@ public class HTTPDataConnectorParser extends AbstractDataConnectorParser {
         builder.setDestroyMethodName("destroy");
     }
 // Checkstyle: CyclomaticComplexity|MethodLength ON
+    
+    /**
+     * Build the definition of the HttpClientSecurityParameters as a factory bean which merges the inline params data 
+     * and parameters ref inputs.
+     * 
+     * @param inlineParams the bean definition for the inline params data such as TLS TrustEngine and client Credential
+     * @param parametersRef the security parameters bean reference
+     * @param parserContext context
+     * @return the bean definition with the parameters.
+     */
+    @Nonnull protected static BeanDefinition buildHttpClientSecurityParameters(@Nullable final BeanDefinition inlineParams,
+            @Nullable final String parametersRef, @Nonnull final ParserContext parserContext) {
+        
+        final BeanDefinitionBuilder factoryBuilder =
+                BeanDefinitionBuilder.genericBeanDefinition(HttpClientSecurityParametersMergingFactoryBean.class);
+        
+        final List<BeanMetadataElement> factoryInputs = new ManagedList<>(2);
+
+        // First order-of-precedence
+        if (inlineParams != null)  {
+            factoryInputs.add(inlineParams);
+        }
+
+        // Second order-of-precedence
+        if (parametersRef != null) {
+            factoryInputs.add(new RuntimeBeanReference(parametersRef));
+        }
+
+        factoryBuilder.addPropertyValue("parameters", factoryInputs);
+        
+        return factoryBuilder.getBeanDefinition();
+    }
+    
 
     /**
      * Utility class for parsing v2 schema configuration.
@@ -218,12 +248,6 @@ public class HTTPDataConnectorParser extends AbstractDataConnectorParser {
             // This is duplication but allows the built-in builder to access the parameters if desired.
             if (httpClientSecurityParams != null) {
                 templateBuilder.addPropertyValue("httpClientSecurityParameters", httpClientSecurityParams);
-            } else {
-                final String securityParamsRef =
-                        StringSupport.trimOrNull(configElement.getAttributeNS(null, "httpClientSecurityParametersRef"));
-                if (securityParamsRef != null) {
-                    templateBuilder.addPropertyReference("httpClientSecurityParameters", securityParamsRef);
-                }
             }
             
             if (urlTemplates.size() > 1) {
@@ -287,12 +311,6 @@ public class HTTPDataConnectorParser extends AbstractDataConnectorParser {
             // This is duplication but allows the built-in builder to access the parameters if desired.
             if (httpClientSecurityParams != null) {
                 templateBuilder.addPropertyValue("httpClientSecurityParameters", httpClientSecurityParams);
-            } else {
-                final String securityParamsRef =
-                        StringSupport.trimOrNull(configElement.getAttributeNS(null, "httpClientSecurityParametersRef"));
-                if (securityParamsRef != null) {
-                    templateBuilder.addPropertyReference("httpClientSecurityParameters", securityParamsRef);
-                }
             }
             
             if (urlTemplates.size() > 1) {
diff --git a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/dc/http/impl/HTTPDataConnectorParserTest.java b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/dc/http/impl/HTTPDataConnectorParserTest.java
index 6eb47baad..a9831f1e5 100644
--- a/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/dc/http/impl/HTTPDataConnectorParserTest.java
+++ b/shib-attribute-resolver-spring/src/test/java/net/shibboleth/idp/attribute/resolver/spring/dc/http/impl/HTTPDataConnectorParserTest.java
@@ -105,6 +105,37 @@ public class HTTPDataConnectorParserTest {
         assertTrue(cache.size() == 1);
     }
 
+    @Test public void v2GoodProtocolsViaRef() throws Exception {
+        
+        final MockPropertySource propSource = singletonPropertySource("serviceURL", TEST_URL);
+        propSource.setProperty("scriptPath", (SCRIPT_PATH) + "test.js");
+        propSource.setProperty("userAgent", "disguised/1.0.0 hidden/3.4.5");
+        propSource.setProperty("certificateAuthority", CREDS_PATH + "repo-rootCA.crt");
+        
+        final HTTPDataConnector connector =
+                getDataConnector(propSource,
+                        "net/shibboleth/idp/attribute/resolver/spring/dc/http/http-attribute-resolver-v2-goodprotocols-ref.xml");
+        assertNotNull(connector);
+        
+        final AttributeResolutionContext context =
+                TestSources.createResolutionContext(TestSources.PRINCIPAL_ID, TestSources.IDP_ENTITY_ID,
+                        TestSources.SP_ENTITY_ID);
+        
+        final Map<String,IdPAttribute> attrs = connector.resolve(context);
+        assert attrs != null;
+        assertEquals(attrs.size(), 2);
+        
+        assertEquals(attrs.get("foo").getValues().size(), 1);
+        assertEquals(((StringAttributeValue)attrs.get("foo").getValues().get(0)).getValue(), "foo1");
+        
+        assertEquals(attrs.get("bar").getValues().size(), 2);
+        assertEquals(((StringAttributeValue)attrs.get("bar").getValues().get(0)).getValue(), "bar1");
+        assertEquals(((StringAttributeValue)attrs.get("bar").getValues().get(1)).getValue(), "bar2");
+        final Cache<String, Map<String, IdPAttribute>> cache = connector.getResultsCache();
+        assert cache != null;
+        assertTrue(cache.size() == 1);
+    }
+
     @Test(expectedExceptions=ResolutionException.class) public void v2BadProtocol() throws Exception {
         
         final MockPropertySource propSource = singletonPropertySource("serviceURL", TEST_URL);
diff --git a/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/http/http-attribute-resolver-v2-goodprotocols-ref.xml b/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/http/http-attribute-resolver-v2-goodprotocols-ref.xml
new file mode 100644
index 000000000..0f7a41ca8
--- /dev/null
+++ b/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/http/http-attribute-resolver-v2-goodprotocols-ref.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<AttributeResolver 
+            xmlns="urn:mace:shibboleth:2.0:resolver" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
+            xsi:schemaLocation="urn:mace:shibboleth:2.0:resolver http://shibboleth.net/schema/idp/shibboleth-attribute-resolver.xsd">
+
+    <DataConnector id="myHTTP" xsi:type="HTTP"
+            acceptStatuses="200 201"
+            httpClientRef="TrustEngineHttpClient"
+            httpClientSecurityParametersRef="GoodProtocolParameters"
+            certificateAuthority="%{certificateAuthority}"
+            acceptTypes="application/json">
+            
+        <URLTemplate customObjectRef="CustomObject">%{serviceURL}</URLTemplate>
+        
+        <ResponseMapping>
+            <ScriptFile>%{scriptPath}</ScriptFile>
+        </ResponseMapping>
+        
+        <ResultCache expireAfterWrite="PT10S" />
+        
+    </DataConnector>
+    
+</AttributeResolver>
diff --git a/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/http/spring-beans.xml b/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/http/spring-beans.xml
index 00d35bc31..4ba86b75b 100644
--- a/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/http/spring-beans.xml
+++ b/shib-attribute-resolver-spring/src/test/resources/net/shibboleth/idp/attribute/resolver/spring/dc/http/spring-beans.xml
@@ -32,6 +32,10 @@
         class="org.opensaml.security.httpclient.HttpClientSecurityParameters"
         p:TLSProtocols="SSLv3" />
 
+    <bean id="GoodProtocolParameters"
+        class="org.opensaml.security.httpclient.HttpClientSecurityParameters"
+        p:TLSProtocols="#{ {'TLSv1.3', 'TLSv1.2'} }" />
+
     <bean id="shibboleth.VelocityEngine"
             class="net.shibboleth.shared.spring.velocity.VelocityEngineFactoryBean">
         <property name="velocityProperties">

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


More information about the commits mailing list