replace DocumentBuilder in XmlTextStructureFinder

Author: richard-dennehy

libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java

@@ -32,12 +32,26 @@ public class XmlUtils {
      *
      * @throws ParserConfigurationException if one of the features can't be set on the DocumentBuilderFactory
      */
-    @SuppressForbidden(reason = "This is the only allowed way to construct a DocumentBuilder")
     public static DocumentBuilder getHardenedBuilder(String[] schemaFiles) throws ParserConfigurationException {
-        final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+        final DocumentBuilderFactory dbf = getHardenedBuilderFactory();
         dbf.setNamespaceAware(true);
         // Ensure that Schema Validation is enabled for the factory
         dbf.setValidating(true);
+        // This is required, otherwise schema validation causes signature invalidation
+        dbf.setFeature("http://apache.org/xml/features/validation/schema/normalized-value", false);
+        // Make sure that URL schema namespaces are not resolved/downloaded from URLs we do not control
+        dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "file,jar");
+        dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar");
+        // We ship our own xsd files for schema validation since we do not trust anyone else.
+        dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaSource", schemaFiles);
+        DocumentBuilder documentBuilder = dbf.newDocumentBuilder();
+        documentBuilder.setErrorHandler(new ErrorHandler());
+        return documentBuilder;
+    }
+
+    @SuppressForbidden(reason = "This is the only allowed way to construct a DocumentBuilder")
+    public static DocumentBuilderFactory getHardenedBuilderFactory() throws ParserConfigurationException {
+        final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
         // Disallow internal and external entity expansion
         dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
         dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
@@ -46,11 +60,8 @@ public static DocumentBuilder getHardenedBuilder(String[] schemaFiles) throws Pa
         dbf.setFeature("http://xml.org/sax/features/validation", true);
         dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
         dbf.setIgnoringComments(true);
-        // This is required, otherwise schema validation causes signature invalidation
-        dbf.setFeature("http://apache.org/xml/features/validation/schema/normalized-value", false);
-        // Make sure that URL schema namespaces are not resolved/downloaded from URLs we do not control
-        dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "file,jar");
-        dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar");
+        dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+        dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
         dbf.setFeature("http://apache.org/xml/features/honour-all-schemaLocations", true);
         // Ensure we do not resolve XIncludes. Defaults to false, but set it explicitly to be future-proof
         dbf.setXIncludeAware(false);
@@ -61,11 +72,8 @@ public static DocumentBuilder getHardenedBuilder(String[] schemaFiles) throws Pa
         dbf.setAttribute("http://apache.org/xml/features/validation/schema", true);
         dbf.setAttribute("http://apache.org/xml/features/validation/schema-full-checking", true);
         dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaLanguage", XMLConstants.W3C_XML_SCHEMA_NS_URI);
-        // We ship our own xsd files for schema validation since we do not trust anyone else.
-        dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaSource", schemaFiles);
-        DocumentBuilder documentBuilder = dbf.newDocumentBuilder();
-        documentBuilder.setErrorHandler(new ErrorHandler());
-        return documentBuilder;
+
+        return dbf;
     }
 
     @SuppressForbidden(reason = "This is the only allowed way to construct a Transformer")

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java

@@ -14,6 +14,7 @@
 import org.elasticsearch.common.util.NamedFormatter;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.core.Tuple;
+import org.elasticsearch.core.XmlUtils;
 import org.elasticsearch.test.MockLog;
 import org.elasticsearch.xpack.core.watcher.watch.ClockMock;
 import org.hamcrest.Matchers;
@@ -1501,7 +1502,7 @@ private Encrypter getEncrypter(Tuple<X509Certificate, PrivateKey> keyPair) throw
     }
 
     private Response toResponse(String xml) throws SAXException, IOException, ParserConfigurationException {
-        final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+        final DocumentBuilderFactory dbf = XmlUtils.getHardenedBuilderFactory();
         dbf.setNamespaceAware(true);
         final Document doc = dbf.newDocumentBuilder().parse(new InputSource(new StringReader(xml)));
         return authenticator.buildXmlObject(doc.getDocumentElement(), Response.class);

x-pack/plugin/text-structure/src/main/java/org/elasticsearch/xpack/textstructure/structurefinder/XmlTextStructureFinder.java

@@ -7,6 +7,7 @@
 package org.elasticsearch.xpack.textstructure.structurefinder;
 
 import org.elasticsearch.core.Tuple;
+import org.elasticsearch.core.XmlUtils;
 import org.elasticsearch.xpack.core.textstructure.structurefinder.FieldStats;
 import org.elasticsearch.xpack.core.textstructure.structurefinder.TextStructure;
 import org.w3c.dom.Document;
@@ -29,7 +30,6 @@
 import java.util.TreeMap;
 import java.util.regex.Pattern;
 
-import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -152,21 +152,9 @@ static XmlTextStructureFinder makeXmlTextStructureFinder(
     }
 
     private static DocumentBuilderFactory makeDocBuilderFactory() throws ParserConfigurationException {
-
-        DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
+        DocumentBuilderFactory docBuilderFactory = XmlUtils.getHardenedBuilderFactory();
         docBuilderFactory.setNamespaceAware(false);
         docBuilderFactory.setValidating(false);
-        docBuilderFactory.setXIncludeAware(false);
-        docBuilderFactory.setExpandEntityReferences(false);
-        docBuilderFactory.setIgnoringComments(true);
-        docBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-        // The next 5 should be irrelevant given the previous 1, but it doesn't hurt to set them just in case
-        docBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
-        docBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
-        docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
-        docBuilderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-        docBuilderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
         return docBuilderFactory;
     }