diff --git a/build-tools-internal/src/main/resources/forbidden/jdk-signatures.txt b/build-tools-internal/src/main/resources/forbidden/jdk-signatures.txt index b6db5776de43c..a852848341861 100644 --- a/build-tools-internal/src/main/resources/forbidden/jdk-signatures.txt +++ b/build-tools-internal/src/main/resources/forbidden/jdk-signatures.txt @@ -102,3 +102,29 @@ java.util.Collections#shuffle(java.util.List) @ Use java.util.Collections#shuffl @defaultMessage Avoid creating FilePermission objects directly, but use FilePermissionUtils instead java.io.FilePermission#(java.lang.String,java.lang.String) + +@defaultMessage DocumentBuilderFactory should not be used directly. Use XmlUtils#getHardenedDocumentBuilder(java.lang.String[]) instead. +javax.xml.parsers.DocumentBuilderFactory#newInstance() +javax.xml.parsers.DocumentBuilderFactory#newInstance(java.lang.String, java.lang.ClassLoader) +javax.xml.parsers.DocumentBuilderFactory#newDefaultNSInstance() +javax.xml.parsers.DocumentBuilderFactory#newNSInstance() +javax.xml.parsers.DocumentBuilderFactory#newNSInstance(java.lang.String, java.lang.ClassLoader) + +@defaultMessage TransformerFactory should not be used directly. Use XmlUtils#getHardenedXMLTransformer() instead. +javax.xml.transform.TransformerFactory#newInstance() +javax.xml.transform.TransformerFactory#newInstance(java.lang.String, java.lang.ClassLoader) + +@defaultMessage SAXParserFactory should not be used directly. Use XmlUtils#getHardenedSaxParser() instead +javax.xml.parsers.SAXParserFactory#newInstance() +javax.xml.parsers.SAXParserFactory#newInstance(java.lang.String, java.lang.ClassLoader) +javax.xml.parsers.SAXParserFactory#newDefaultNSInstance() +javax.xml.parsers.SAXParserFactory#newNSInstance() +javax.xml.parsers.SAXParserFactory#newNSInstance(java.lang.String, java.lang.ClassLoader) + +@defaultMessage SchemaValidator should not be used directly. Use XmlUtils#getHardenedSchemaValidator() instead +javax.xml.validation.SchemaFactory#newDefaultInstance() +javax.xml.validation.SchemaFactory#newInstance(java.lang.String) +javax.xml.validation.SchemaFactory#newInstance(java.lang.String, java.lang.String, java.lang.ClassLoader) + +@defaultMessage Validator should not be used directly. Use XmlUtils#getHardenedValidator() instead +javax.xml.validation.Schema#newValidator() diff --git a/libs/core/src/main/java/module-info.java b/libs/core/src/main/java/module-info.java index 487ce69fad36b..31bdc35fe1c1f 100644 --- a/libs/core/src/main/java/module-info.java +++ b/libs/core/src/main/java/module-info.java @@ -12,6 +12,7 @@ module org.elasticsearch.base { requires static jsr305; requires org.elasticsearch.logging; + requires java.xml; exports org.elasticsearch.core; exports org.elasticsearch.jdk; diff --git a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java new file mode 100644 index 0000000000000..8eea84041e353 --- /dev/null +++ b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java @@ -0,0 +1,186 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.core; + +import org.elasticsearch.logging.LogManager; +import org.elasticsearch.logging.Logger; +import org.xml.sax.SAXException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; +import org.xml.sax.SAXParseException; + +import javax.xml.XMLConstants; +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; +import javax.xml.transform.TransformerFactory; +import javax.xml.validation.Schema; +import javax.xml.validation.SchemaFactory; +import javax.xml.validation.Validator; + +public class XmlUtils { + + private static final Logger LOGGER = LogManager.getLogger(XmlUtils.class); + + /** + * 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 + */ + public static DocumentBuilder getHardenedBuilder(String[] schemaFiles) throws ParserConfigurationException { + 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; + } + + /** + * Returns a DocumentBuilderFactory pre-configured to be secure + */ + @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); + 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); + 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); + // 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); + + return dbf; + } + + /** + * Constructs a Transformer configured to be secure + */ + @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 ErrorListener()); + return transformer; + } + + /** + * Returns a SchemaFactory configured to be secure + */ + @SuppressForbidden(reason = "This is the only allowed way to construct a SchemaFactory") + public static SchemaFactory getHardenedSchemaFactory() throws SAXNotSupportedException, SAXNotRecognizedException { + SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + return schemaFactory; + } + + /** + * Constructs a Validator configured to be secure + */ + @SuppressForbidden(reason = "This is the only allowed way to construct a Validator") + public static Validator getHardenedValidator(Schema schema) throws SAXNotSupportedException, SAXNotRecognizedException { + Validator validator = schema.newValidator(); + validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + 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 + * 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 { + LOGGER.debug("XML Parser error ", e); + throw e; + } + + @Override + public void fatalError(SAXParseException e) throws SAXException { + LOGGER.debug("XML Parser error ", e); + throw e; + } + } + + private static class ErrorListener implements javax.xml.transform.ErrorListener { + + @Override + public void warning(TransformerException e) throws TransformerException { + LOGGER.debug("XML transformation error", e); + throw e; + } + + @Override + public void error(TransformerException e) throws TransformerException { + LOGGER.debug("XML transformation error", e); + throw e; + } + + @Override + public void fatalError(TransformerException e) throws TransformerException { + LOGGER.debug("XML transformation error", e); + throw e; + } + } +} diff --git a/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java b/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java index 9ca9441e6db9f..5b0a0b03d7af4 100644 --- a/test/fixtures/s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java +++ b/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; @@ -46,8 +47,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.xml.parsers.DocumentBuilderFactory; - import static java.nio.charset.StandardCharsets.UTF_8; import static org.elasticsearch.test.fixture.HttpHeaderParser.parseRangeHeader; import static org.junit.Assert.assertEquals; @@ -471,7 +470,7 @@ private static Tuple parseRequestBody(final HttpExchange static List 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(parts.getLength()); for (int partIndex = 0; partIndex < parts.getLength(); partIndex++) { diff --git a/x-pack/plugin/identity-provider/build.gradle b/x-pack/plugin/identity-provider/build.gradle index 42cf78886fa54..bdbff8ccaf8df 100644 --- a/x-pack/plugin/identity-provider/build.gradle +++ b/x-pack/plugin/identity-provider/build.gradle @@ -80,10 +80,6 @@ tasks.named("forbiddenPatterns").configure { exclude '**/*.zip' } -tasks.named('forbiddenApisMain').configure { - signaturesFiles += files('forbidden/xml-signatures.txt') -} - // classes are missing, e.g. com.ibm.icu.lang.UCharacter tasks.named("thirdPartyAudit").configure { ignoreMissingClasses( diff --git a/x-pack/plugin/identity-provider/forbidden/xml-signatures.txt b/x-pack/plugin/identity-provider/forbidden/xml-signatures.txt deleted file mode 100644 index 0acb0db2da1a7..0000000000000 --- a/x-pack/plugin/identity-provider/forbidden/xml-signatures.txt +++ /dev/null @@ -1,8 +0,0 @@ -@defaultMessage DocumentBuilderFactory should not be used directly. (See x-pack-security SamlUtils#getHardenedDocumentBuilder if you ever need one) -javax.xml.parsers.DocumentBuilderFactory#newInstance() -javax.xml.parsers.DocumentBuilderFactory#newInstance(java.lang.String, java.lang.ClassLoader) - -@defaultMessage TransformerFactory should not be used directly. Use IdPSamlTestCase#getHardenedXMLTransformer() instead. -javax.xml.transform.TransformerFactory#newInstance() -javax.xml.transform.TransformerFactory#newInstance(java.lang.String, java.lang.ClassLoader) - diff --git a/x-pack/plugin/identity-provider/qa/idp-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/idp/IdentityProviderAuthenticationIT.java b/x-pack/plugin/identity-provider/qa/idp-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/idp/IdentityProviderAuthenticationIT.java index 93771519aedc8..f619ee0d741d5 100644 --- a/x-pack/plugin/identity-provider/qa/idp-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/idp/IdentityProviderAuthenticationIT.java +++ b/x-pack/plugin/identity-provider/qa/idp-rest-tests/src/javaRestTest/java/org/elasticsearch/xpack/idp/IdentityProviderAuthenticationIT.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.XmlUtils; import org.elasticsearch.xcontent.ObjectPath; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.json.JsonXContent; @@ -163,7 +164,7 @@ public void testCustomAttributesInIdpInitiatedSso() throws Exception { final String samlResponse = generateSamlResponseWithAttributes(SP_ENTITY_ID, SP_ACS, null, attributesMap); // Parse XML directly from samlResponse (it's not base64 encoded at this point) - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory factory = XmlUtils.getHardenedBuilderFactory(); factory.setNamespaceAware(true); // Required for XPath DocumentBuilder builder = factory.newDocumentBuilder(); Document document = builder.parse(new InputSource(new StringReader(samlResponse))); diff --git a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java index 9fc9f4a28d250..1f0151e8b10b1 100644 --- a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java +++ b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/authn/SamlAuthnRequestValidator.java @@ -250,7 +250,7 @@ private static void validateNameIdPolicy(AuthnRequest request, SamlServiceProvid } private boolean validateSignature(ParsedQueryString queryString, Collection credentials) { - final String javaSigAlgorithm = SamlFactory.getJavaAlorithmNameFromUri(queryString.sigAlg); + final String javaSigAlgorithm = SamlFactory.getJavaAlgorithmNameFromUri(queryString.sigAlg); final byte[] contentBytes = queryString.reconstructQueryParameters().getBytes(StandardCharsets.UTF_8); final byte[] signatureBytes = Base64.getDecoder().decode(queryString.signature); return credentials.stream().anyMatch(credential -> { diff --git a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/SamlFactory.java b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/SamlFactory.java index a46bea16830f9..d661dfdf64e47 100644 --- a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/SamlFactory.java +++ b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/SamlFactory.java @@ -11,7 +11,7 @@ import org.elasticsearch.ElasticsearchSecurityException; 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 +24,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 +36,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 +137,7 @@ public static T buildXmlObject(Element element, Class 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,60 +211,16 @@ 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) { + public static String getJavaAlgorithmNameFromUri(String sigAlg) { return switch (sigAlg) { case "http://www.w3.org/2000/09/xmldsig#dsa-sha1" -> "SHA1withDSA"; case "http://www.w3.org/2000/09/xmldsig#dsa-sha256" -> "SHA256withDSA"; @@ -293,31 +243,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 diff --git a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/XmlValidator.java b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/XmlValidator.java index 6cdc94fbb7934..bb259a10ea906 100644 --- a/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/XmlValidator.java +++ b/x-pack/plugin/identity-provider/src/main/java/org/elasticsearch/xpack/idp/saml/support/XmlValidator.java @@ -8,10 +8,13 @@ package org.elasticsearch.xpack.idp.saml.support; import org.elasticsearch.core.IOUtils; +import org.elasticsearch.core.XmlUtils; import org.w3c.dom.bootstrap.DOMImplementationRegistry; import org.w3c.dom.ls.DOMImplementationLS; import org.w3c.dom.ls.LSInput; import org.w3c.dom.ls.LSResourceResolver; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -20,7 +23,6 @@ import java.util.ArrayList; import java.util.List; -import javax.xml.XMLConstants; import javax.xml.transform.stream.StreamSource; import javax.xml.validation.Schema; import javax.xml.validation.SchemaFactory; @@ -34,8 +36,8 @@ public class XmlValidator { private final SchemaFactory schemaFactory; private final String xsdName; - public XmlValidator(String xsdName) { - this.schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + public XmlValidator(String xsdName) throws SAXNotSupportedException, SAXNotRecognizedException { + this.schemaFactory = XmlUtils.getHardenedSchemaFactory(); this.xsdName = xsdName; } @@ -49,9 +51,7 @@ public void validate(InputStream xml) throws Exception { try (InputStream xsdStream = loadSchema(xsdName); ResourceResolver resolver = new ResourceResolver()) { schemaFactory.setResourceResolver(resolver); Schema schema = schemaFactory.newSchema(new StreamSource(xsdStream)); - Validator validator = schema.newValidator(); - validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); - validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + Validator validator = XmlUtils.getHardenedValidator(schema); validator.validate(new StreamSource(xml)); } } diff --git a/x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/saml/test/IdpSamlTestCase.java b/x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/saml/test/IdpSamlTestCase.java index aa489b5d14717..44249f3abe58f 100644 --- a/x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/saml/test/IdpSamlTestCase.java +++ b/x-pack/plugin/identity-provider/src/test/java/org/elasticsearch/xpack/idp/saml/test/IdpSamlTestCase.java @@ -11,13 +11,13 @@ 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; import org.elasticsearch.xpack.idp.saml.idp.SamlIdentityProvider; import org.elasticsearch.xpack.idp.saml.sp.SamlServiceProvider; import org.elasticsearch.xpack.idp.saml.sp.SamlServiceProviderResolver; -import org.elasticsearch.xpack.idp.saml.support.SamlFactory; import org.elasticsearch.xpack.idp.saml.support.SamlInit; import org.elasticsearch.xpack.idp.saml.support.XmlValidator; import org.hamcrest.Matchers; @@ -145,7 +145,7 @@ protected T domElementToXmlObject(Element element, Class 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); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandlerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandlerTests.java index 0e59ec4ec8476..9d037dcebdbce 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlResponseHandlerTests.java +++ b/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))); diff --git a/x-pack/plugin/text-structure/src/main/java/org/elasticsearch/xpack/textstructure/structurefinder/XmlTextStructureFinder.java b/x-pack/plugin/text-structure/src/main/java/org/elasticsearch/xpack/textstructure/structurefinder/XmlTextStructureFinder.java index 1369cf37b43bb..86aba6e441172 100644 --- a/x-pack/plugin/text-structure/src/main/java/org/elasticsearch/xpack/textstructure/structurefinder/XmlTextStructureFinder.java +++ b/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; }