replace uses of DocumentBuilderFactory

Author: richard-dennehy

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

@@ -20,6 +20,7 @@
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerException;
@@ -121,6 +122,20 @@ public static Validator getHardenedValidator(Schema schema) throws SAXNotSupport
         return validator;
     }
 
+    @SuppressForbidden(reason = "This is the only allowed way to construct a SAXParser")
+    public static SAXParserFactory getHardenedSaxParserFactory() throws SAXNotSupportedException, SAXNotRecognizedException,
+        ParserConfigurationException {
+        var saxParserFactory = SAXParserFactory.newInstance();
+
+        saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        saxParserFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        saxParserFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        saxParserFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        saxParserFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+
+        return saxParserFactory;
+    }
+
     private static class ErrorHandler implements org.xml.sax.ErrorHandler {
         /**
          * Enabling schema validation with `setValidating(true)` in our

test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java

@@ -22,6 +22,7 @@
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.SuppressForbidden;
 import org.elasticsearch.core.Tuple;
+import org.elasticsearch.core.XmlUtils;
 import org.elasticsearch.logging.LogManager;
 import org.elasticsearch.logging.Logger;
 import org.elasticsearch.rest.RestStatus;
@@ -471,7 +472,7 @@ private static Tuple<String, BytesReference> parseRequestBody(final HttpExchange
 
     static List<String> extractPartEtags(BytesReference completeMultipartUploadBody) {
         try {
-            final var document = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(completeMultipartUploadBody.streamInput());
+            final var document = XmlUtils.getHardenedBuilderFactory().newDocumentBuilder().parse(completeMultipartUploadBody.streamInput());
             final var parts = document.getElementsByTagName("Part");
             final var result = new ArrayList<String>(parts.getLength());
             for (int partIndex = 0; partIndex < parts.getLength(); partIndex++) {

x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Streams;
+import org.elasticsearch.core.XmlUtils;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.rest.RestUtils;
 import org.elasticsearch.xpack.idp.action.SamlValidateAuthnRequestResponse;

x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/SamlFactory.java

@@ -12,6 +12,7 @@
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.hash.MessageDigests;
 import org.elasticsearch.core.SuppressForbidden;
+import org.elasticsearch.core.XmlUtils;
 import org.opensaml.core.xml.XMLObject;
 import org.opensaml.core.xml.XMLObjectBuilderFactory;
 import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
@@ -24,8 +25,6 @@
 import org.opensaml.security.credential.Credential;
 import org.opensaml.security.x509.X509Credential;
 import org.w3c.dom.Element;
-import org.xml.sax.SAXException;
-import org.xml.sax.SAXParseException;
 
 import java.io.StringWriter;
 import java.io.Writer;
@@ -38,16 +37,12 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 
-import javax.xml.XMLConstants;
 import javax.xml.namespace.QName;
 import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Transformer;
-import javax.xml.transform.TransformerConfigurationException;
 import javax.xml.transform.TransformerException;
-import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.stream.StreamResult;
 
@@ -143,7 +138,7 @@ public static <T extends XMLObject> T buildXmlObject(Element element, Class<T> t
     }
 
     void print(Element element, Writer writer, boolean pretty) throws TransformerException {
-        final Transformer serializer = getHardenedXMLTransformer();
+        final Transformer serializer = XmlUtils.getHardenedXMLTransformer();
         if (pretty) {
             serializer.setOutputProperty(OutputKeys.INDENT, "yes");
         }
@@ -217,57 +212,14 @@ public static Element toDomElement(XMLObject object) {
         }
     }
 
-    @SuppressForbidden(reason = "This is the only allowed way to construct a Transformer")
-    public static Transformer getHardenedXMLTransformer() throws TransformerConfigurationException {
-        final TransformerFactory tfactory = TransformerFactory.newInstance();
-        tfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
-        tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
-        tfactory.setAttribute("indent-number", 2);
-        Transformer transformer = tfactory.newTransformer();
-        transformer.setErrorListener(new SamlFactory.TransformerErrorListener());
-        return transformer;
-    }
-
     /**
      * Constructs a DocumentBuilder with all the necessary features for it to be secure
      *
      * @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();
-        dbf.setNamespaceAware(true);
-        // Ensure that Schema Validation is enabled for the factory
-        dbf.setValidating(true);
-        // 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);
-        dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
-        dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
-        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.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);
-        // Ensure we do not expand entity reference nodes
-        dbf.setExpandEntityReferences(false);
-        // Further limit danger from denial of service attacks
-        dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
-        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", resolveSchemaFilePaths(schemaFiles));
-        DocumentBuilder documentBuilder = dbf.newDocumentBuilder();
-        documentBuilder.setErrorHandler(new SamlFactory.DocumentBuilderErrorHandler());
-        return documentBuilder;
+        return XmlUtils.getHardenedBuilder(resolveSchemaFilePaths(schemaFiles));
     }
 
     public static String getJavaAlorithmNameFromUri(String sigAlg) {
@@ -293,31 +245,6 @@ private static String[] resolveSchemaFilePaths(String[] relativePaths) {
         }).filter(Objects::nonNull).toArray(String[]::new);
     }
 
-    private static class DocumentBuilderErrorHandler implements org.xml.sax.ErrorHandler {
-        /**
-         * Enabling schema validation with `setValidating(true)` in our
-         * DocumentBuilderFactory requires that we provide our own
-         * ErrorHandler implementation
-         *
-         * @throws SAXException If the document we attempt to parse is not valid according to the specified schema.
-         */
-        @Override
-        public void warning(SAXParseException e) throws SAXException {
-            LOGGER.debug("XML Parser error ", e);
-            throw e;
-        }
-
-        @Override
-        public void error(SAXParseException e) throws SAXException {
-            warning(e);
-        }
-
-        @Override
-        public void fatalError(SAXParseException e) throws SAXException {
-            warning(e);
-        }
-    }
-
     private static class TransformerErrorListener implements javax.xml.transform.ErrorListener {
 
         @Override

x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/saml/test/IdpSamlTestCase.java

@@ -11,6 +11,7 @@
 import org.apache.logging.log4j.Logger;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.common.ssl.PemUtils;
+import org.elasticsearch.core.XmlUtils;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.FileMatchers;
 import org.elasticsearch.xpack.core.ssl.CertParsingUtils;
@@ -145,7 +146,7 @@ protected <T extends XMLObject> T domElementToXmlObject(Element element, Class<T
     }
 
     protected void print(Element element, Writer writer, boolean pretty) throws TransformerException {
-        final Transformer serializer = SamlFactory.getHardenedXMLTransformer();
+        final Transformer serializer = XmlUtils.getHardenedXMLTransformer();
         if (pretty) {
             serializer.setOutputProperty(OutputKeys.INDENT, "yes");
         }

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

@@ -12,6 +12,7 @@
 import org.apache.xml.security.encryption.XMLCipher;
 import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.core.Tuple;
+import org.elasticsearch.core.XmlUtils;
 import org.elasticsearch.xpack.core.watcher.watch.ClockMock;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -151,7 +152,7 @@ protected String randomId() {
     }
 
     protected Document parseDocument(String xml) throws ParserConfigurationException, SAXException, IOException {
-        final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
+        final DocumentBuilderFactory dbf = XmlUtils.getHardenedBuilderFactory();
         dbf.setNamespaceAware(true);
         final DocumentBuilder documentBuilder = dbf.newDocumentBuilder();
         return documentBuilder.parse(new InputSource(new StringReader(xml)));