From cdaf7a5b36a59672daa2ac79d341c5f038418d6b Mon Sep 17 00:00:00 2001 From: Richard Dennehy Date: Wed, 27 Aug 2025 13:46:26 +0100 Subject: [PATCH 01/10] move hardened XML factories to core --- .../resources/forbidden/jdk-signatures.txt | 18 +++ libs/core/src/main/java/module-info.java | 1 + .../java/org/elasticsearch/core/XmlUtils.java | 131 ++++++++++++++++++ .../saml/TransportSamlSpMetadataAction.java | 3 +- .../xpack/security/authc/saml/SamlUtils.java | 103 +------------- 5 files changed, 155 insertions(+), 101 deletions(-) create mode 100644 libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java 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..f854668de43f2 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,21 @@ 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) 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..99961c501d750 --- /dev/null +++ b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java @@ -0,0 +1,131 @@ +/* + * 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.SAXParseException; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerConfigurationException; +import javax.xml.transform.TransformerException; +import javax.xml.transform.TransformerFactory; + +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 + */ + @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", schemaFiles); + DocumentBuilder documentBuilder = dbf.newDocumentBuilder(); + documentBuilder.setErrorHandler(new ErrorHandler()); + return documentBuilder; + } + + @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; + } + + 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/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlSpMetadataAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlSpMetadataAction.java index 213e72b7d99c9..1cc390d177ec5 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlSpMetadataAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlSpMetadataAction.java @@ -11,6 +11,7 @@ import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.common.util.concurrent.EsExecutors; +import org.elasticsearch.core.XmlUtils; import org.elasticsearch.injection.guice.Inject; import org.elasticsearch.tasks.Task; import org.elasticsearch.transport.TransportService; @@ -75,7 +76,7 @@ private void prepareMetadata(SamlRealm realm, ActionListener Date: Wed, 27 Aug 2025 12:56:00 +0000 Subject: [PATCH 02/10] [CI] Auto commit changes from spotless --- libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java index 99961c501d750..1de0d108d3d2d 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java +++ b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java @@ -107,7 +107,6 @@ public void fatalError(SAXParseException e) throws SAXException { } } - private static class ErrorListener implements javax.xml.transform.ErrorListener { @Override From eca8c634906b6ddaa78eeacf3550f72cffd13e9c Mon Sep 17 00:00:00 2001 From: Richard Dennehy Date: Wed, 27 Aug 2025 15:15:53 +0100 Subject: [PATCH 03/10] replace DocumentBuilder in XmlTextStructureFinder --- .../java/org/elasticsearch/core/XmlUtils.java | 32 ++++++++++++------- .../authc/saml/SamlAuthenticatorTests.java | 3 +- .../XmlTextStructureFinder.java | 16 ++-------- 3 files changed, 24 insertions(+), 27 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java index 1de0d108d3d2d..7115e4f7eeaf1 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java +++ b/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") diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java index adf0b7f2653ca..a46d79329751c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java +++ b/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 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/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; } From a040bf25594646d983765799fda04ad33acce317 Mon Sep 17 00:00:00 2001 From: Richard Dennehy Date: Wed, 27 Aug 2025 15:23:15 +0100 Subject: [PATCH 04/10] forbid direct construction of xml SchemaValidator and Validator --- .../src/main/resources/forbidden/jdk-signatures.txt | 8 ++++++++ x-pack/plugin/identity-provider/build.gradle | 4 ---- .../plugin/identity-provider/forbidden/xml-signatures.txt | 8 -------- x-pack/plugin/security/build.gradle | 1 - x-pack/plugin/security/forbidden/xml-signatures.txt | 8 -------- 5 files changed, 8 insertions(+), 21 deletions(-) delete mode 100644 x-pack/plugin/identity-provider/forbidden/xml-signatures.txt delete mode 100644 x-pack/plugin/security/forbidden/xml-signatures.txt 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 f854668de43f2..a852848341861 100644 --- a/build-tools-internal/src/main/resources/forbidden/jdk-signatures.txt +++ b/build-tools-internal/src/main/resources/forbidden/jdk-signatures.txt @@ -120,3 +120,11 @@ javax.xml.parsers.SAXParserFactory#newInstance(java.lang.String, java.lang.Class 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/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/security/build.gradle b/x-pack/plugin/security/build.gradle index bfe6f6ec4fa44..bfeaa03adc422 100644 --- a/x-pack/plugin/security/build.gradle +++ b/x-pack/plugin/security/build.gradle @@ -192,7 +192,6 @@ tasks.named("forbiddenPatterns").configure { tasks.named('forbiddenApisMain').configure { signaturesFiles += files( 'forbidden/ldap-signatures.txt', - 'forbidden/xml-signatures.txt', 'forbidden/oidc-signatures.txt', project(':modules:transport-netty4').file('forbidden/netty-signatures.txt') ) diff --git a/x-pack/plugin/security/forbidden/xml-signatures.txt b/x-pack/plugin/security/forbidden/xml-signatures.txt deleted file mode 100644 index 3beeaea240c64..0000000000000 --- a/x-pack/plugin/security/forbidden/xml-signatures.txt +++ /dev/null @@ -1,8 +0,0 @@ -@defaultMessage DocumentBuilderFactory should not be used directly. Use SamlUtils#getHardenedDocumentBuilder(java.lang.String[]) instead. -javax.xml.parsers.DocumentBuilderFactory#newInstance() -javax.xml.parsers.DocumentBuilderFactory#newInstance(java.lang.String, java.lang.ClassLoader) - -@defaultMessage TransformerFactory should not be used directly. Use SamlUtils#getHardenedXMLTransformer() instead. -javax.xml.transform.TransformerFactory#newInstance() -javax.xml.transform.TransformerFactory#newInstance(java.lang.String, java.lang.ClassLoader) - From 43376ae6cf40006d7d8c5ba68d43d7b96bbec046 Mon Sep 17 00:00:00 2001 From: Richard Dennehy Date: Wed, 27 Aug 2025 16:57:54 +0100 Subject: [PATCH 05/10] replace constructions of SchemaFactory and Validator --- .../java/org/elasticsearch/core/XmlUtils.java | 33 +++++++++++++++++++ .../xpack/idp/saml/support/XmlValidator.java | 12 +++---- .../xpack/security/authc/saml/SamlUtils.java | 6 ++-- 3 files changed, 41 insertions(+), 10 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java index 7115e4f7eeaf1..248b7cfd53134 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java +++ b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java @@ -12,6 +12,8 @@ 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; @@ -22,6 +24,9 @@ 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 { @@ -49,6 +54,9 @@ public static DocumentBuilder getHardenedBuilder(String[] schemaFiles) throws Pa 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(); @@ -76,6 +84,9 @@ public static DocumentBuilderFactory getHardenedBuilderFactory() throws ParserCo 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(); @@ -88,6 +99,28 @@ public static Transformer getHardenedXMLTransformer() throws TransformerConfigur 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; + } + private static class ErrorHandler implements org.xml.sax.ErrorHandler { /** * Enabling schema validation with `setValidating(true)` in our 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/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java index 5b62745ebf56e..78dddbf3e5bf1 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java @@ -207,13 +207,11 @@ static String describeSamlObject(SAMLObject object) { } static void validate(InputStream xml, String xsdName) throws Exception { - SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + SchemaFactory schemaFactory = XmlUtils.getHardenedSchemaFactory(); 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)); } } From 9c725e8f143aeecdd39c187d075c31a003a8e149 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 27 Aug 2025 16:13:55 +0000 Subject: [PATCH 06/10] [CI] Auto commit changes from spotless --- .../org/elasticsearch/xpack/security/authc/saml/SamlUtils.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java index 78dddbf3e5bf1..37461b3261922 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlUtils.java @@ -46,7 +46,6 @@ import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; -import javax.xml.XMLConstants; import javax.xml.namespace.QName; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.ParserConfigurationException; From b3945dbcdb5582f351f8eb5db8d4ffcb18813da2 Mon Sep 17 00:00:00 2001 From: Richard Dennehy Date: Thu, 28 Aug 2025 09:39:41 +0100 Subject: [PATCH 07/10] replace DocumentBuilder in IdentityProviderAuthenticationIT --- .../xpack/idp/IdentityProviderAuthenticationIT.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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))); From f2ebb45894d721c6c06b975dc5e4b4c045375c75 Mon Sep 17 00:00:00 2001 From: Richard Dennehy Date: Thu, 28 Aug 2025 10:45:46 +0100 Subject: [PATCH 08/10] replace uses of DocumentBuilderFactory --- .../java/org/elasticsearch/core/XmlUtils.java | 15 ++++ .../main/java/fixture/s3/S3HttpHandler.java | 3 +- .../saml/authn/SamlAuthnRequestValidator.java | 1 + .../xpack/idp/saml/support/SamlFactory.java | 79 +------------------ .../xpack/idp/saml/test/IdpSamlTestCase.java | 3 +- .../authc/saml/SamlResponseHandlerTests.java | 3 +- 6 files changed, 25 insertions(+), 79 deletions(-) diff --git a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java b/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java index 248b7cfd53134..8eea84041e353 100644 --- a/libs/core/src/main/java/org/elasticsearch/core/XmlUtils.java +++ b/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 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..418242b9bfd46 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; @@ -471,7 +472,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/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..d62c3627f3fac 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 @@ -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; 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..329af9b663ac0 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 @@ -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 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,18 +212,6 @@ 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 * @@ -236,38 +219,7 @@ public static Transformer getHardenedXMLTransformer() throws TransformerConfigur */ @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 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..5ab939f8d9faf 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,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 domElementToXmlObject(Element element, Class Date: Thu, 28 Aug 2025 09:58:24 +0000 Subject: [PATCH 09/10] [CI] Auto commit changes from spotless --- .../s3-fixture/src/main/java/fixture/s3/S3HttpHandler.java | 2 -- .../xpack/idp/saml/authn/SamlAuthnRequestValidator.java | 1 - .../org/elasticsearch/xpack/idp/saml/test/IdpSamlTestCase.java | 1 - 3 files changed, 4 deletions(-) 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 418242b9bfd46..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 @@ -47,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; 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 d62c3627f3fac..9fc9f4a28d250 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 @@ -13,7 +13,6 @@ 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; 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 5ab939f8d9faf..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 @@ -18,7 +18,6 @@ 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; From 8db23bf9e11af53f116cf0afab42b64a44e7fe16 Mon Sep 17 00:00:00 2001 From: Richard Dennehy Date: Thu, 28 Aug 2025 11:37:11 +0100 Subject: [PATCH 10/10] cleanup --- .../xpack/idp/saml/authn/SamlAuthnRequestValidator.java | 2 +- .../org/elasticsearch/xpack/idp/saml/support/SamlFactory.java | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) 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 329af9b663ac0..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,6 @@ 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; @@ -217,12 +216,11 @@ public static Element toDomElement(XMLObject object) { * * @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 { 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";