ppkarwasz opened a new pull request, #1427: URL: https://github.com/apache/knox/pull/1427
[KNOX-3484](https://issues.apache.org/jira/browse/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](https://commons.apache.org/proper/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 apache/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](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
