Piotr Karwasz created CAMEL-24664:
-------------------------------------
Summary: Use Apache Commons Secure XML for JAXP factory creation
Key: CAMEL-24664
URL: https://issues.apache.org/jira/browse/CAMEL-24664
Project: Camel
Issue Type: Improvement
Components: camel-core, camel-validator, camel-xslt
Reporter: Piotr Karwasz
Camel hardens JAXP by hand. Around forty call sites each configure their own
subset of {{setFeature}} / {{setProperty}} calls, and the subsets disagree:
* {{XmlConverter.createDocumentBuilderFactory()}} omits
{{external-parameter-entities}} and {{load-external-dtd}};
{{createSAXParserFactory()}} omits {{disallow-doctype-decl}}.
* {{XMLConverterHelper}} sets {{disallow-doctype-decl}} twice, the second block
added by CAMEL-18825 directly beneath an identical one that had been there
since 2014.
* {{ProcessorDetailDevConsole}} sets neither {{FEATURE_SECURE_PROCESSING}} nor
{{load-external-dtd}}.
* Most sites swallow {{ParserConfigurationException}}, so a parser that rejects
a feature ends up hardened by accident rather than by contract.
The hardening accreted over twelve years in at least six waves (CAMEL-6933 in
2013, CAMEL-7123 and CAMEL-7131 in 2014, CAMEL-7713 in 2014, CAMEL-10894 in
2017, CAMEL-12444 in 2018, CAMEL-18825 in 2022). Each wave patched whichever
sites its author happened to be looking at. The threat model in
{{docs/user-manual/modules/ROOT/pages/security-model.adoc}} records the cost of
that drift: CVE-2015-0263 (XML converter in camel-core), CVE-2015-0264 (XPath
language), CVE-2017-5643 (Validation component) and CVE-2018-8027 (XSD
validation processor) are all in the XXE / remote-DTD class, and all land in
the code this issue touches.
h1. Proposal
Create every JAXP factory through [Apache Commons Secure
XML|https://commons.apache.org/proper/commons-secure-xml/]
({{org.apache.commons:commons-secure-xml}}, 1.0.0, Java 8+, no transitive
compile dependencies, Saxon-HE optional/provided, OSGi bundle), and delete the
hand-rolled feature blocks it subsumes.
h2. Feature-based hardening versus a resolver floor
This is the substantive change, and it is not a like-for-like swap.
|| || Today (feature based) || Proposed (resolver floor) ||
| Mechanism | a set of {{setFeature}} / {{setProperty}} calls per factory | a
non-removable ignore-all resolver installed under the parser |
| Coverage | whatever the implementation on the classpath honours | identical
on the stock JDK, Xerces, Woodstox, Saxon and Android |
| Default | varies per site, per implementation | external references are *not
fetched*, everywhere |
| Failure mode | implementation-specific error, or a silent fetch | the
reference resolves to *empty content* |
| Evidence | read the flag back with {{getFeature}} | observe what the parser
actually fetches |
Every factory returned by the library carries a floor that refuses the whole
family of external resource fetches: the external DTD subset, general and
parameter entities, {{xs:import}} / {{xs:include}} / {{xs:redefine}} and
{{xsi:schemaLocation}}, {{xsl:import}} / {{xsl:include}} and {{document()}},
{{XInclude}}, and the URI-fetching XPath functions {{doc()}}, {{collection()}}
and {{unparsed-text()}}.
*The floor cannot be removed, and a user resolver is layered in front of it
rather than replacing it.* {{setEntityResolver}}, {{setURIResolver}},
{{setResourceResolver}} and {{setXMLResolver}} install a _delegate that is
consulted first_; returning a non-null result opts that specific reference in,
and anything left unresolved falls through to the floor. So the extension point
is unchanged in shape, and every resolver Camel already installs
({{XsltUriResolver}}, {{DefaultLSResourceResolver}}, a user's own) keeps
working exactly as before.
Two consequences worth stating up front:
* An unresolved reference yields *empty content rather than an error*. A parse
that used to fail loudly may now continue with nothing in place of the
resource. The library offers
{{-Dorg.apache.commons.xml.secure.throwOnUnresolved=true}} to reject instead,
but it is JVM-global, so Camel cannot set it on a user's behalf.
* Reading a security feature back off a factory no longer tells you whether the
parser will fetch, because the blocking moved beneath the flag. Assertions must
move from factory configuration to parse behaviour.
h2. Why this is the right direction
The prevailing philosophy in JAXP is _allowed unless forbidden_, and every
forbidding knob is optional, implementation-specific and easy to forget. The
floor inverts it to
*forbidden unless allowed*, which is the property that would have prevented the
four CVEs above: a new consumer, converter or data format is secure because it
did nothing, rather than insecure because its author missed a flag.
For deployments that need a real external-resource policy rather than an
all-or-nothing switch, [XML Resolver|https://www.xmlresolver.org/index.html] is
a good companion: it implements OASIS catalogs and can be installed as the
delegate in front of the floor, giving a curated allow-list instead of a
blanket opt-in.
h1. Scope
One sub-task per JAXP factory. Listed in increasing order of effort rather than
in the order of the JAXP API, because the later ones depend on decisions taken
in the earlier.
h2. 1. SAX
The easiest, and the one with no user-facing decision left to take.
The 4.23 upgrade guide (_camel-core - XmlConverter SAX parser factory_) already
documents the escape hatch, and it is not a feature flag:
{quote}Routes that genuinely need to resolve an external DTD or parameter
entity through this converter must supply their own {{SAXParserFactory}}.{quote}
That statement stays true verbatim under the floor, so no new escape hatch and
no documentation change is required. The work is:
* {{SAXParserFactory.newInstance()}} ->
{{SecureSAXParserFactory.newNSInstance()}} (the {{NS}} variant folds in the
existing {{setNamespaceAware(true)}}).
* Delete {{FEATURE_SECURE_PROCESSING}}, {{external-general-entities}},
{{external-parameter-entities}} and {{nonvalidating/load-external-dtd}},
together with the {{try}} / {{catch}} scaffolding around them.
{{disallow-doctype-decl}} is deliberately not set on the SAX path today, and
does not need to be: the floor makes a DOCTYPE harmless without rejecting the
document.
Sites: {{XmlConverter.createSAXParserFactory()}}, {{XmlLineNumberParser}},
{{XmlPrettyPrinter}}, {{RouteCoverageXmlParser}},
{{SchematronProcessorFactory}}, {{DefaultBulkApiClient}}.
h2. 2. DOM
Harder, because the documented escape hatch _is_ a feature flag.
The 4.23 upgrade guide (_camel-xpath_, from CAMEL-24475) tells users that
Camel's {{DocumentBuilderFactory}} "disallows a {{DOCTYPE}} declaration and
does not resolve external entities", that such a document is "rejected with a
{{SAXParseException}}", and that the way to parse one anyway is:
{code:none}
-Dorg.apache.camel.xmlconverter.documentBuilderFactory.feature:http://apache.org/xml/features/disallow-doctype-decl=false
{code}
Under the floor that property still relaxes the DOCTYPE check, but it no longer
restores external entity resolution, because the fetch is blocked one layer
below the flag. The escape hatch therefore has to be restated as either:
* install an {{EntityResolver}} on the {{DocumentBuilder}} that allow-lists
what the deployment genuinely needs, or
* supply your own {{DocumentBuilderFactory}}, matching what SAX already says.
*Interaction with CAMEL-23878.* That issue asks for {{createSAXParserFactory}}
to be aligned with {{createDocumentBuilderFactory}} by adding
{{disallow-doctype-decl=true}} and {{external-parameter-entities=false}} to the
SAX path, and notes that code relying on DOCTYPE handling through SAX
conversion would break. With a resolver floor the cheaper convergence runs the
other way: *let both paths accept a DOCTYPE and let the floor neutralise its
external references*, rather than teaching SAX to reject documents it accepts
today. That closes CAMEL-23878 by making the two paths agree, with no breakage
on either side.
The counter-argument has to be recorded honestly: dropping
{{disallow-doctype-decl}} from the DOM path relaxes a default that the 4.23
upgrade guide promised in the same release, and the threat model's committer
checklist requires an upgrade-guide entry plus PMC review for exactly that. It
is a decision for the PMC, not an implementation detail. Measured behaviour to
inform it: with the floor in place and the DOCTYPE permitted, a document
carrying {{<!ENTITY xxe SYSTEM "file:///...">}} parses and the entity expands
to *empty* on DOM, SAX and StAX alike; nothing is read.
Sites: {{XmlConverter.createDocumentBuilderFactory()}}, {{XMLConverterHelper}},
{{XmlLineNumberParser}}, {{BaseParser}}, {{RouteCoverageXmlParser}},
{{ProcessorDetailDevConsole}}, {{FlatpackConverter}},
{{CMSenderOneMessageImpl}}, {{SaxonXmlSourceHandlerFactoryImpl}} (currently
unhardened).
h2. 3. StAX
Little work. {{XMLInputFactory.newInstance()}} ->
{{SecureXMLInputFactory.newFactory()}}.
The properties currently set can be reclassified rather than simply deleted:
* {{SUPPORT_DTD}} and {{IS_SUPPORTING_EXTERNAL_ENTITIES}} become
*optimisations*. The floor would catch these references anyway; switching them
off stops the parser from walking a DTD it is not allowed to act on. Keeping
them is cheap and shortens the path.
* {{IS_REPLACING_ENTITY_REFERENCES}} is *not a security property* at all. It
selects whether entity references are reported as events or replaced inline,
and should be documented as a parsing choice rather than left sitting in a
hardening block.
{{StaxConverter}} also installs an {{XMLResolver}} that throws {{"Reading
external entities is disabled"}}. Under the floor it becomes the delegate and
still throws, which is how the loud failure mode is preserved against the
library's silent-empty default. Worth keeping for that reason, though note
{{SUPPORT_DTD=false}} pre-empts it for DOCTYPE-bearing payloads, so it is
reached less often than it appears.
{{XMLOutputFactory}} is unaffected; it writes.
Sites: {{StaxConverter}}, {{XmlStreamDetector}}, {{UblDataFormat}},
{{StAXJAXBIteratorExpression}}.
h2. 4. Validation
Camel already has the concept this needs: the global option
{{CamelXmlValidatorAccessExternalDTD}} ({{SchemaReader.ACCESS_EXTERNAL_DTD}},
CAMEL-10894), which today skips the {{ACCESS_EXTERNAL_DTD}} /
{{ACCESS_EXTERNAL_SCHEMA}} properties on the {{SchemaFactory}} and the
{{Validator}}. Re-express it as a resolver:
* {{SchemaFactory.newInstance(lang)}} ->
{{SecureSchemaFactory.newInstance(lang)}}.
* When the option is off, install only the resolver the endpoint is configured
with. The default {{DefaultLSResourceResolver}} already serves {{xs:import}} /
{{xs:include}} relative to the endpoint's resource URI, and continues to do so
as the floor's delegate, so multi-file schemas are unaffected.
* When the option is on, chain an allow-all {{LSResourceResolver}} _after_ the
configured one, so a schema that resolves locally still does, and anything else
is opted in.
Two refinements worth considering while the code is open:
* Split the single switch into {{CamelXmlValidatorAccessExternalDTD}} and a new
{{CamelXmlValidatorAccessExternalSchema}}, mirroring the JAXP distinction
between {{ACCESS_EXTERNAL_DTD}} and {{ACCESS_EXTERNAL_SCHEMA}}. Today one
option controls both, which forces a deployment that needs a remote schema to
also accept remote DTDs.
* The allow-all resolver must return an {{LSInput}} carrying *identifiers
only*, with no stream attached, so the parser opens the resource itself.
Opening it inside the resolver forces the checked {{IOException}} to be
swallowed (the interface declares none), which converts a real fetch failure
into silent empty content. This is not theoretical: it is what
{{ValidatorDtdAccessOnTest}} detects, because it asserts on the
{{UnknownHostException}} reaching the route.
{{ValidatingProcessor}} should keep setting {{ACCESS_EXTERNAL_*}} on the
non-opt-in path: the {{Schema}} may come from a user-injected
{{SchemaFactory}}, which Camel cannot secure, and that path would otherwise
lose its only protection.
Sites: {{SchemaReader}}, {{ValidatingProcessor}}, {{XmlSignatureProcessor}},
{{JaxbDataFormat}} (see _Follow-ups_).
h2. 5. TrAX
The most wiring, because the protection works differently here.
Commons Secure XML secures a transform by *replacing every parser-less source
with one it owns*: a {{StreamSource}}, or a {{SAXSource}} whose
{{getXMLReader()}} returns null, is rewritten to carry a secure {{XMLReader}}
before the implementation ever sees it. A {{DOMSource}}, or a {{SAXSource}}
that already carries the caller's own reader, is passed through untouched, on
the assumption that the caller has taken responsibility for it. The same
rewrite is applied to sources handed back by a {{URIResolver}}, so an opted-in
{{xsl:include}} is parsed on the same floor as everything else.
Camel can and should do the same thing for the sources it constructs itself, so
that a body converted to a {{Source}} by the type converter arrives at the
transform already carrying a secure reader rather than relying on the TrAX
implementation to add one: {{XmlConverter.toSAXSource}} / {{toStreamSource}}
and {{XsltBuilder}} are the places.
Notes:
* {{XsltUriResolver}} already serves {{xsl:include}}, {{xsl:import}} and
{{document()}} over {{classpath:}}, {{file:}}, {{http:}}, {{ref:}} and
{{bean:}}, and keeps working as the floor's delegate.
* A top-level URI the caller passes is fetched as-is by the implementation;
only follow-up references go through the floor. Camel already resolves the
top-level stylesheet itself through {{XsltUriResolver}}
({{XsltEndpoint.loadResource}}), so the opt-in policy applied to the main
stylesheet must be applied to its includes too. That is what the existing
resolver does, and it is the property to protect in review.
* The guarantees govern what a transform reads, not what it writes.
{{xsl:result-document}} still writes wherever the stylesheet says, so an
untrusted stylesheet remains a route-author concern.
* {{XmlConverter.configureSaxonTransformerFactory}} and
{{StaxConverter.isWoodstox}} both identify the implementation by
{{factory.getClass().getName()}}, which becomes the wrapper. Both need
restructuring: probe the classpath and let {{setAttribute}} reject a non-Saxon
factory (attributes are delegated, so a real Saxon still gets configured), and
resolve the StAX implementation once from a plain factory.
Sites: {{XmlConverter.createTransformerFactory()}}, {{XMLConverterHelper}},
{{XsltBuilder}}, {{CachedCxfPayload}}, {{SpringWebserviceHelper}},
{{TikaProducer}}, {{FopProducer}}, {{RouteCoverageMojo}}.
h1. Threat model
{{security-model.adoc}} must gain a list of the settings that *invalidate the
guarantees*, alongside the existing XXE and SSRF sections. A guarantee that can
be switched off silently is not a guarantee, and these are the switches:
|| Setting || Effect ||
| {{org.apache.camel.xmlconverter.documentBuilderFactory.feature:<uri>}} with
{{external-general-entities}}, {{external-parameter-entities}} or
{{load-external-dtd}} set to {{true}} | re-enables external entity resolution
on the DOM path |
| {{org.apache.camel.xmlconverter.documentBuilderFactory.feature:<uri>}} with
{{disallow-doctype-decl}} set to {{false}} | permits a DOCTYPE; external
references stay blocked unless a resolver opts them in |
| {{CamelXmlValidatorAccessExternalDTD}} (and a future
{{CamelXmlValidatorAccessExternalSchema}}) | allows the validator to fetch
external DTDs and schemas |
| a caller-supplied {{DocumentBuilderFactory}}, {{TransformerFactory}} or
{{SchemaFactory}} ({{XmlConverter.setDocumentBuilderFactory}},
{{validator:...?schemaFactory=#ref}}, the {{CamelDocumentBuilderFactory}}
exchange property) | bypasses the floor entirely; the library exposes no way to
wrap an existing instance |
| a caller-installed {{EntityResolver}} / {{URIResolver}} /
{{LSResourceResolver}} / {{XMLResolver}} | opts in whatever it resolves; the
floor only covers what the resolver declines |
The last two are the important ones, because they are the extension points the
framework offers by design. The threat model should state plainly that a
factory or resolver supplied by the deployment is the deployment's
responsibility, which is consistent with operators and route authors being
trusted, but is currently implicit.
h1. Dependency
{{org.apache.commons:commons-secure-xml:1.0.0}}, released to Maven Central in
September 2026. Java 8 target, {{Multi-Release}} jar, OSGi bundle
{{org.apache.commons.xml.secure}}, Apache 2.0. It has *no transitive compile
dependencies*; Saxon-HE is {{provided}} and imported optionally. Managed in
{{parent/pom.xml}} as {{commons-secure-xml-version}}; {{camel-dependencies}}
picks the property up automatically and neither the BOM nor the catalog needs
an edit.
This would be the first third-party compile dependency in
{{core/camel-xml-jaxp}}. The whole {{core/}} tree currently carries four
(slf4j-api, jspecify, JAXB, Jackson-YAML), so the addition needs justification
on the record: it removes roughly 120 lines of security code whose divergence
is the direct cause of four accepted CVEs, and replaces it with a guarantee
that does not depend on which parser the deployment happens to ship.
h1. Follow-ups, deliberately out of scope
* *Saxon-selecting components*: {{camel-xslt-saxon}}, {{camel-xj}},
{{camel-schematron}} and {{camel-xpath}}'s Saxon path all promise a specific
implementation class ({{transformerFactoryClass}}, {{saxon=true}},
{{objectModelUri}}) and then act on {{instanceof}} or a class-name check. A
wrapper makes those checks fail *silently*, so {{saxonConfiguration}},
{{saxonExtensionFunctions}}, {{secureProcessing}} and Saxon's {{linenumbering}}
attribute would stop being applied with no error. These stay on their current
hardening until Commons Secure XML offers a public {{wrap(TransformerFactory)}}
/ {{unwrap()}} pair, which is worth raising upstream as a feature request.
* *camel-jaxb* compiles multi-file schemas with no {{LSResourceResolver}},
relying on its {{accessExternalSchemaProtocols}} option. Securing it before
giving it a resolver would turn "schema fails to load loudly" into "schema
silently validates nothing", which is worse than the current behaviour. It
needs the resolver first.
* *camel-jbang, camel-kamelet-main, tooling*: three copied private
secure-factory helpers, all missing {{disallow-doctype-decl}} and all
swallowing exceptions, parsing POMs fetched from remote repositories, POMs from
arbitrary third-party jars, and MCP-client-supplied strings. The least-trusted
input in the repository, and the cleanest win once the core pattern is settled.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
