Make XInclude opt-in and rework XML configuration hardening

log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java

@@ -42,6 +42,7 @@
 import org.apache.logging.log4j.core.config.composite.CompositeConfiguration;
 import org.apache.logging.log4j.core.filter.ThreadContextMapFilter;
 import org.apache.logging.log4j.core.test.junit.LoggerContextSource;
+import org.apache.logging.log4j.test.junit.SetTestProperty;
 import org.apache.logging.log4j.test.junit.TempLoggingDir;
 import org.apache.logging.log4j.util.Strings;
 import org.junit.jupiter.api.Tag;
@@ -103,6 +104,7 @@ void xml(final LoggerContext context) throws IOException {
     }
 
     @Test
+    @SetTestProperty(key = "log4j2.configurationEnableXInclude", value = "true")
     @LoggerContextSource("log4j-xinclude.xml")
     void xinclude(final LoggerContext context) throws IOException {
         checkConfiguration(context);

log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationSecurityTest.java

@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.config.xml;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.net.URI;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.ConfigurationException;
+import org.apache.logging.log4j.core.config.ConfigurationFactory;
+import org.apache.logging.log4j.test.junit.SetTestProperty;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+/**
+ * Verifies the contract of {@link XmlConfiguration} regarding the resolution of external resources.
+ *
+ * <p>Configuration files are part of the trusted computing base (see the
+ * <a href="https://logging.apache.org/security.html">Log4j security policy</a>); the hardenings checked here are
+ * defense in depth, not a security boundary against a malicious configuration. These tests only cover what Log4j is
+ * responsible for:</p>
+ * <ul>
+ *     <li>a forbidden external fetch is surfaced as a {@link ConfigurationException} (rather than a half-built
+ *         configuration), and</li>
+ *     <li>XInclude resources are subject to the same {@code ALLOWED_PROTOCOLS} restrictions as the configuration
+ *         file itself.</li>
+ * </ul>
+ *
+ * <p>All external fetches are forbidden; whether a forbidden fetch raises an error or is silently skipped is part of
+ * the contract of the {@code eu.copernik:copernik-xml-factory} library and is tested there.</p>
+ */
+@Tag("functional")
+@Tag("security")
+class XmlConfigurationSecurityTest {
+
+    private static final ConfigurationFactory FACTORY = new XmlConfigurationFactory();
+
+    private static Configuration getConfiguration(final String resource) {
+        return FACTORY.getConfiguration(null, null, URI.create("classpath:XmlConfigurationSecurity/" + resource));
+    }
+
+    /**
+     * A configuration that triggers a forbidden external fetch fails to load with a {@link ConfigurationException}.
+     * The {@code @Timeout} guards against an SSRF: a fetch attempt against the unreachable host would block until the
+     * connection times out.
+     */
+    @Test
+    @Timeout(5)
+    void forbiddenExternalFetchThrowsConfigurationException() {
+        assertThatThrownBy(() -> getConfiguration("external-parameter-entity.xml"))
+                .isInstanceOf(ConfigurationException.class);
+    }
+
+    /**
+     * XInclude resources are resolved through {@code ConfigurationSource}, so they are subject to the same
+     * {@code ALLOWED_PROTOCOLS} restrictions as the configuration file. With {@code http} excluded, an {@code http}
+     * include cannot be resolved and the configuration fails to load.
+     */
+    @Test
+    @Timeout(5)
+    @SetTestProperty(key = "log4j2.configurationEnableXInclude", value = "true")
+    @SetTestProperty(key = "log4j2.configurationAllowedProtocols", value = "file")
+    void xIncludeRespectsAllowedProtocols() {
+        assertThatThrownBy(() -> getConfiguration("xinclude.xml")).isInstanceOf(ConfigurationException.class);
+    }
+}

log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/xml/XmlConfigurationXIncludeTest.java

@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.core.config.xml;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
+import javax.xml.parsers.DocumentBuilder;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.apache.logging.log4j.core.config.Node;
+import org.junit.jupiter.api.Test;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.xml.sax.InputSource;
+
+/**
+ * Unit tests for the XInclude handling of {@link XmlConfiguration}.
+ * <p>
+ * The {@code parse}-based tests exercise the {@link DocumentBuilder} returned by
+ * {@link XmlConfiguration#newDocumentBuilder(boolean)} directly, so they are independent of the
+ * {@code log4j2.configurationEnableXInclude} property plumbing covered by the integration tests in
+ * {@code ConfigurationFactoryTest}. {@link #fixupAttributesAreStripped()} drives {@link XmlConfiguration#constructHierarchy}
+ * to verify that the {@code xml:base}/{@code xml:lang} attributes added by the XInclude fix-up features are not
+ * exposed as Log4j configuration attributes.
+ * </p>
+ */
+class XmlConfigurationXIncludeTest {
+
+    private Document parse(final boolean enableXInclude) throws Exception {
+        return parse("/log4j-xinclude.xml", enableXInclude);
+    }
+
+    private Document parse(final String resource, final boolean enableXInclude) throws Exception {
+        final URL url = getClass().getResource(resource);
+        final DocumentBuilder builder = XmlConfiguration.newDocumentBuilder(enableXInclude);
+        try (final java.io.InputStream is = url.openStream()) {
+            final InputSource source = new InputSource(is);
+            // Required so that relative `href` attributes can be resolved against the configuration location.
+            source.setSystemId(url.toExternalForm());
+            return builder.parse(source);
+        }
+    }
+
+    @Test
+    void xInclude_enabled_resolves_includes() throws Exception {
+        final Document document = parse(true);
+        // The `xi:include` elements have been replaced by the content of the included files.
+        assertEquals(0, document.getElementsByTagNameNS("*", "include").getLength(), "no `xi:include` should remain");
+        assertEquals(1, document.getElementsByTagName("Console").getLength(), "Console appender from include");
+        assertEquals(1, document.getElementsByTagName("File").getLength(), "File appender from include");
+        assertEquals(1, document.getElementsByTagName("List").getLength(), "List appender from include");
+        assertEquals(2, document.getElementsByTagName("Logger").getLength(), "Loggers from include");
+    }
+
+    @Test
+    void xInclude_disabled_keeps_includes_unresolved() throws Exception {
+        final Document document = parse(false);
+        // Without XInclude support the `xi:include` elements are left untouched and nothing is included.
+        assertEquals(2, document.getElementsByTagNameNS("*", "include").getLength(), "`xi:include` elements remain");
+        assertEquals(0, document.getElementsByTagName("Console").getLength(), "no appenders should be included");
+    }
+
+    @Test
+    void xInclude_resolves_classpath_scheme() throws Exception {
+        // The custom resolver delegates to `ConfigurationSource`, which understands the `classpath:` URI scheme.
+        final Document document = parse("/log4j-xinclude-classpath.xml", true);
+        assertEquals(0, document.getElementsByTagNameNS("*", "include").getLength(), "no `xi:include` should remain");
+        assertEquals(
+                1, document.getElementsByTagName("Console").getLength(), "Console appender from classpath include");
+        assertEquals(2, document.getElementsByTagName("Logger").getLength(), "Loggers from classpath include");
+    }
+
+    /**
+     * The XInclude {@code fixup-base-uris} and {@code fixup-language} features (both enabled by default) add
+     * {@code xml:base} and {@code xml:lang} attributes to the top-level included elements. Those belong to the
+     * reserved XML namespace and must not be exposed as Log4j configuration attributes.
+     */
+    @Test
+    void fixupAttributesAreStripped() throws Exception {
+        // `log4j-xinclude-fixup.xml` carries `xml:lang` on the root and includes a `<Console>` appender, so the
+        // included element receives both `xml:base` (from `fixup-base-uris`) and `xml:lang` (from `fixup-language`).
+        final Element rootElement = parse("/log4j-xinclude-fixup.xml", true).getDocumentElement();
+
+        final ConfigurationSource source = ConfigurationSource.fromResource(
+                "log4j-xinclude-fixup.xml", getClass().getClassLoader());
+        final XmlConfiguration configuration = new XmlConfiguration(new LoggerContext("test"), source);
+        // `constructHierarchy` resolves child elements to plugin types, so the plugins must be collected first.
+        configuration.getPluginManager().collectPlugins();
+
+        final Node root = new Node();
+        configuration.constructHierarchy(root, rootElement);
+
+        // Sanity check: the include was resolved and the `<Console>` node is present in the tree.
+        assertThat(collectNodeNames(root, new ArrayList<>())).contains("Console");
+        // None of the nodes carries an `xml:`-namespaced attribute.
+        assertThat(collectXmlNamespaceAttributes(root, new ArrayList<>())).isEmpty();
+    }
+
+    private static List<String> collectNodeNames(final Node node, final List<String> names) {
+        names.add(node.getName());
+        node.getChildren().forEach(child -> collectNodeNames(child, names));
+        return names;
+    }
+
+    private static List<String> collectXmlNamespaceAttributes(final Node node, final List<String> found) {
+        node.getAttributes().keySet().stream()
+                .filter(key -> key.startsWith("xml:"))
+                .forEach(found::add);
+        node.getChildren().forEach(child -> collectXmlNamespaceAttributes(child, found));
+        return found;
+    }
+}

log4j-core-test/src/test/resources/XmlConfigurationSecurity/xinclude.xml

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one or more
+  ~ contributor license agreements.  See the NOTICE file distributed with
+  ~ this work for additional information regarding copyright ownership.
+  ~ The ASF licenses this file to you under the Apache License, Version 2.0
+  ~ (the "License"); you may not use this file except in compliance with
+  ~ the License.  You may obtain a copy of the License at
+  ~
+  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+<Configuration xmlns:xi="http://www.w3.org/2001/XInclude"
+  status="OFF" name="XmlConfigurationSecurity">
+  <Appenders>
+    <List name="list">
+      <PatternLayout pattern="%m%n"/>
+    </List>
+  </Appenders>
+  <!-- An `http` resource that `ALLOWED_PROTOCOLS` must reject. -->
+  <xi:include href="http://localhost:12345/evil3.xml"/>
+  <Loggers>
+    <Root level="info">
+      <AppenderRef ref="list"/>
+    </Root>
+  </Loggers>
+</Configuration>

log4j-core-test/src/test/resources/log4j-xinclude-classpath.xml

@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one or more
+  ~ contributor license agreements.  See the NOTICE file distributed with
+  ~ this work for additional information regarding copyright ownership.
+  ~ The ASF licenses this file to you under the Apache License, Version 2.0
+  ~ (the "License"); you may not use this file except in compliance with
+  ~ the License.  You may obtain a copy of the License at
+  ~
+  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+<Configuration xmlns:xi="http://www.w3.org/2001/XInclude"
+  status="OFF" name="XMLConfigTest">
+  <Properties>
+    <Property name="filename">${test:logging.path}/test-xinclude.log</Property>
+  </Properties>
+  <ThresholdFilter level="debug"/>
+  <xi:include href="classpath:log4j-xinclude-appenders.xml" />
+  <xi:include href="classpath:log4j-xinclude-loggers.xml" />
+</Configuration>

log4j-core-test/src/test/resources/log4j-xinclude-fixup-console.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one or more
+  ~ contributor license agreements.  See the NOTICE file distributed with
+  ~ this work for additional information regarding copyright ownership.
+  ~ The ASF licenses this file to you under the Apache License, Version 2.0
+  ~ (the "License"); you may not use this file except in compliance with
+  ~ the License.  You may obtain a copy of the License at
+  ~
+  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+<Console name="STDOUT">
+  <PatternLayout pattern="%m%n" />
+</Console>

log4j-core-test/src/test/resources/log4j-xinclude-fixup.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one or more
+  ~ contributor license agreements.  See the NOTICE file distributed with
+  ~ this work for additional information regarding copyright ownership.
+  ~ The ASF licenses this file to you under the Apache License, Version 2.0
+  ~ (the "License"); you may not use this file except in compliance with
+  ~ the License.  You may obtain a copy of the License at
+  ~
+  ~      http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+<Configuration xmlns:xi="http://www.w3.org/2001/XInclude"
+  xml:lang="en" status="OFF" name="XIncludeFixup">
+  <Appenders>
+    <xi:include href="log4j-xinclude-fixup-console.xml" />
+  </Appenders>
+  <Loggers>
+    <Root level="error">
+      <AppenderRef ref="STDOUT" />
+    </Root>
+  </Loggers>
+</Configuration>

log4j-core/pom.xml

@@ -157,6 +157,11 @@
       <artifactId>commons-csv</artifactId>
       <optional>true</optional>
     </dependency>
+    <!-- Create hardened JAXP factories -->
+    <dependency>
+      <groupId>eu.copernik</groupId>
+      <artifactId>copernik-xml-factory</artifactId>
+    </dependency>
     <!-- Alternative implementation of BlockingQueue using Conversant Disruptor for AsyncAppender -->
     <dependency>
       <groupId>com.conversantmedia</groupId>