Skip to content

Commit a00c6b7

Browse files
richard-dennehyelasticsearchmachine
authored andcommitted
Introduce hardened XML utilities in core (elastic#133644)
Co-authored-by: elasticsearchmachine <[email protected]>
1 parent de458f4 commit a00c6b7

File tree

18 files changed

+243
-235
lines changed

18 files changed

+243
-235
lines changed

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,29 @@ 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)
123+
124+
@defaultMessage SchemaValidator should not be used directly. Use XmlUtils#getHardenedSchemaValidator() instead
125+
javax.xml.validation.SchemaFactory#newDefaultInstance()
126+
javax.xml.validation.SchemaFactory#newInstance(java.lang.String)
127+
javax.xml.validation.SchemaFactory#newInstance(java.lang.String, java.lang.String, java.lang.ClassLoader)
128+
129+
@defaultMessage Validator should not be used directly. Use XmlUtils#getHardenedValidator() instead
130+
javax.xml.validation.Schema#newValidator()

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;
Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
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.SAXNotRecognizedException;
16+
import org.xml.sax.SAXNotSupportedException;
17+
import org.xml.sax.SAXParseException;
18+
19+
import javax.xml.XMLConstants;
20+
import javax.xml.parsers.DocumentBuilder;
21+
import javax.xml.parsers.DocumentBuilderFactory;
22+
import javax.xml.parsers.ParserConfigurationException;
23+
import javax.xml.parsers.SAXParserFactory;
24+
import javax.xml.transform.Transformer;
25+
import javax.xml.transform.TransformerConfigurationException;
26+
import javax.xml.transform.TransformerException;
27+
import javax.xml.transform.TransformerFactory;
28+
import javax.xml.validation.Schema;
29+
import javax.xml.validation.SchemaFactory;
30+
import javax.xml.validation.Validator;
31+
32+
public class XmlUtils {
33+
34+
private static final Logger LOGGER = LogManager.getLogger(XmlUtils.class);
35+
36+
/**
37+
* Constructs a DocumentBuilder with all the necessary features for it to be secure
38+
*
39+
* @throws ParserConfigurationException if one of the features can't be set on the DocumentBuilderFactory
40+
*/
41+
public static DocumentBuilder getHardenedBuilder(String[] schemaFiles) throws ParserConfigurationException {
42+
final DocumentBuilderFactory dbf = getHardenedBuilderFactory();
43+
dbf.setNamespaceAware(true);
44+
// Ensure that Schema Validation is enabled for the factory
45+
dbf.setValidating(true);
46+
// This is required, otherwise schema validation causes signature invalidation
47+
dbf.setFeature("http://apache.org/xml/features/validation/schema/normalized-value", false);
48+
// Make sure that URL schema namespaces are not resolved/downloaded from URLs we do not control
49+
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "file,jar");
50+
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "file,jar");
51+
// We ship our own xsd files for schema validation since we do not trust anyone else.
52+
dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaSource", schemaFiles);
53+
DocumentBuilder documentBuilder = dbf.newDocumentBuilder();
54+
documentBuilder.setErrorHandler(new ErrorHandler());
55+
return documentBuilder;
56+
}
57+
58+
/**
59+
* Returns a DocumentBuilderFactory pre-configured to be secure
60+
*/
61+
@SuppressForbidden(reason = "This is the only allowed way to construct a DocumentBuilder")
62+
public static DocumentBuilderFactory getHardenedBuilderFactory() throws ParserConfigurationException {
63+
final DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
64+
// Disallow internal and external entity expansion
65+
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
66+
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
67+
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
68+
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
69+
dbf.setFeature("http://xml.org/sax/features/validation", true);
70+
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
71+
dbf.setIgnoringComments(true);
72+
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
73+
dbf.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
74+
dbf.setFeature("http://apache.org/xml/features/honour-all-schemaLocations", true);
75+
// Ensure we do not resolve XIncludes. Defaults to false, but set it explicitly to be future-proof
76+
dbf.setXIncludeAware(false);
77+
// Ensure we do not expand entity reference nodes
78+
dbf.setExpandEntityReferences(false);
79+
// Further limit danger from denial of service attacks
80+
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
81+
dbf.setAttribute("http://apache.org/xml/features/validation/schema", true);
82+
dbf.setAttribute("http://apache.org/xml/features/validation/schema-full-checking", true);
83+
dbf.setAttribute("http://java.sun.com/xml/jaxp/properties/schemaLanguage", XMLConstants.W3C_XML_SCHEMA_NS_URI);
84+
85+
return dbf;
86+
}
87+
88+
/**
89+
* Constructs a Transformer configured to be secure
90+
*/
91+
@SuppressForbidden(reason = "This is the only allowed way to construct a Transformer")
92+
public static Transformer getHardenedXMLTransformer() throws TransformerConfigurationException {
93+
final TransformerFactory tfactory = TransformerFactory.newInstance();
94+
tfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
95+
tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
96+
tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
97+
tfactory.setAttribute("indent-number", 2);
98+
Transformer transformer = tfactory.newTransformer();
99+
transformer.setErrorListener(new ErrorListener());
100+
return transformer;
101+
}
102+
103+
/**
104+
* Returns a SchemaFactory configured to be secure
105+
*/
106+
@SuppressForbidden(reason = "This is the only allowed way to construct a SchemaFactory")
107+
public static SchemaFactory getHardenedSchemaFactory() throws SAXNotSupportedException, SAXNotRecognizedException {
108+
SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
109+
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
110+
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
111+
return schemaFactory;
112+
}
113+
114+
/**
115+
* Constructs a Validator configured to be secure
116+
*/
117+
@SuppressForbidden(reason = "This is the only allowed way to construct a Validator")
118+
public static Validator getHardenedValidator(Schema schema) throws SAXNotSupportedException, SAXNotRecognizedException {
119+
Validator validator = schema.newValidator();
120+
validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
121+
validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
122+
return validator;
123+
}
124+
125+
@SuppressForbidden(reason = "This is the only allowed way to construct a SAXParser")
126+
public static SAXParserFactory getHardenedSaxParserFactory() throws SAXNotSupportedException, SAXNotRecognizedException,
127+
ParserConfigurationException {
128+
var saxParserFactory = SAXParserFactory.newInstance();
129+
130+
saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
131+
saxParserFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
132+
saxParserFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
133+
saxParserFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
134+
saxParserFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
135+
136+
return saxParserFactory;
137+
}
138+
139+
private static class ErrorHandler implements org.xml.sax.ErrorHandler {
140+
/**
141+
* Enabling schema validation with `setValidating(true)` in our
142+
* DocumentBuilderFactory requires that we provide our own
143+
* ErrorHandler implementation
144+
*
145+
* @throws SAXException If the document we attempt to parse is not valid according to the specified schema.
146+
*/
147+
@Override
148+
public void warning(SAXParseException e) throws SAXException {
149+
LOGGER.debug("XML Parser error ", e);
150+
throw e;
151+
}
152+
153+
@Override
154+
public void error(SAXParseException e) throws SAXException {
155+
LOGGER.debug("XML Parser error ", e);
156+
throw e;
157+
}
158+
159+
@Override
160+
public void fatalError(SAXParseException e) throws SAXException {
161+
LOGGER.debug("XML Parser error ", e);
162+
throw e;
163+
}
164+
}
165+
166+
private static class ErrorListener implements javax.xml.transform.ErrorListener {
167+
168+
@Override
169+
public void warning(TransformerException e) throws TransformerException {
170+
LOGGER.debug("XML transformation error", e);
171+
throw e;
172+
}
173+
174+
@Override
175+
public void error(TransformerException e) throws TransformerException {
176+
LOGGER.debug("XML transformation error", e);
177+
throw e;
178+
}
179+
180+
@Override
181+
public void fatalError(TransformerException e) throws TransformerException {
182+
LOGGER.debug("XML transformation error", e);
183+
throw e;
184+
}
185+
}
186+
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.elasticsearch.core.Nullable;
2323
import org.elasticsearch.core.SuppressForbidden;
2424
import org.elasticsearch.core.Tuple;
25+
import org.elasticsearch.core.XmlUtils;
2526
import org.elasticsearch.logging.LogManager;
2627
import org.elasticsearch.logging.Logger;
2728
import org.elasticsearch.rest.RestStatus;
@@ -46,8 +47,6 @@
4647
import java.util.regex.Matcher;
4748
import java.util.regex.Pattern;
4849

49-
import javax.xml.parsers.DocumentBuilderFactory;
50-
5150
import static java.nio.charset.StandardCharsets.UTF_8;
5251
import static org.elasticsearch.test.fixture.HttpHeaderParser.parseRangeHeader;
5352
import static org.junit.Assert.assertEquals;
@@ -471,7 +470,7 @@ private static Tuple<String, BytesReference> parseRequestBody(final HttpExchange
471470

472471
static List<String> extractPartEtags(BytesReference completeMultipartUploadBody) {
473472
try {
474-
final var document = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(completeMultipartUploadBody.streamInput());
473+
final var document = XmlUtils.getHardenedBuilderFactory().newDocumentBuilder().parse(completeMultipartUploadBody.streamInput());
475474
final var parts = document.getElementsByTagName("Part");
476475
final var result = new ArrayList<String>(parts.getLength());
477476
for (int partIndex = 0; partIndex < parts.getLength(); partIndex++) {

x-pack/plugin/identity-provider/build.gradle

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,6 @@ tasks.named("forbiddenPatterns").configure {
8080
exclude '**/*.zip'
8181
}
8282

83-
tasks.named('forbiddenApisMain').configure {
84-
signaturesFiles += files('forbidden/xml-signatures.txt')
85-
}
86-
8783
// classes are missing, e.g. com.ibm.icu.lang.UCharacter
8884
tasks.named("thirdPartyAudit").configure {
8985
ignoreMissingClasses(

x-pack/plugin/identity-provider/forbidden/xml-signatures.txt

Lines changed: 0 additions & 8 deletions
This file was deleted.

x-pack/plugin/identity-provider/qa/idp-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/idp/IdentityProviderAuthenticationIT.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.common.settings.Settings;
1818
import org.elasticsearch.common.util.concurrent.ThreadContext;
1919
import org.elasticsearch.core.Nullable;
20+
import org.elasticsearch.core.XmlUtils;
2021
import org.elasticsearch.xcontent.ObjectPath;
2122
import org.elasticsearch.xcontent.XContentBuilder;
2223
import org.elasticsearch.xcontent.json.JsonXContent;
@@ -163,7 +164,7 @@ public void testCustomAttributesInIdpInitiatedSso() throws Exception {
163164
final String samlResponse = generateSamlResponseWithAttributes(SP_ENTITY_ID, SP_ACS, null, attributesMap);
164165

165166
// Parse XML directly from samlResponse (it's not base64 encoded at this point)
166-
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
167+
DocumentBuilderFactory factory = XmlUtils.getHardenedBuilderFactory();
167168
factory.setNamespaceAware(true); // Required for XPath
168169
DocumentBuilder builder = factory.newDocumentBuilder();
169170
Document document = builder.parse(new InputSource(new StringReader(samlResponse)));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ private static void validateNameIdPolicy(AuthnRequest request, SamlServiceProvid
250250
}
251251

252252
private boolean validateSignature(ParsedQueryString queryString, Collection<X509Credential> credentials) {
253-
final String javaSigAlgorithm = SamlFactory.getJavaAlorithmNameFromUri(queryString.sigAlg);
253+
final String javaSigAlgorithm = SamlFactory.getJavaAlgorithmNameFromUri(queryString.sigAlg);
254254
final byte[] contentBytes = queryString.reconstructQueryParameters().getBytes(StandardCharsets.UTF_8);
255255
final byte[] signatureBytes = Base64.getDecoder().decode(queryString.signature);
256256
return credentials.stream().anyMatch(credential -> {

0 commit comments

Comments
 (0)