[spring-extensions] branch maint-6 updated: JSE-52 - Spring is still falling through to remote access of XML files

Scott Cantor cantor.2 at osu.edu
Thu Dec 8 18:45:52 UTC 2022


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

scantor pushed a commit to branch maint-6
in repository spring-extensions.

View the commit online:
http://git.shibboleth.net/view/?p=spring-extensions.git;a=commit;h=bf84df26887132d30dbc93c847e1b71529032e3d

The following commit(s) were added to refs/heads/maint-6 by this push:
     new bf84df2  JSE-52 - Spring is still falling through to remote access of XML files
bf84df2 is described below

commit bf84df26887132d30dbc93c847e1b71529032e3d
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Dec 8 13:45:49 2022 -0500

    JSE-52 - Spring is still falling through to remote access of XML files
    
    https://shibboleth.atlassian.net/browse/JSE-52
    
    Backported JSSH-20 patch.
---
 .../util/LocalOnlyResourceEntityResolver.java      | 127 +++++++++++++++++++++
 .../SchemaTypeAwareXMLBeanDefinitionReader.java    |  12 ++
 .../shibboleth/ext/spring/util/CanaryParser.java   |  54 +++++++++
 .../ext/spring/util/CanarySchemaTest.java          |  50 ++++++++
 src/test/resources/META-INF/spring.handlers        |   1 +
 src/test/resources/META-INF/spring.schemas         |   2 +
 .../net/shibboleth/ext/spring/util/canary.xml      |   4 +
 src/test/resources/schema/canary.xsd               |  18 +++
 8 files changed, 268 insertions(+)

diff --git a/src/main/java/net/shibboleth/ext/spring/util/LocalOnlyResourceEntityResolver.java b/src/main/java/net/shibboleth/ext/spring/util/LocalOnlyResourceEntityResolver.java
new file mode 100644
index 0000000..05032dd
--- /dev/null
+++ b/src/main/java/net/shibboleth/ext/spring/util/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.ext.spring.util;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+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;
+
+/**
+ * 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 = new URL(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/src/main/java/net/shibboleth/ext/spring/util/SchemaTypeAwareXMLBeanDefinitionReader.java b/src/main/java/net/shibboleth/ext/spring/util/SchemaTypeAwareXMLBeanDefinitionReader.java
index 2b973b0..af1c006 100644
--- a/src/main/java/net/shibboleth/ext/spring/util/SchemaTypeAwareXMLBeanDefinitionReader.java
+++ b/src/main/java/net/shibboleth/ext/spring/util/SchemaTypeAwareXMLBeanDefinitionReader.java
@@ -18,7 +18,9 @@
 package net.shibboleth.ext.spring.util;
 
 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/src/test/java/net/shibboleth/ext/spring/util/CanaryParser.java b/src/test/java/net/shibboleth/ext/spring/util/CanaryParser.java
new file mode 100644
index 0000000..2de35e9
--- /dev/null
+++ b/src/test/java/net/shibboleth/ext/spring/util/CanaryParser.java
@@ -0,0 +1,54 @@
+/*
+ * 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.ext.spring.util;
+
+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.utilities.java.support.annotation.constraint.NotEmpty;
+
+/**
+ * Custom namespace parser for JSSH-20 test canary.
+ */
+public class CanaryParser extends BaseSpringNamespaceHandler {
+    
+    /**
+     * Test namespace.
+     */
+    @Nonnull @NotEmpty protected static final String NAMESPACE = "urn:mace:shibboleth:2.0:canary";
+    
+    /** {@inheritDoc} */
+    @Override
+    public void init() {
+        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/src/test/java/net/shibboleth/ext/spring/util/CanarySchemaTest.java b/src/test/java/net/shibboleth/ext/spring/util/CanarySchemaTest.java
new file mode 100644
index 0000000..df22509
--- /dev/null
+++ b/src/test/java/net/shibboleth/ext/spring/util/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.ext.spring.util;
+
+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/ext/spring/util/canary.xml"));
+        context.refresh();
+    }
+    
+}
\ No newline at end of file
diff --git a/src/test/resources/META-INF/spring.handlers b/src/test/resources/META-INF/spring.handlers
index b647db9..60f3118 100644
--- a/src/test/resources/META-INF/spring.handlers
+++ b/src/test/resources/META-INF/spring.handlers
@@ -1 +1,2 @@
 urn\:mace\:shibboleth\:2.0\:naturestudy = net.shibboleth.ext.spring.naturestudy.NamespaceHandler
+urn\:mace\:shibboleth\:2.0\:canary = net.shibboleth.ext.spring.util.CanaryParser
\ No newline at end of file
diff --git a/src/test/resources/META-INF/spring.schemas b/src/test/resources/META-INF/spring.schemas
index 4a15e24..41a38b8 100644
--- a/src/test/resources/META-INF/spring.schemas
+++ b/src/test/resources/META-INF/spring.schemas
@@ -1 +1,3 @@
 urn\:mace\:shibboleth\:2.0\:naturestudy = schema/nature-study.xsd
+
+http\://shibboleth.net/schema/canary.xsd = schema/canary.xsd
\ No newline at end of file
diff --git a/src/test/resources/net/shibboleth/ext/spring/util/canary.xml b/src/test/resources/net/shibboleth/ext/spring/util/canary.xml
new file mode 100644
index 0000000..f06fdca
--- /dev/null
+++ b/src/test/resources/net/shibboleth/ext/spring/util/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/src/test/resources/schema/canary.xsd b/src/test/resources/schema/canary.xsd
new file mode 100644
index 0000000..64b3317
--- /dev/null
+++ b/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