Merged in SHIBUI-984 (pull request #255)

SHIBUI-984 Approved-by: Bill Smith <wsmith@unicon.net> Approved-by: Dmitriy Kopylenko <dkopylenko@unicon.net>
TIER · Dec 11, 2018 · de6a5ef · de6a5ef
2 parents 2440647 + 81b0fed
commit de6a5ef
Show file tree

Hide file tree

Showing 8 changed files with 231 additions and 1 deletion.
diff --git a/...oovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy b/...oovy/edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImpl.groovy
@@ -1,6 +1,7 @@
 package edu.internet2.tier.shibboleth.admin.ui.service
 
 import com.google.common.base.Predicate
+import edu.internet2.tier.shibboleth.admin.ui.configuration.ShibUIConfiguration
 import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter
 import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilterTarget
 import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityRoleWhiteListFilter
@@ -52,6 +53,9 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
     @Autowired
     private MetadataResolversPositionOrderContainerService resolversPositionOrderContainerService
 
+    @Autowired
+    private ShibUIConfiguration shibUIConfiguration
+
     // TODO: enhance
     @Override
     void reloadFilters(String metadataResolverResourceId) {
@@ -64,6 +68,13 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
 
             List<MetadataFilter> metadataFilters = new ArrayList<>()
 
+            // set up namespace protection
+            if (shibUIConfiguration.protectedAttributeNamespaces && shibUIConfiguration.protectedAttributeNamespaces.size() > 0) {
+                def target = new org.opensaml.saml.metadata.resolver.filter.impl.EntityAttributesFilter()
+                target.attributeFilter = new ScriptedPredicate(new EvaluableScript(protectedNamespaceScript()))
+                metadataFilters.add(target)
+            }
+
             for (edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter metadataFilter : jpaMetadataResolver.getMetadataFilters()) {
                 if (metadataFilter instanceof EntityAttributesFilter) {
                     EntityAttributesFilter entityAttributesFilter = (EntityAttributesFilter) metadataFilter
@@ -104,6 +115,21 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
         }
     }
 
+    private String protectedNamespaceScript() {
+        return """(function (attribute) {
+                "use strict";
+                var namespaces = [${shibUIConfiguration.protectedAttributeNamespaces.collect({"\"${it}\""}).join(', ')}];
+                // check the parameter
+                if (attribute === null) { return true; }
+                for (var i in namespaces) {
+                    if (attribute.getName().startsWith(namespaces[i])) {
+                        return false;
+                    }
+                }
+                return true;
+            }(input));"""
+    }
+
     private class ScriptedPredicate extends net.shibboleth.utilities.java.support.logic.ScriptedPredicate<EntityDescriptor> {
         protected ScriptedPredicate(@Nonnull EvaluableScript theScript) {
             super(theScript)
@@ -133,9 +159,18 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
                         if ((mr.type != 'BaseMetadataResolver') && (mr.enabled)) {
                             constructXmlNodeForResolver(mr, delegate) {
                                 //TODO: enhance
+                                def didNamespaceProtectionFilter = !(shibUIConfiguration.protectedAttributeNamespaces && shibUIConfiguration.protectedAttributeNamespaces.size() > 0)
                                 mr.metadataFilters.each { edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter filter ->
+                                    if (filter instanceof EntityAttributesFilter && !didNamespaceProtectionFilter) {
+                                        constructXmlNodeForEntityAttributeNamespaceProtection(delegate)
+                                        didNamespaceProtectionFilter = true
+                                    }
                                     constructXmlNodeForFilter(filter, delegate)
                                 }
+                                if (!didNamespaceProtectionFilter) {
+                                    constructXmlNodeForEntityAttributeNamespaceProtection(delegate)
+                                    didNamespaceProtectionFilter = true
+                                }
                             }
                         }
                 }
@@ -144,6 +179,16 @@ class JPAMetadataResolverServiceImpl implements MetadataResolverService {
         }
     }
 
+    void constructXmlNodeForEntityAttributeNamespaceProtection(def markupBuilderDelegate) {
+        markupBuilderDelegate.MetadataFilter('xsi:type': 'EntityAttributes') {
+            AttributeFilterScript() {
+                Script() {
+                    mkp.yieldUnescaped("\n<![CDATA[\n${protectedNamespaceScript()}\n]]>\n")
+                }
+            }
+        }
+    }
+
     void constructXmlNodeForFilter(SignatureValidationFilter filter, def markupBuilderDelegate) {
         if(filter.xmlShouldBeGenerated()) {
             markupBuilderDelegate.MetadataFilter(id: filter.name,

diff --git a/...in/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java b/...in/java/edu/internet2/tier/shibboleth/admin/ui/configuration/CoreShibUiConfiguration.java
@@ -47,7 +47,7 @@
 import javax.servlet.http.HttpServletRequest;
 
 @Configuration
-@EnableConfigurationProperties(CustomPropertiesConfiguration.class)
+@EnableConfigurationProperties({CustomPropertiesConfiguration.class, ShibUIConfiguration.class})
 public class CoreShibUiConfiguration {
     private static final Logger logger = LoggerFactory.getLogger(CoreShibUiConfiguration.class);
 

diff --git a/...c/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/ShibUIConfiguration.java b/...c/main/java/edu/internet2/tier/shibboleth/admin/ui/configuration/ShibUIConfiguration.java
@@ -0,0 +1,20 @@
+package edu.internet2.tier.shibboleth.admin.ui.configuration;
+
+import lombok.Getter;
+import lombok.Setter;
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.context.annotation.Configuration;
+
+import java.util.List;
+
+@Configuration
+@ConfigurationProperties(prefix = "shibui")
+@Getter
+@Setter
+public class ShibUIConfiguration {
+    /**
+     * A list of namespaces that should be excluded from incoming metadata. This is used to prevent third party metadata
+     * sources from using attributes that they might not have the rights to use.
+     */
+    private List<String> protectedAttributeNamespaces;
+}
diff --git a/...edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy b/...edu/internet2/tier/shibboleth/admin/ui/service/JPAMetadataResolverServiceImplTests.groovy
@@ -5,6 +5,7 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.Internationalization
 import edu.internet2.tier.shibboleth.admin.ui.configuration.MetadataResolverConverterConfiguration
 import edu.internet2.tier.shibboleth.admin.ui.configuration.PlaceholderResolverComponentsConfiguration
 import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration
+import edu.internet2.tier.shibboleth.admin.ui.configuration.ShibUIConfiguration
 import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter
 import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilterTarget
 import edu.internet2.tier.shibboleth.admin.ui.domain.filters.RequiredValidUntilFilter
@@ -42,7 +43,14 @@ import org.springframework.test.annotation.DirtiesContext
 import org.springframework.test.context.ContextConfiguration
 import org.xmlunit.builder.DiffBuilder
 import org.xmlunit.builder.Input
+import spock.lang.Ignore
 import spock.lang.Specification
+import spock.lang.Unroll
+
+import javax.xml.transform.OutputKeys
+import javax.xml.transform.TransformerFactory
+import javax.xml.transform.dom.DOMSource
+import javax.xml.transform.stream.StreamResult
 
 import static edu.internet2.tier.shibboleth.admin.ui.util.TestHelpers.generatedXmlIsTheSameAsExpectedXml
 
@@ -74,6 +82,9 @@ class JPAMetadataResolverServiceImplTests extends Specification {
     @Autowired
     MetadataResolverConverterService mdrConverterService
 
+    @Autowired
+    ShibUIConfiguration shibUIConfiguration
+
     TestObjectGenerator testObjectGenerator
 
     DOMBuilder domBuilder
@@ -349,6 +360,51 @@ class JPAMetadataResolverServiceImplTests extends Specification {
         generatedXmlIsTheSameAsExpectedXml('/conf/704.3.xml', domBuilder.parseText(writer.toString()))
     }
 
+    @Unroll
+    @DirtiesContext(methodMode = DirtiesContext.MethodMode.AFTER_METHOD)
+    def 'test namespace protection [#namespaces]'() {
+        setup:
+        shibUIConfiguration.protectedAttributeNamespaces = namespaces
+        def resolver = new DynamicHttpMetadataResolver().with {
+            it.xmlId = 'DynamicHttpMetadataResolver'
+            it.metadataRequestURLConstructionScheme = new MetadataQueryProtocolScheme().with {
+                it.content = 'http://mdq-beta.incommon.org/global'
+                it
+            }
+            it
+        }
+        metadataResolverRepository.save(resolver)
+
+        expect:
+        generatedXmlIsTheSameAsExpectedXml(filename, metadataResolverService.generateConfiguration())
+
+        where:
+        namespaces | filename
+        ['http://shibboleth.net/ns/profiles'] | '/conf/984.xml'
+        ['http://shibboleth.net/ns/profiles', 'http://scaldingspoon.com/iam'] | '/conf/984-2.xml'
+    }
+
+    @Ignore('there is a bug in org.opensaml.saml.metadata.resolver.filter.impl.EntityAttributesFilter.applyFilter')
+    def 'test namespace protection internal filtering'() {
+        setup:
+        shibUIConfiguration.protectedAttributeNamespaces = ['http://shibboleth.net/ns/profiles']
+        def resolver = new edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.ResourceBackedMetadataResolver().with {
+            it.resourceId = 'testme984'
+            it.name = 'testme984'
+            it.classpathMetadataResource = new ClasspathMetadataResource('metadata/984-3.xml')
+            it
+        }
+        def x = metadataResolverRepository.save(resolver)
+        ((OpenSamlChainingMetadataResolver)metadataResolver).resolvers.add(mdrConverterService.convertToOpenSamlRepresentation(x))
+
+        when:
+        metadataResolverService.reloadFilters('testme984')
+        def ed = metadataResolver.resolveSingle(new CriteriaSet(new EntityIdCriterion('http://test.scaldingspoon.org/test1')))
+
+        then:
+        !DiffBuilder.compare(Input.fromStream(this.class.getResourceAsStream('/metadata/984-3-expected.xml'))).withTest(Input.fromString(openSamlObjects.marshalToXmlString(ed))).ignoreComments().ignoreWhitespace().build().hasDifferences()
+    }
+
     static genXmlSnippet(MarkupBuilder xml, Closure xmlNodeGenerator) {
         xml.MetadataProvider('id': 'ShibbolethMetadata',
                 'xmlns': 'urn:mace:shibboleth:2.0:metadata',

diff --git a/backend/src/test/resources/conf/984-2.xml b/backend/src/test/resources/conf/984-2.xml
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<MetadataProvider xmlns="urn:mace:shibboleth:2.0:metadata" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                  id="ShibbolethMetadata"
+                  xsi:schemaLocation="urn:mace:shibboleth:2.0:metadata http://shibboleth.net/schema/idp/shibboleth-metadata.xsd urn:mace:shibboleth:2.0:resource http://shibboleth.net/schema/idp/shibboleth-resource.xsd urn:mace:shibboleth:2.0:security http://shibboleth.net/schema/idp/shibboleth-security.xsd urn:oasis:names:tc:SAML:2.0:metadata http://docs.oasis-open.org/security/saml/v2.0/saml-schema-metadata-2.0.xsd urn:oasis:names:tc:SAML:2.0:assertion http://docs.oasis-open.org/security/saml/v2.0/saml-schema-assertion-2.0.xsd"
+                  xsi:type="ChainingMetadataProvider">
+    <MetadataProvider backgroundInitializationFromCacheDelay="PT2S" cleanupTaskInterval="PT30M"
+                      connectionRequestTimeout="PT5S" connectionTimeout="PT5S" id="DynamicHttpMetadataResolver"
+                      maxCacheDuration="PT8H" maxConnectionsPerRoute="100" maxConnectionsTotal="100"
+                      maxIdleEntityData="PT8H" minCacheDuration="PT10M" refreshDelayFactor="0.75"
+                      removeIdleEntityData="true" socketTimeout="PT5S" xsi:type="DynamicHttpMetadataProvider">
+        <MetadataQueryProtocol>http://mdq-beta.incommon.org/global</MetadataQueryProtocol>
+        <MetadataFilter xsi:type="EntityAttributes">
+            <AttributeFilterScript>
+                <Script>
+                    <![CDATA[
+            (function (attribute) {
+                "use strict";
+                var namespaces = ["http://shibboleth.net/ns/profiles", "http://scaldingspoon.com/iam"];
+                // check the parameter
+                if (attribute === null) { return true; }
+                for (var i in namespaces) {
+                    if (attribute.getName().startsWith(namespaces[i])) {
+                        return false;
+                    }
+                }
+                return true;
+            }(input));
+]]>
+                </Script>
+            </AttributeFilterScript>
+        </MetadataFilter>
+    </MetadataProvider>
+</MetadataProvider>
diff --git a/backend/src/test/resources/conf/984.xml b/backend/src/test/resources/conf/984.xml
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<MetadataProvider xmlns="urn:mace:shibboleth:2.0:metadata" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                  id="ShibbolethMetadata"
+                  xsi:schemaLocation="urn:mace:shibboleth:2.0:metadata http://shibboleth.net/schema/idp/shibboleth-metadata.xsd urn:mace:shibboleth:2.0:resource http://shibboleth.net/schema/idp/shibboleth-resource.xsd urn:mace:shibboleth:2.0:security http://shibboleth.net/schema/idp/shibboleth-security.xsd urn:oasis:names:tc:SAML:2.0:metadata http://docs.oasis-open.org/security/saml/v2.0/saml-schema-metadata-2.0.xsd urn:oasis:names:tc:SAML:2.0:assertion http://docs.oasis-open.org/security/saml/v2.0/saml-schema-assertion-2.0.xsd"
+                  xsi:type="ChainingMetadataProvider">
+    <MetadataProvider backgroundInitializationFromCacheDelay="PT2S" cleanupTaskInterval="PT30M"
+                      connectionRequestTimeout="PT5S" connectionTimeout="PT5S" id="DynamicHttpMetadataResolver"
+                      maxCacheDuration="PT8H" maxConnectionsPerRoute="100" maxConnectionsTotal="100"
+                      maxIdleEntityData="PT8H" minCacheDuration="PT10M" refreshDelayFactor="0.75"
+                      removeIdleEntityData="true" socketTimeout="PT5S" xsi:type="DynamicHttpMetadataProvider">
+        <MetadataQueryProtocol>http://mdq-beta.incommon.org/global</MetadataQueryProtocol>
+        <MetadataFilter xsi:type="EntityAttributes">
+            <AttributeFilterScript>
+                <Script>
+                    <![CDATA[
+            (function (attribute) {
+                "use strict";
+                var namespaces = ["http://shibboleth.net/ns/profiles"];
+                // check the parameter
+                if (attribute === null) { return true; }
+                for (var i in namespaces) {
+                    if (attribute.getName().startsWith(namespaces[i])) {
+                        return false;
+                    }
+                }
+                return true;
+            }(input));
+]]>
+                </Script>
+            </AttributeFilterScript>
+        </MetadataFilter>
+    </MetadataProvider>
+</MetadataProvider>
diff --git a/backend/src/test/resources/metadata/984-3-expected.xml b/backend/src/test/resources/metadata/984-3-expected.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0"?>
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
+                     xmlns:mdattr="urn:oasis:names:tc:SAML:metadata:attribute"
+                     xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
+                     entityID="http://test.scaldingspoon.org/test1">
+    <md:Extensions>
+        <mdattr:EntityAttributes>
+            <saml:Attribute Name="http://scaldingspoon.org/realm"
+                            NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
+                <saml:AttributeValue>internal</saml:AttributeValue>
+            </saml:Attribute>
+        </mdattr:EntityAttributes>
+    </md:Extensions>
+    <md:SPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+        <md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat>
+        <md:AssertionConsumerService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
+                                     Location="https://test.scaldingspoon.org/test1/acs"
+                                     index="1"/>
+    </md:SPSSODescriptor>
+</md:EntityDescriptor>
diff --git a/backend/src/test/resources/metadata/984-3.xml b/backend/src/test/resources/metadata/984-3.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0"?>
+<md:EntityDescriptor xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"
+                     xmlns:mdattr="urn:oasis:names:tc:SAML:metadata:attribute"
+                     xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
+                     entityID="http://test.scaldingspoon.org/test1">
+    <md:Extensions>
+        <mdattr:EntityAttributes>
+            <saml:Attribute Name="http://scaldingspoon.org/realm" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
+                <saml:AttributeValue>internal</saml:AttributeValue>
+            </saml:Attribute>
+            <saml:Attribute Name="http://shibboleth.net/ns/attributes/releaseAllValues" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
+                <saml:AttributeValue>givenName</saml:AttributeValue>
+                <saml:AttributeValue>employeeNumber</saml:AttributeValue>
+            </saml:Attribute>
+        </mdattr:EntityAttributes>
+    </md:Extensions>
+    <md:SPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+        <md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat>
+        <md:AssertionConsumerService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
+                                     Location="https://test.scaldingspoon.org/test1/acs"
+                                     index="1" />
+    </md:SPSSODescriptor>
+</md:EntityDescriptor>