[jira] [Created] (KNOX-3484) Use Apache Commons Secure XML for all JAXP factories

["Piotr Karwasz (Jira)" <ji...@apache.org>]

Piotr Karwasz created KNOX-3484:
-----------------------------------

             Summary: Use Apache Commons Secure XML for all JAXP factories
                 Key: KNOX-3484
                 URL: https://issues.apache.org/jira/browse/KNOX-3484
             Project: Apache Knox
          Issue Type: Improvement
            Reporter: Piotr Karwasz


Knox configures its XML parsers by hand. Every call site that creates a JAXP factory repeats its own subset of features and properties ({{{}disallow-doctype-decl{}}}, {{{}FEATURE_SECURE_PROCESSING{}}}, {{{}SUPPORT_DTD{}}}, {{{}isSupportingExternalEntities{}}}, {{{}ACCESS_EXTERNAL_*{}}}, ...), and the subsets differ from site to site. Some sites set nothing and rely on the defaults of whichever implementation (JDK, Woodstox, MOXy, Glassfish JAXB, Commons Digester) happens to serve them. Some ignore a failure to apply a setting.

Keeping this correct is a recurring cost for the PMC: every new parser has to remember the full list, several of the feature names are implementation-specific, and whether a given setting takes effect depends on which implementation serves which call site.

[Apache Commons Secure XML|https://commons.apache.org/proper/commons-secure-xml/] (1.0.0, Java 8+, no runtime dependencies, Apache-2.0) moves that work into one dependency. Its factories are hardened as delivered: external resources (DTDs, external entities, XInclude, schema imports, stylesheets) are ignored by non-removable resolvers, independently of the underlying JAXP implementation, and a factory that cannot be hardened fails loudly instead of silently. The guarantees are documented in its [threat model|https://commons.apache.org/proper/commons-secure-xml/threat_model.html] and tested against the JDK, Xerces, Woodstox and Saxon.

This is a hardening and maintenance change: it replaces the ad hoc configuration with one consistent dependency, so that the PMC no longer has to configure XML parsers.
h2. Current state
||Module||Class||API||Current configuration||
|gateway-util-common|{{XmlUtils}} ({{{}readXml{}}}, {{{}createDocument{}}}, {{{}getTransformer{}}})|DOM, TrAX|{{{}FEATURE_SECURE_PROCESSING{}}}, {{disallow-doctype-decl}}|
|gateway-service-admin|{{{}ServiceDefinitionUnmarshaller{}}}, {{TopologyMarshaller}}|StAX + JAXB|{{{}SUPPORT_DTD{}}}, {{{}isSupportingExternalEntities{}}}, {{isReplacingEntityReferences}}|
|gateway-provider-rewrite|{{XmlFilterReader}}|StAX, XPath|StAX: same three properties; XPath: {{{}FEATURE_SECURE_PROCESSING{}}}, failure ignored|
|gateway-discovery-ambari|{{ServiceURLPropertyConfig}}|XPath|none|
|gateway-server|{{TopologyValidator}}|Validation|{{ACCESS_EXTERNAL_*}} on the {{Validator}} only|
|gateway-spi|{{UrlRewriteRulesDescriptorAdapter}}|TrAX|none|
|gateway-server|{{{}ServiceDefinitionsLoader{}}}, {{{}ApplicationDeploymentContributor{}}}, {{TopologyToDescriptor}}|JAXB {{unmarshal(File/InputStream)}}|none (depends on the JAXB provider)|
|gateway-service-remoteconfig|{{RemoteConfigurationRegistriesParser}}|JAXB {{unmarshal(File)}}|none (depends on the JAXB provider)|
|gateway-topology-simple|{{ProviderConfigurationParser}}|JAXB {{unmarshal(InputStream)}}|none (depends on the JAXB provider)|
|gateway-server, gateway-provider-rewrite-common|{{{}TopologyUtils{}}}, {{{}XmlGatewayDescriptorImporter{}}}, {{XmlUrlRewriteRulesImporter}}|Commons Digester|none (depends on Digester's {{{}SAXParserFactory{}}})|
h2. Proposed changes

Add {{org.apache.commons:commons-secure-xml}} to the root {{dependencyManagement}} and to the modules above, then:
 * *DOM and TrAX:* {{XmlUtils}} and {{UrlRewriteRulesDescriptorAdapter}} obtain their factories from {{SecureDocumentBuilderFactory}} and {{{}SecureTransformerFactory{}}}. The security-motivated feature settings are removed; {{disallow-doctype-decl}} may be kept where Knox prefers to reject a DOCTYPE outright, as a behavioral choice rather than a security setting.
 * *StAX:* {{{}ServiceDefinitionUnmarshaller{}}}, {{TopologyMarshaller}} and {{XmlFilterReader}} obtain their {{XMLInputFactory}} from {{{}SecureXMLInputFactory.newFactory(){}}}. {{SUPPORT_DTD=false}} may likewise be kept as a behavioral choice: Commons Secure XML accepts settings that restrict further.
 * *XPath:* {{XmlFilterReader}} and {{ServiceURLPropertyConfig}} use {{{}SecureXPathFactory{}}}.
 * *Validation:* {{TopologyValidator}} uses {{{}SecureSchemaFactory{}}}.
 * *JAXB:* the {{unmarshal(File)}} and {{unmarshal(InputStream)}} calls are replaced with {{{}unmarshal(XMLEventReader){}}}, where the reader is created by {{{}SecureXMLInputFactory.newFactory().createXMLEventReader(...){}}}. The hardening then no longer depends on which JAXB provider (MOXy or the Glassfish runtime) serves a given context.
 * *Commons Digester:* the next Digester release depends on Commons Secure XML and hardens its parser by default, so upgrading Digester brings the same protection and the dependency comes at no extra cost. Until then, the three importers can pass a hardened reader explicitly: {{{}DigesterLoader.newDigester(SecureSAXParserFactory.newNSInstance().newSAXParser().getXMLReader(), rules){}}}.

h2. Enforcement

To keep raw factories from coming back, add a [forbidden-apis|https://github.com/policeman-tools/forbidden-apis] check with a signatures file that rejects the plain JAXP factory methods ({{{}DocumentBuilderFactory{}}}, {{{}SAXParserFactory{}}}, {{{}XMLInputFactory{}}}, {{{}TransformerFactory{}}}, {{{}SchemaFactory{}}}, {{{}XPathFactory{}}}) and the {{unmarshal(File)}} / {{unmarshal(InputStream)}} overloads of {{jakarta.xml.bind.Unmarshaller}} in main code.

No new XXE tests are needed: the guarantees are the library's and are tested there. The existing KNOX-1308 and KNOX-3447 tests should keep passing unchanged.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)