ppkarwasz commented on PR #4366: URL: https://github.com/apache/logging-log4j2/pull/4366#issuecomment-5906292315
Hi @jmestwa-coder, Thanks for the PR. You are right: the schema is parsed by a factory that resolves external entities. However, we would rather reduce the amount of security-related code in the XML configuration than add to it. The configuration file is a trusted input, so resolving external resources from it is not a vulnerability. The current DOM hardening was added in [LOG4J2-1959](https://issues.apache.org/jira/browse/LOG4J2-1959) "just in case" (see the comments there), without a threat model behind it. Since then, it has broken legitimate setups (see the entity-based configuration reuse reported in the same issue), needed fixes like [LOG4J2-3531](https://issues.apache.org/jira/browse/LOG4J2-3531), and it is still incomplete: `FEATURE_SECURE_PROCESSING` is not set, and schemas and XInclude can fetch external resources anyway. Every patch like this one invites the next. We keep some settings only to save reporters the time of finding and discarding the false positive. Instead of tuning them by hand, #4162 replaces all of this with [Commons Secure XML](https://commons.apache.org/proper/commons-secure-xml/). External resources will then be loaded through `ConfigurationSource`, restricted only by [`log4j2.configurationAllowedProtocols`](https://logging.apache.org/log4j/2.x/manual/systemproperties.html#log4j2.configurationAllowedProtocols). That filter needs some work of its own, for example, to recognize that `jar:http:` uses HTTP and to always allow `classpath:` resources without requiring the user to whitelist each protocol used in the classpath. So I would rather not merge this and fix the schema path as part of #4162. -- 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]
