Skip to content

Commit cdaf7a5

Browse files
richard-dennehyrichard-dennehy
committed
move hardened XML factories to core
1 parent 699f4cf commit cdaf7a5

File tree

5 files changed

+155
-101
lines changed

5 files changed

+155
-101
lines changed

‎build-tools-internal/src/main/resources/forbidden/jdk-signatures.txt‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,21 @@ java.util.Collections#shuffle(java.util.List) @ Use java.util.Collections#shuffl
102102

103103
@defaultMessage Avoid creating FilePermission objects directly, but use FilePermissionUtils instead
104104
java.io.FilePermission#<init>(java.lang.String,java.lang.String)
105+
106+
@defaultMessage DocumentBuilderFactory should not be used directly. Use XmlUtils#getHardenedDocumentBuilder(java.lang.String[]) instead.
107+
javax.xml.parsers.DocumentBuilderFactory#newInstance()
108+
javax.xml.parsers.DocumentBuilderFactory#newInstance(java.lang.String, java.lang.ClassLoader)
109+
javax.xml.parsers.DocumentBuilderFactory#newDefaultNSInstance()
110+
javax.xml.parsers.DocumentBuilderFactory#newNSInstance()
111+
javax.xml.parsers.DocumentBuilderFactory#newNSInstance(java.lang.String, java.lang.ClassLoader)
112+
113+
@defaultMessage TransformerFactory should not be used directly. Use XmlUtils#getHardenedXMLTransformer() instead.
114+
javax.xml.transform.TransformerFactory#newInstance()
115+
javax.xml.transform.TransformerFactory#newInstance(java.lang.String, java.lang.ClassLoader)
116+
117+
@defaultMessage SAXParserFactory should not be used directly. Use XmlUtils#getHardenedSaxParser() instead
118+
javax.xml.parsers.SAXParserFactory#newInstance()
119+
javax.xml.parsers.SAXParserFactory#newInstance(java.lang.String, java.lang.ClassLoader)
120+
javax.xml.parsers.SAXParserFactory#newDefaultNSInstance()
121+
javax.xml.parsers.SAXParserFactory#newNSInstance()
122+
javax.xml.parsers.SAXParserFactory#newNSInstance(java.lang.String, java.lang.ClassLoader)

‎libs/core/src/main/java/module-info.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
module org.elasticsearch.base {
1313
requires static jsr305;
1414
requires org.elasticsearch.logging;
15+
requires java.xml;
1516

1617
exports org.elasticsearch.core;
1718
exports org.elasticsearch.jdk;

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.core;
11+
12+
import org.elasticsearch.logging.LogManager;
13+
import org.elasticsearch.logging.Logger;
14+
import org.xml.sax.SAXException;
15+
import org.xml.sax.SAXParseException;
16+
17+
import javax.xml.XMLConstants;
18+
import javax.xml.parsers.DocumentBuilder;
19+
import javax.xml.parsers.DocumentBuilderFactory;
20+
import javax.xml.parsers.ParserConfigurationException;
21+
import javax.xml.transform.Transformer;
22+
import javax.xml.transform.TransformerConfigurationException;
23+
import javax.xml.transform.TransformerException;
24+
import javax.xml.transform.TransformerFactory;
25+
26+
public class XmlUtils {
27+
28+
private static final Logger LOGGER = LogManager.getLogger(XmlUtils.class);
29+
30+
/**
31+
* Constructs a DocumentBuilder with all the necessary features for it to be secure
32+
*
33+
* @throws ParserConfigurationException if one of the features can't be set on the DocumentBuilderFactory
34+
*/
35+
@SuppressForbidden(reason = "This is the only allowed way to construct a DocumentBuilder")
36+
public static DocumentBuilder getHardenedBuilder(String[] schemaFiles) throws ParserConfigurationException {
37+
final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
38+
dbf.setNamespaceAware(true);
39+
// Ensure that Schema Validation is enabled for the factory
40+
dbf.setValidating(true);
41+
// Disallow internal and external entity expansion
42+
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
43+
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
44+
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
45+
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
46+
dbf.setFeature("http://xml.org/sax/features/validation", true);
47+
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
48+
dbf.setIgnoringComments(true);
49+
// This is required, otherwise schema validation causes signature invalidation
50+
dbf.setFeature("http://apache.org/xml/features/validation/schema/normalized-value", false);
51+
// Make sure that URL schema namespaces are not resolved/downloaded from URLs we do not control
52+
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "file,jar");
53+
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar");
54+
dbf.setFeature("http://apache.org/xml/features/honour-all-schemaLocations", true);
55+
// Ensure we do not resolve XIncludes. Defaults to false, but set it explicitly to be future-proof
56+
dbf.setXIncludeAware(false);
57+
// Ensure we do not expand entity reference nodes
58+
dbf.setExpandEntityReferences(false);
59+
// Further limit danger from denial of service attacks
60+
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
61+
dbf.setAttribute("http://apache.org/xml/features/validation/schema", true);
62+
dbf.setAttribute("http://apache.org/xml/features/validation/schema-full-checking", true);
63+
dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaLanguage", XMLConstants.W3C_XML_SCHEMA_NS_URI);
64+
// We ship our own xsd files for schema validation since we do not trust anyone else.
65+
dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaSource", schemaFiles);
66+
DocumentBuilder documentBuilder = dbf.newDocumentBuilder();
67+
documentBuilder.setErrorHandler(new ErrorHandler());
68+
return documentBuilder;
69+
}
70+
71+
@SuppressForbidden(reason = "This is the only allowed way to construct a Transformer")
72+
public static Transformer getHardenedXMLTransformer() throws TransformerConfigurationException {
73+
final TransformerFactory tfactory = TransformerFactory.newInstance();
74+
tfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
75+
tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
76+
tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
77+
tfactory.setAttribute("indent-number", 2);
78+
Transformer transformer = tfactory.newTransformer();
79+
transformer.setErrorListener(new ErrorListener());
80+
return transformer;
81+
}
82+
83+
private static class ErrorHandler implements org.xml.sax.ErrorHandler {
84+
/**
85+
* Enabling schema validation with `setValidating(true)` in our
86+
* DocumentBuilderFactory requires that we provide our own
87+
* ErrorHandler implementation
88+
*
89+
* @throws SAXException If the document we attempt to parse is not valid according to the specified schema.
90+
*/
91+
@Override
92+
public void warning(SAXParseException e) throws SAXException {
93+
LOGGER.debug("XML Parser error ", e);
94+
throw e;
95+
}
96+
97+
@Override
98+
public void error(SAXParseException e) throws SAXException {
99+
LOGGER.debug("XML Parser error ", e);
100+
throw e;
101+
}
102+
103+
@Override
104+
public void fatalError(SAXParseException e) throws SAXException {
105+
LOGGER.debug("XML Parser error ", e);
106+
throw e;
107+
}
108+
}
109+
110+
111+
private static class ErrorListener implements javax.xml.transform.ErrorListener {
112+
113+
@Override
114+
public void warning(TransformerException e) throws TransformerException {
115+
LOGGER.debug("XML transformation error", e);
116+
throw e;
117+
}
118+
119+
@Override
120+
public void error(TransformerException e) throws TransformerException {
121+
LOGGER.debug("XML transformation error", e);
122+
throw e;
123+
}
124+
125+
@Override
126+
public void fatalError(TransformerException e) throws TransformerException {
127+
LOGGER.debug("XML transformation error", e);
128+
throw e;
129+
}
130+
}
131+
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlSpMetadataAction.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.action.support.ActionFilters;
1212
import org.elasticsearch.action.support.HandledTransportAction;
1313
import org.elasticsearch.common.util.concurrent.EsExecutors;
14+
import org.elasticsearch.core.XmlUtils;
1415
import org.elasticsearch.injection.guice.Inject;
1516
import org.elasticsearch.tasks.Task;
1617
import org.elasticsearch.transport.TransportService;
@@ -75,7 +76,7 @@ private void prepareMetadata(SamlRealm realm, ActionListener<SamlSpMetadataRespo
7576
final EntityDescriptor descriptor = builder.build();
7677
final Element element = marshaller.marshall(descriptor);
7778
final StringWriter writer = new StringWriter();
78-
final Transformer serializer = SamlUtils.getHardenedXMLTransformer();
79+
final Transformer serializer = XmlUtils.getHardenedXMLTransformer();
7980
serializer.transform(new DOMSource(element), new StreamResult(writer));
8081
listener.onResponse(new SamlSpMetadataResponse(writer.toString()));
8182
} catch (Exception e) {

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java‎

Lines changed: 3 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import org.elasticsearch.SpecialPermission;
1313
import org.elasticsearch.common.hash.MessageDigests;
1414
import org.elasticsearch.core.IOUtils;
15-
import org.elasticsearch.core.SuppressForbidden;
15+
import org.elasticsearch.core.XmlUtils;
1616
import org.elasticsearch.xpack.core.security.support.RestorableContextClassLoader;
1717
import org.opensaml.core.config.InitializationService;
1818
import org.opensaml.core.xml.XMLObject;
@@ -30,8 +30,6 @@
3030
import org.w3c.dom.ls.DOMImplementationLS;
3131
import org.w3c.dom.ls.LSInput;
3232
import org.w3c.dom.ls.LSResourceResolver;
33-
import org.xml.sax.SAXException;
34-
import org.xml.sax.SAXParseException;
3533

3634
import java.io.IOException;
3735
import java.io.InputStream;
@@ -51,13 +49,10 @@
5149
import javax.xml.XMLConstants;
5250
import javax.xml.namespace.QName;
5351
import javax.xml.parsers.DocumentBuilder;
54-
import javax.xml.parsers.DocumentBuilderFactory;
5552
import javax.xml.parsers.ParserConfigurationException;
5653
import javax.xml.transform.OutputKeys;
5754
import javax.xml.transform.Transformer;
58-
import javax.xml.transform.TransformerConfigurationException;
5955
import javax.xml.transform.TransformerException;
60-
import javax.xml.transform.TransformerFactory;
6156
import javax.xml.transform.dom.DOMSource;
6257
import javax.xml.transform.stream.StreamResult;
6358
import javax.xml.transform.stream.StreamSource;
@@ -161,7 +156,7 @@ static String toString(Element element) {
161156
}
162157

163158
static void print(Element element, Writer writer, boolean pretty) throws TransformerException {
164-
final Transformer serializer = getHardenedXMLTransformer();
159+
final Transformer serializer = XmlUtils.getHardenedXMLTransformer();
165160
if (pretty) {
166161
serializer.setOutputProperty(OutputKeys.INDENT, "yes");
167162
}
@@ -211,18 +206,6 @@ static String describeSamlObject(SAMLObject object) {
211206
return getXmlContent(object, true);
212207
}
213208

214-
@SuppressForbidden(reason = "This is the only allowed way to construct a Transformer")
215-
public static Transformer getHardenedXMLTransformer() throws TransformerConfigurationException {
216-
final TransformerFactory tfactory = TransformerFactory.newInstance();
217-
tfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
218-
tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
219-
tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
220-
tfactory.setAttribute("indent-number", 2);
221-
Transformer transformer = tfactory.newTransformer();
222-
transformer.setErrorListener(new ErrorListener());
223-
return transformer;
224-
}
225-
226209
static void validate(InputStream xml, String xsdName) throws Exception {
227210
SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
228211
try (InputStream xsdStream = loadSchema(xsdName); ResourceResolver resolver = new ResourceResolver()) {
@@ -277,40 +260,8 @@ public void close() throws IOException {
277260
*
278261
* @throws ParserConfigurationException if one of the features can't be set on the DocumentBuilderFactory
279262
*/
280-
@SuppressForbidden(reason = "This is the only allowed way to construct a DocumentBuilder")
281263
public static DocumentBuilder getHardenedBuilder(String[] schemaFiles) throws ParserConfigurationException {
282-
final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
283-
dbf.setNamespaceAware(true);
284-
// Ensure that Schema Validation is enabled for the factory
285-
dbf.setValidating(true);
286-
// Disallow internal and external entity expansion
287-
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
288-
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
289-
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
290-
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
291-
dbf.setFeature("http://xml.org/sax/features/validation", true);
292-
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
293-
dbf.setIgnoringComments(true);
294-
// This is required, otherwise schema validation causes signature invalidation
295-
dbf.setFeature("http://apache.org/xml/features/validation/schema/normalized-value", false);
296-
// Make sure that URL schema namespaces are not resolved/downloaded from URLs we do not control
297-
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "file,jar");
298-
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar");
299-
dbf.setFeature("http://apache.org/xml/features/honour-all-schemaLocations", true);
300-
// Ensure we do not resolve XIncludes. Defaults to false, but set it explicitly to be future-proof
301-
dbf.setXIncludeAware(false);
302-
// Ensure we do not expand entity reference nodes
303-
dbf.setExpandEntityReferences(false);
304-
// Further limit danger from denial of service attacks
305-
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
306-
dbf.setAttribute("http://apache.org/xml/features/validation/schema", true);
307-
dbf.setAttribute("http://apache.org/xml/features/validation/schema-full-checking", true);
308-
dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaLanguage", XMLConstants.W3C_XML_SCHEMA_NS_URI);
309-
// We ship our own xsd files for schema validation since we do not trust anyone else.
310-
dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaSource", resolveSchemaFilePaths(schemaFiles));
311-
DocumentBuilder documentBuilder = dbf.newDocumentBuilder();
312-
documentBuilder.setErrorHandler(new ErrorHandler());
313-
return documentBuilder;
264+
return XmlUtils.getHardenedBuilder(resolveSchemaFilePaths(schemaFiles));
314265
}
315266

316267
private static String[] resolveSchemaFilePaths(String[] relativePaths) {
@@ -324,52 +275,4 @@ private static String[] resolveSchemaFilePaths(String[] relativePaths) {
324275
}
325276
}).filter(Objects::nonNull).toArray(String[]::new);
326277
}
327-
328-
private static class ErrorListener implements javax.xml.transform.ErrorListener {
329-
330-
@Override
331-
public void warning(TransformerException e) throws TransformerException {
332-
LOGGER.debug("XML transformation error", e);
333-
throw e;
334-
}
335-
336-
@Override
337-
public void error(TransformerException e) throws TransformerException {
338-
LOGGER.debug("XML transformation error", e);
339-
throw e;
340-
}
341-
342-
@Override
343-
public void fatalError(TransformerException e) throws TransformerException {
344-
LOGGER.debug("XML transformation error", e);
345-
throw e;
346-
}
347-
}
348-
349-
private static class ErrorHandler implements org.xml.sax.ErrorHandler {
350-
/**
351-
* Enabling schema validation with `setValidating(true)` in our
352-
* DocumentBuilderFactory requires that we provide our own
353-
* ErrorHandler implementation
354-
*
355-
* @throws SAXException If the document we attempt to parse is not valid according to the specified schema.
356-
*/
357-
@Override
358-
public void warning(SAXParseException e) throws SAXException {
359-
LOGGER.debug("XML Parser error ", e);
360-
throw e;
361-
}
362-
363-
@Override
364-
public void error(SAXParseException e) throws SAXException {
365-
LOGGER.debug("XML Parser error ", e);
366-
throw e;
367-
}
368-
369-
@Override
370-
public void fatalError(SAXParseException e) throws SAXException {
371-
LOGGER.debug("XML Parser error ", e);
372-
throw e;
373-
}
374-
}
375278
}

0 commit comments

Comments
 (0)