Make XInclude opt-in and rework XML configuration hardening

[ppkarwasz]

Fixes #4064.

Summary

This PR makes XInclude support in XML configurations opt-in and reworks the XML hardening, addressing the security-hardening request in #4064.

1. XInclude is now disabled by default

XInclude processing is no longer enabled unconditionally. It can be turned on with the log4j2.configurationEnableXInclude property. Since enabling XInclude is now an explicit user choice, a failure to activate setXIncludeAware is no longer silently ignored, and XML parse errors now propagate as a ConfigurationException instead of leaving a half-built configuration.

2. Hardening delegated to copernik-xml-factory

The hand-rolled JAXP hardening code is replaced with the eu.copernik:copernik-xml-factory library (an incubating project intended for Apache Commons or Xerces). It applies the security features supported by the actual factory implementation rather than swallowing the ones it does not support, and provides Android support out of the box.

3. XInclude/entity resolution through ConfigurationSource

A custom EntityResolver based on ConfigurationSource.fromUri resolves external entities and XInclude resources the same way the configuration itself is resolved. As a result, includes now benefit from Log4j URI conventions, in particular the classpath: scheme (for example, to pull in a default configuration shipped on the classpath, as Spring Boot does), which was not possible before, and they honor the log4j2.configurationAllowedProtocols restrictions.

Notes

The XmlConfigurationSecurity test was being silently skipped by Surefire because its class name did not end in Test; it is renamed to XmlConfigurationSecurityTest and updated to the new behavior.

Checklist

• Base your changes on 2.x branch
• ./mvnw verify succeeds
• Non-trivial changes contain an entry file in the src/changelog/.2.x.x directory
• Tests are provided

[vy]

The hand-rolled JAXP hardening code is replaced with the eu.copernik:copernik-xml-factory library

@rgoers, @garydgregory, do you agree with adding this external dependency to log4j-core? As we discussed this earlier, I'm -0 on this change, because:

• We will effective be replacing 10 LoC (which is working and bug-free) in log4j-core with an external dependency.
• This doesn't really bring any compelling benefit either to users, or to maintainers.
• Adding a new dependency for a feature that is used by a majority of the user base (i.e., XML-based configuration) can break fat-JAR setups.

[Copilot]

Pull request overview

This PR hardens Log4j2’s XML configuration parsing by making XInclude processing opt-in, delegating XML parser hardening to eu.copernik:copernik-xml-factory, and resolving XInclude/external entities via ConfigurationSource to enforce Log4j URI conventions and allowed-protocol restrictions.

Changes:

• Disable XInclude by default; enable explicitly via log4j2.configurationEnableXInclude, and propagate XML parse failures as ConfigurationException.
• Replace in-house JAXP hardening with copernik-xml-factory-provided hardened factories.
• Add/adjust tests and resources to validate XInclude behavior, classpath includes, and security restrictions; update OSGi tests for the new bundle.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/changelog/.2.x.x/4064_xinclude_resolver.xml	Changelog entry for resolving XIncludes via ConfigurationSource.
src/changelog/.2.x.x/4064_xinclude_opt_in.xml	Changelog entry documenting XInclude opt-in property.
log4j-parent/pom.xml	Adds dependency management entry for copernik-xml-factory.
log4j-core/pom.xml	Adds copernik-xml-factory dependency to core.
log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/DisruptorTest.java	Includes eu.copernik.xml.factory bundle in Pax Exam setup.
log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/CoreOsgiTest.java	Includes eu.copernik.xml.factory bundle in Pax Exam setup.
log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/AbstractLoadBundleTest.java	Installs/starts/stops the XML factory bundle alongside API/Core.
log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java	Implements opt-in XInclude + hardened factory usage + ConfigurationSource-based entity resolver + parse error propagation.
log4j-core-test/src/test/resources/XmlConfigurationSecurity/xinclude.xml	Security test resource exercising forbidden protocol in XInclude.
log4j-core-test/src/test/resources/XmlConfigurationSecurity/external-parameter-entity.xml	Security test resource exercising forbidden external entity fetch.
log4j-core-test/src/test/resources/log4j-xinclude-classpath.xml	Test resource verifying classpath: XInclude resolution.
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationXIncludeTest.java	Unit tests for XInclude enabled/disabled and classpath: includes.
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurityTest.java	Renamed/updated security tests validating failure behavior and protocol restrictions.
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurity.java	Removes old misnamed test class that Surefire skipped.
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java	Ensures XInclude integration test enables XInclude via property.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ppkarwasz]

Hi @vy,

• Adding a new dependency for a feature that is used by a majority of the user base (i.e., XML-based configuration) can break fat-JAR setups.

I do not yet see the failure mode. An uber-JAR absorbs its dependencies by design, so one more compile-scope dependency is shaded in like any other.

• We will effective be replacing 10 LoC (which is working and bug-free) in log4j-core with an external dependency.
• This doesn't really bring any compelling benefit either to users, or to maintainers.

I would push back on "bug-free". The current block:

logging-log4j2/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/XmlConfiguration.java

Lines 191 to 197 in 6beea3f

	private static void disableDtdProcessing(final DocumentBuilderFactory factory) {
	factory.setValidating(false);
	factory.setExpandEntityReferences(false);
	setFeature(factory, "http://xml.org/sax/features/external-general-entities", false);
	setFeature(factory, "http://xml.org/sax/features/external-parameter-entities", false);
	setFeature(factory, "http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
	}

blocks external fetches only if the factory supports the features we set, and swallows the exception when it does not, so on an unsupported parser we silently get no hardening and never learn about it. Two more things are hard to justify:

• we disable external entities but never enable FEATURE_SECURE_PROCESSING (Billion Laughs stays open), and
• we set setExpandEntityReferences(false), which has nothing to do with external fetching and at worst changes our DOM, since entities then arrive as Entity rather than Text nodes.

These are all hardenings (the configuration is trusted), but in this case we should choose whether we want to make the parser safe from known XML-based attacks or don't do anything at all.

The benefits for maintainers:

• Independence from the concrete DocumentBuilderFactory chosen by ServiceLoader at runtime: the library applies the features each implementation actually supports, not a fixed list.
• A DocumentBuilder protected against Billion Laughs and XXE/SSRF by library contract, with the entity-attack tests maintained upstream rather than here.
• One audited place to point SAST tools and AI reviewers at. This does not make findings vanish on its own (as the CodeQL comment Make XInclude opt-in and rework XML configuration hardening #4144 (comment) shows), but the rebuttal and any suppression then have a single home instead of being re-argued per call site.

If consensus is that the dependency is not worth it I will keep the current code, but "10 bug-free lines" is not the right baseline.

[ppkarwasz]

BTW: I like Ceki's perspective on XML features. If a feature required by security is absent, parsing throws (see LOGBACK-1577).

For users of the Oracle XML Parser library, our hardenings are apparently not effective.