KNOX-3484 - Use Apache Commons Secure XML for all JAXP factories

[ppkarwasz]

KNOX-3484 - Use Apache Commons Secure XML for all JAXP factories

What changes were proposed in this pull request?

Knox configured its XML parsers by hand: each call site set its own subset of features and properties, and some relied on the defaults of whichever implementation (JDK, Woodstox, MOXy, Glassfish JAXB, Commons Digester) served them. This PR replaces those ad hoc settings with one dependency, Apache Commons Secure XML 1.0.0, whose factories are hardened as delivered: external resources (DTDs, external entities, XInclude, schemas, stylesheets) are ignored, independently of the JAXP implementation.

This is a hardening and maintenance change: the PMC no longer has to configure XML parsers.

• DOM, TrAX, XPath, Validation: XmlUtils, UrlRewriteRulesDescriptorAdapter, XmlFilterReader, ServiceURLPropertyConfig, TopologyValidator and TopologyToDescriptor obtain their factories from SecureDocumentBuilderFactory, SecureTransformerFactory, SecureXPathFactory and SecureSchemaFactory. The hand-written feature settings are removed.

• StAX: XmlFilterReader uses SecureXMLInputFactory.

• JAXB: ServiceDefinitionUnmarshaller, TopologyMarshaller (XML bodies), ServiceDefinitionsLoader, ApplicationDeploymentContributor, TopologyToDescriptor, RemoteConfigurationRegistriesParser and ProviderConfigurationParser unmarshal through two new helpers, XmlUtils.unmarshal(Unmarshaller, Class<T>, File) and XmlUtils.unmarshal(Unmarshaller, Class<T>, String, InputStream), which unmarshal from an XMLEventReader created by SecureXMLInputFactory. This follows the StAX approach documented for JAXB in Document JAXB commons-secure-xml#103, and makes the result independent of the JAXB provider serving each context.

• TopologyMarshaller: selects the XML or JSON reader by exact media type (application/xml or application/json, parameters ignored) and passes MOXy a fixed media type instead of the request's. Other types, including wildcards, are rejected with HTTP 415.

• Commons Digester: TopologyUtils, XmlGatewayDescriptorImporter and XmlUrlRewriteRulesImporter pass Digester an XMLReader created by SecureSAXParserFactory. Since Digester 3.2 resolves any absolute system id by itself, each site also installs a resolver that returns null, leaving external references to Commons Secure XML.

• Enforcement: a new forbidden-apis execution (build-tools/forbiddenapis/xml-signatures.txt, main code only) rejects the plain JAXP factory methods, the Unmarshaller.unmarshal overloads that let the JAXB provider create its own parser (File, InputStream, Reader, URL, InputSource, Source) and the Digester constructors that create their own parser. TopologyMarshaller is excluded from this check in gateway-service-admin/pom.xml, because MOXy parses its JSON request bodies with its own JSON reader. On the current master it reports 23 violations in 9 modules, for example:

  [ERROR] Forbidden method invocation: javax.xml.parsers.DocumentBuilderFactory#newInstance() [Use SecureDocumentBuilderFactory from Apache Commons Secure XML]
  [ERROR]   in org.apache.knox.gateway.util.XmlUtils (XmlUtils.java:47)
  [ERROR] Forbidden method invocation: javax.xml.stream.XMLInputFactory#newFactory() [Use SecureXMLInputFactory from Apache Commons Secure XML]
  [ERROR]   in org.apache.knox.gateway.filter.rewrite.impl.xml.XmlFilterReader (XmlFilterReader.java:97)
  [ERROR] Forbidden method invocation: org.apache.commons.digester3.binder.DigesterLoader#newDigester() [Pass an XMLReader created by SecureSAXParserFactory]
  [ERROR]   in org.apache.knox.gateway.util.TopologyUtils (TopologyUtils.java:41)
  

Behavior change: documents containing a DOCTYPE are now accepted everywhere. External references are ignored and internal entities are expanded within the limits of secure processing. Previously XmlUtils rejected any DOCTYPE and the StAX readers disabled DTD support.

Minor fixes in touched code: XmlUtils.readXml(File) and ProviderConfigurationParser.parseXML(File) no longer leak their input stream, and ServiceDefinitionsLoader no longer opens a stream it does not read.

How was this patch tested?

• mvn install on the affected modules (gateway-util-common, gateway-spi, gateway-service-admin, gateway-provider-rewrite, gateway-provider-rewrite-common, gateway-server, gateway-discovery-ambari, gateway-service-remoteconfig, gateway-topology-simple) and their dependencies: all unit tests, checkstyle, forbidden-apis and dependency analysis pass.
• GatewayAdminTopologyFuncTest: 32/32 pass.
• No new tests are added, since the guarantees are the library's and are tested there. The existing KNOX-1308 and KNOX-3447 tests (GatewayAdminTopologyFuncTest#testPutTopologyWithEntityExpansion, ServiceDefinitionUnmarshallerTest#testEntityExpansionIsNotResolved) asserted that internal entities are not expanded; they now assert that the content of an external entity does not leak. Reverting ServiceDefinitionUnmarshaller to a plain XMLInputFactory makes both unit tests fail.

Integration Tests

No integration tests are added or changed: the change does not alter any Knox feature, and the existing suites cover the affected parsing paths.

🤖 Generated with Claude Code

[github-actions]

Test Results

  4 files    4 suites   41s ⏱️
127 tests 127 ✅ 0 💤 0 ❌
145 runs  145 ✅ 0 💤 0 ❌

Results for commit 3ef1b8a.

♻️ This comment has been updated with latest results.