[java-shib-shared] branch main updated: JSSH-20 - Spring is still falling through to remote access of XML files

Scott Cantor cantor.2 at osu.edu
Thu Dec 8 16:53:15 UTC 2022


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

scantor pushed a commit to branch main
in repository java-shib-shared.

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

The following commit(s) were added to refs/heads/main by this push:
     new dd12a685 JSSH-20 - Spring is still falling through to remote access of XML files
dd12a685 is described below

commit dd12a685c3ea6da1030ea757db269dab62dd76e8
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Dec 8 11:53:12 2022 -0500

    JSSH-20 - Spring is still falling through to remote access of XML files
    
    https://shibboleth.atlassian.net/browse/JSSH-20
    
    Includes a canary test.
---
 .../context/DelimiterAwareApplicationContext.java  |   4 +-
 .../custom/LocalOnlyResourceEntityResolver.java    | 127 +++++++++++++++++++++
 .../SchemaTypeAwareXMLBeanDefinitionReader.java    |  12 ++
 .../shared/spring/custom/CanaryParser.java         |  51 +++++++++
 .../shared/spring/custom/CanarySchemaTest.java     |  50 ++++++++
 .../src/test/resources/META-INF/spring.handlers    |   3 +-
 .../src/test/resources/META-INF/spring.schemas     |   3 +-
 .../net/shibboleth/shared/spring/custom/canary.xml |   4 +
 shib-spring/src/test/resources/schema/canary.xsd   |  18 +++
 9 files changed, 268 insertions(+), 4 deletions(-)

diff --git a/shib-spring/src/main/java/net/shibboleth/shared/spring/context/DelimiterAwareApplicationContext.java b/shib-spring/src/main/java/net/shibboleth/shared/spring/context/DelimiterAwareApplicationContext.java
index 15bf39d7..d44e7258 100644
--- a/shib-spring/src/main/java/net/shibboleth/shared/spring/context/DelimiterAwareApplicationContext.java
+++ b/shib-spring/src/main/java/net/shibboleth/shared/spring/context/DelimiterAwareApplicationContext.java
@@ -23,10 +23,10 @@ import javax.annotation.Nonnull;
 
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.support.DefaultListableBeanFactory;
-import org.springframework.beans.factory.xml.ResourceEntityResolver;
 import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
 import org.springframework.util.StringUtils;
 
+import net.shibboleth.shared.spring.custom.LocalOnlyResourceEntityResolver;
 import net.shibboleth.shared.spring.custom.SchemaTypeAwareXMLBeanDefinitionReader;
 
 /**
@@ -67,7 +67,7 @@ public class DelimiterAwareApplicationContext extends DeferPlaceholderFileSystem
         // resource loading environment.
         beanDefinitionReader.setEnvironment(getEnvironment());
         beanDefinitionReader.setResourceLoader(this);
-        beanDefinitionReader.setEntityResolver(new ResourceEntityResolver(this));
+        beanDefinitionReader.setEntityResolver(new LocalOnlyResourceEntityResolver(this));
 
         // Allow a subclass to provide custom initialization of the reader,
         // then proceed with actually loading the bean definitions.
diff --git a/shib-spring/src/main/java/net/shibboleth/shared/spring/custom/LocalOnlyResourceEntityResolver.java b/shib-spring/src/main/java/net/shibboleth/shared/spring/custom/LocalOnlyResourceEntityResolver.java
new file mode 100644
index 00000000..0128f8c4
--- /dev/null
+++ b/shib-spring/src/main/java/net/shibboleth/shared/spring/custom/LocalOnlyResourceEntityResolver.java
@@ -0,0 +1,127 @@
+/*
+ * 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.shared.spring.custom;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.xml.DelegatingEntityResolver;
+import org.springframework.beans.factory.xml.ResourceEntityResolver;
+import org.springframework.core.io.Resource;
+import org.springframework.core.io.ResourceLoader;
+import org.springframework.util.ResourceUtils;
+
+/**
+ * Modified copy of Spring's existing {@link ResourceEntityResolver} class that
+ * elides the fall-through logic allowing for http(s) resolution of entities.
+ */
+public class LocalOnlyResourceEntityResolver extends DelegatingEntityResolver {
+
+	@Nonnull private final Logger log = LoggerFactory.getLogger(LocalOnlyResourceEntityResolver.class);
+
+	@Nonnull private final ResourceLoader resourceLoader;
+
+	/**
+	 * Create a ResourceEntityResolver for the specified ResourceLoader
+	 * (usually, an ApplicationContext).
+	 * 
+	 * @param loader the ResourceLoader (or ApplicationContext)
+	 * to load XML entity includes with
+	 */
+	public LocalOnlyResourceEntityResolver(@Nonnull final ResourceLoader loader) {
+		super(loader.getClassLoader());
+		resourceLoader = loader;
+	}
+
+	/** {@inheritDoc} */
+	@Override
+	@Nullable public InputSource resolveEntity(@Nullable final String publicId, @Nullable final String systemId)
+			throws SAXException, IOException {
+
+		InputSource source = super.resolveEntity(publicId, systemId);
+
+		if (source == null && systemId != null) {
+			String resourcePath = null;
+			try {
+				String decodedSystemId = URLDecoder.decode(systemId, StandardCharsets.UTF_8);
+				assert decodedSystemId != null;
+				String givenUrl = ResourceUtils.toURL(decodedSystemId).toString();
+				String systemRootUrl = new File("").toURI().toURL().toString();
+				// Try relative to resource base if currently in system root.
+				if (givenUrl.startsWith(systemRootUrl)) {
+					resourcePath = givenUrl.substring(systemRootUrl.length());
+				}
+			}
+			catch (Exception ex) {
+				// Typically a MalformedURLException or AccessControlException.
+				log.debug("Could not resolve XML entity [{}] against system root URL", systemId, ex);
+				// No URL (or no resolvable URL) -> try relative to resource base.
+				resourcePath = systemId;
+			}
+			if (resourcePath != null) {
+				log.trace("Trying to locate XML entity [{}] as resource [{}]", systemId, resourcePath);
+				Resource resource = this.resourceLoader.getResource(resourcePath);
+				source = new InputSource(resource.getInputStream());
+				source.setPublicId(publicId);
+				source.setSystemId(systemId);
+				log.debug("Found XML entity [{}]:", systemId, resource);
+			}
+			else if (systemId.endsWith(DTD_SUFFIX) || systemId.endsWith(XSD_SUFFIX)) {
+				// External dtd/xsd lookup via https even for canonical http declaration
+				String url = systemId;
+				if (url.startsWith("http:")) {
+					url = "https:" + url.substring(5);
+				}
+				
+				log.warn("Blocking attempted remote resolution of [{}]", systemId);
+				// If we don't throw here, Java's broken parser just blindly proceeds with its own
+				// internal entity resolution.
+				throw new IOException("Blocked atttempted remote resolution");
+
+                // This is being elided.
+                
+				/*
+				try {
+					source = new InputSource(ResourceUtils.toURL(url).openStream());
+					source.setPublicId(publicId);
+					source.setSystemId(systemId);
+				}
+				catch (IOException ex) {
+					if (logger.isDebugEnabled()) {
+						logger.debug("Could not resolve XML entity [" + systemId + "] through URL [" + url + "]", ex);
+					}
+					// Fall back to the parser's default behavior.
+					source = null;
+				}
+				*/
+			}
+		}
+
+		return source;
+	}
+
+}
\ No newline at end of file
diff --git a/shib-spring/src/main/java/net/shibboleth/shared/spring/custom/SchemaTypeAwareXMLBeanDefinitionReader.java b/shib-spring/src/main/java/net/shibboleth/shared/spring/custom/SchemaTypeAwareXMLBeanDefinitionReader.java
index 2238da10..4125e261 100644
--- a/shib-spring/src/main/java/net/shibboleth/shared/spring/custom/SchemaTypeAwareXMLBeanDefinitionReader.java
+++ b/shib-spring/src/main/java/net/shibboleth/shared/spring/custom/SchemaTypeAwareXMLBeanDefinitionReader.java
@@ -18,7 +18,9 @@
 package net.shibboleth.shared.spring.custom;
 
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
+import org.springframework.beans.factory.xml.DelegatingEntityResolver;
 import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
+import org.springframework.core.io.ResourceLoader;
 
 /**
  * An extension to the standard {@link XmlBeanDefinitionReader} that defaults some settings.
@@ -41,5 +43,15 @@ public class SchemaTypeAwareXMLBeanDefinitionReader extends XmlBeanDefinitionRea
         setDocumentReaderClass(SchemaTypeAwareBeanDefinitionDocumentReader.class);
 
         setValidationMode(VALIDATION_XSD);
+
+        // This installs the appropriate XML EntityResolver with our version if needed.
+        final ResourceLoader resourceLoader = getResourceLoader();
+        if (resourceLoader != null) {
+            setEntityResolver(new LocalOnlyResourceEntityResolver(resourceLoader));
+        }
+        else {
+            setEntityResolver(new DelegatingEntityResolver(getBeanClassLoader()));
+        }
     }
+
 }
\ No newline at end of file
diff --git a/shib-spring/src/test/java/net/shibboleth/shared/spring/custom/CanaryParser.java b/shib-spring/src/test/java/net/shibboleth/shared/spring/custom/CanaryParser.java
new file mode 100644
index 00000000..677f978a
--- /dev/null
+++ b/shib-spring/src/test/java/net/shibboleth/shared/spring/custom/CanaryParser.java
@@ -0,0 +1,51 @@
+/*
+ * 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.shared.spring.custom;
+
+import javax.annotation.Nonnull;
+import javax.xml.namespace.QName;
+
+import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.xml.BeanDefinitionParser;
+import org.springframework.beans.factory.xml.ParserContext;
+import org.w3c.dom.Element;
+
+import net.shibboleth.shared.annotation.constraint.NotEmpty;
+
+/**
+ * Custom namespace parser for JSSH-20 test canary.
+ */
+public class CanaryParser extends BaseSpringNamespaceHandler {
+    
+    @Nonnull @NotEmpty protected static final String NAMESPACE = "urn:mace:shibboleth:2.0:canary";
+    
+    /** {@inheritDoc} */
+    @Override
+    public void doInit() {
+        registerBeanDefinitionParser(new QName(NAMESPACE, "OurElement"), new OurElementParser());
+    }
+
+    static class OurElementParser implements BeanDefinitionParser {
+        
+        /** {@inheritDoc} */
+        public BeanDefinition parse(@Nonnull final Element config, @Nonnull final ParserContext parserContext) {
+            return null;
+        }
+    }
+    
+}
\ No newline at end of file
diff --git a/shib-spring/src/test/java/net/shibboleth/shared/spring/custom/CanarySchemaTest.java b/shib-spring/src/test/java/net/shibboleth/shared/spring/custom/CanarySchemaTest.java
new file mode 100644
index 00000000..7bd291a9
--- /dev/null
+++ b/shib-spring/src/test/java/net/shibboleth/shared/spring/custom/CanarySchemaTest.java
@@ -0,0 +1,50 @@
+/*
+ * 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.shared.spring.custom;
+
+import org.springframework.beans.factory.xml.XmlBeanDefinitionStoreException;
+import org.springframework.context.support.GenericApplicationContext;
+import org.springframework.core.io.ClassPathResource;
+import org.testng.annotations.Test;
+
+/**
+ * Test for JSSH-20, remote entity access by Spring.
+ */
+public class CanarySchemaTest {
+
+    @Test(expectedExceptions=XmlBeanDefinitionStoreException.class)
+    void Test() {
+        final GenericApplicationContext context = new GenericApplicationContext();
+        context.setDisplayName("ApplicationContext for Canary");
+
+        final SchemaTypeAwareXMLBeanDefinitionReader beanDefinitionReader =
+                new SchemaTypeAwareXMLBeanDefinitionReader(context);
+
+        // This should throw XmlBeanDefinitionStoreException due to the underlying attempt to resolve the AFP schema
+        // as an import. If the bug still existed or manifests differently, the schema will be fetched directly from
+        // shibboleth.net and the import will work.
+        
+        // Note that transitory issues with shibboleth.net should be ok here. While they would mask things such that the
+        // bug might exist again but the test "fail" due to shibboleth.net being down, that shouldn't persist long
+        // enough and we'd catch it eventually "working".
+        
+        beanDefinitionReader.loadBeanDefinitions(new ClassPathResource("/net/shibboleth/shared/spring/custom/canary.xml"));
+        context.refresh();
+    }
+    
+}
\ No newline at end of file
diff --git a/shib-spring/src/test/resources/META-INF/spring.handlers b/shib-spring/src/test/resources/META-INF/spring.handlers
index 5152d084..25b08f8e 100644
--- a/shib-spring/src/test/resources/META-INF/spring.handlers
+++ b/shib-spring/src/test/resources/META-INF/spring.handlers
@@ -1,2 +1,3 @@
 urn\:mace\:shibboleth\:2.0\:naturestudy = net.shibboleth.ext.spring.naturestudy.NamespaceHandler
-urn\:mace\:shibboleth\:2.0\:nested = net.shibboleth.shared.spring.custom.LowerParsersAndBean
\ No newline at end of file
+urn\:mace\:shibboleth\:2.0\:nested = net.shibboleth.shared.spring.custom.LowerParsersAndBean
+urn\:mace\:shibboleth\:2.0\:canary = net.shibboleth.shared.spring.custom.CanaryParser
\ No newline at end of file
diff --git a/shib-spring/src/test/resources/META-INF/spring.schemas b/shib-spring/src/test/resources/META-INF/spring.schemas
index 76ed0cc5..b7462681 100644
--- a/shib-spring/src/test/resources/META-INF/spring.schemas
+++ b/shib-spring/src/test/resources/META-INF/spring.schemas
@@ -1,3 +1,4 @@
 urn\:mace\:shibboleth\:2.0\:naturestudy = schema/nature-study.xsd
 urn\:mace\:shibboleth\:2.0\:nested = schema/nested.xsd
-http\://shibboleth.net/schema/nested.xsd = schema/nested.xsd
\ No newline at end of file
+http\://shibboleth.net/schema/nested.xsd = schema/nested.xsd
+http\://shibboleth.net/schema/canary.xsd = schema/canary.xsd
\ No newline at end of file
diff --git a/shib-spring/src/test/resources/net/shibboleth/shared/spring/custom/canary.xml b/shib-spring/src/test/resources/net/shibboleth/shared/spring/custom/canary.xml
new file mode 100644
index 00000000..f06fdca0
--- /dev/null
+++ b/shib-spring/src/test/resources/net/shibboleth/shared/spring/custom/canary.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<OurElement xmlns="urn:mace:shibboleth:2.0:canary" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
+   xsi:schemaLocation="urn:mace:shibboleth:2.0:canary http://shibboleth.net/schema/canary.xsd"
+/>
diff --git a/shib-spring/src/test/resources/schema/canary.xsd b/shib-spring/src/test/resources/schema/canary.xsd
new file mode 100644
index 00000000..64b33170
--- /dev/null
+++ b/shib-spring/src/test/resources/schema/canary.xsd
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<schema xmlns="http://www.w3.org/2001/XMLSchema" 
+        xmlns:ns="urn:mace:shibboleth:2.0:canary"
+        xmlns:afp="urn:mace:shibboleth:2.0:afp"
+        targetNamespace="urn:mace:shibboleth:2.0:canary"
+        elementFormDefault="qualified">
+
+    <import namespace="urn:mace:shibboleth:2.0:afp" schemaLocation="http://shibboleth.net/schema/idp/shibboleth-afp.xsd"/>
+
+    <element name="OurElement">
+        <complexType>
+            <sequence>
+                <element ref="afp:AttributeFilterPolicy" minOccurs="0" />
+            </sequence>
+        </complexType>
+    </element>
+    
+</schema>

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


More information about the commits mailing list