Introduce hardened XML utilities in core #133644
Conversation
richard-dennehy
added
>refactoring
:Security/Security
richard-dennehy
force-pushed
the
xml-safe-utils
branch
from
35c987e to
a040bf2
Compare
richard-dennehy
changed the title
| javax.xml.parsers.DocumentBuilderFactory#newDefaultNSInstance() | ||
| javax.xml.parsers.DocumentBuilderFactory#newNSInstance() | ||
| javax.xml.parsers.DocumentBuilderFactory#newNSInstance(java.lang.String, java.lang.ClassLoader) |
There was a problem hiding this comment.
I'm assuming we want to block all of these as well
| * 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 { |
There was a problem hiding this comment.
We need to provide a DocumentBuilder that doesn't validate schema as some existing usages of this API don't perform schema validation
| tfactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); | ||
| tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); | ||
| tfactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); |
There was a problem hiding this comment.
As I understand, we don't actually need to configure ACCESS_EXTERNAL_DTD or ACCESS_EXTERNAL_STYLESHEET when FEATURE_SECURE_PROCESSING is true; as per JAXP Properties for External Access Restrictions:
Explicitly turning on Feature for Secure Processing (FSP) through the API, for example, factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true), disables all external connections.
There was a problem hiding this comment.
You are right. This is probably for historical reasons and old JDKs. If no harm, I think we should keep it.
| ParserConfigurationException { | ||
| var saxParserFactory = SAXParserFactory.newInstance(); | ||
|
|
||
| saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); |
There was a problem hiding this comment.
As I understand, this is the only security feature we actually need to set
|
Pinging @elastic/es-security (Team:Security) |
elasticsearchmachine
added
the
serverless-linked
Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
3 participants
Create/move a set of utility functions that wrap the various java XML classes, providing secure default settings (e.g. prevent XXE), and enforce that these are used.
Provides secured versions of:
I've tested integrating with #130337 in 665ce15 and the tests pass