[
https://issues.apache.org/jira/browse/KNOX-3484?focusedWorklogId=1043812&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1043812
]
ASF GitHub Bot logged work on KNOX-3484:
----------------------------------------
Author: ASF GitHub Bot
Created on: 24/Sep/26 21:34
Start Date: 24/Sep/26 21:34
Worklog Time Spent: 10m
Work Description: github-actions[bot] commented on PR #1427:
URL: https://github.com/apache/knox/pull/1427#issuecomment-5822611066
## Test Results
4 files 4 suites 41s ⏱️
127 tests 127 ✅ 0 💤 0 ❌
145 runs 145 ✅ 0 💤 0 ❌
Results for commit a3177f09.
[test-results]:data:application/gzip;base64,H4sIAEeXtWoC/13MSw7CIBSF4a00jB1chELqZgzP5Ma2GAqjxr1LSUV0+P0nOTvxOLuN3AZ+GciWMTXYHFXCsB6kxWVJx0av8qP7lo35Tw98lgQteIXzT3AxhniWmNf6yccT7bIr38fq7rC6/zNhWTAVEMWolB4mbUECAyuYM8JaAYqLcfJUC8OoNoy83ldcXP4EAQAA
Issue Time Tracking
-------------------
Worklog Id: (was: 1043812)
Time Spent: 20m (was: 10m)
> 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
> Priority: Minor
> Time Spent: 20m
> Remaining Estimate: 0h
>
> 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)
