You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rg...@apache.org on 2022/02/17 05:40:56 UTC
[logging-log4j2] branch master updated: LOG4J2-3297 - Limit loading of configuration via a url to https by default
This is an automated email from the ASF dual-hosted git repository.
rgoers pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/master by this push:
new 2f615e8 LOG4J2-3297 - Limit loading of configuration via a url to https by default
2f615e8 is described below
commit 2f615e82e36087837c7f79aa91d6fc5e47d64abb
Author: Ralph Goers <rg...@apache.org>
AuthorDate: Wed Feb 16 22:40:48 2022 -0700
LOG4J2-3297 - Limit loading of configuration via a url to https by default
---
.../org/apache/logging/log4j/util/StringsTest.java | 10 ++
.../org/apache/logging/log4j/util/Strings.java | 2 +-
.../log4j/core/net/UrlConnectionFactoryTest.java | 177 +++++++++++++++++++++
.../logging/log4j/core/util/WatchHttpTest.java | 2 +
.../log4j/core/config/ConfigurationFactory.java | 6 +-
.../log4j/core/net/UrlConnectionFactory.java | 26 ++-
.../core/util/BasicAuthorizationProvider.java | 2 +-
src/changes/changes.xml | 3 +
src/site/asciidoc/manual/configuration.adoc | 53 ++++++
9 files changed, 276 insertions(+), 5 deletions(-)
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/StringsTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/StringsTest.java
index 78b797a..7c9695d 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/StringsTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/StringsTest.java
@@ -81,6 +81,16 @@ public class StringsTest {
}
@Test
+ public void splitList() {
+ String[] list = Strings.splitList("1, 2, 3");
+ assertEquals(3, list.length);
+ list = Strings.splitList("");
+ assertEquals(1, list.length);
+ list = Strings.splitList(null);
+ assertEquals(0, list.length);
+ }
+
+ @Test
public void testQuote() {
assertEquals("'Q'", Strings.quote("Q"));
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
index 286d86d..305a696 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java
@@ -309,7 +309,7 @@ public final class Strings {
* @return An array of strings.
*/
public static String[] splitList(String string) {
- return string.split(COMMA_DELIMITED_RE);
+ return string != null ? string.split(COMMA_DELIMITED_RE) : new String[0];
}
/**
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/UrlConnectionFactoryTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/UrlConnectionFactoryTest.java
new file mode 100644
index 0000000..07cd754
--- /dev/null
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/net/UrlConnectionFactoryTest.java
@@ -0,0 +1,177 @@
+/*
+ * 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.net;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URI;
+import java.nio.file.Files;
+import java.util.Base64;
+import java.util.Enumeration;
+import java.util.Properties;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.core.config.ConfigurationSource;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.servlet.DefaultServlet;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+/**
+ * Tests the UrlConnectionFactory
+ */
+public class UrlConnectionFactoryTest {
+
+ private static final Logger LOGGER = LogManager.getLogger(UrlConnectionFactoryTest.class);
+ private static final String BASIC = "Basic ";
+ private static final String expectedCreds = "testuser:password";
+ private static Server server;
+ private static Base64.Decoder decoder = Base64.getDecoder();
+ private static int port;
+ private static final int NOT_MODIFIED = 304;
+ private static final int NOT_AUTHORIZED = 401;
+ private static final int OK = 200;
+ private static final int BUF_SIZE = 1024;
+
+ @BeforeAll
+ public static void startServer() throws Exception {
+ try {
+ server = new Server(0);
+ ServletContextHandler context = new ServletContextHandler();
+ ServletHolder defaultServ = new ServletHolder("default", TestServlet.class);
+ defaultServ.setInitParameter("resourceBase", System.getProperty("user.dir"));
+ defaultServ.setInitParameter("dirAllowed", "true");
+ context.addServlet(defaultServ, "/");
+ server.setHandler(context);
+
+ // Start Server
+ server.start();
+ port = ((ServerConnector) server.getConnectors()[0]).getLocalPort();
+ } catch (Throwable ex) {
+ ex.printStackTrace();
+ throw ex;
+ }
+ }
+
+ @AfterAll
+ public static void stopServer() throws Exception {
+ server.stop();
+ }
+
+ @Test
+ public void testBadCrdentials() throws Exception {
+ System.setProperty("log4j2.Configuration.username", "foo");
+ System.setProperty("log4j2.Configuration.password", "bar");
+ System.setProperty("log4j2.Configuration.allowedProtocols", "http");
+ URI uri = new URI("http://localhost:" + port + "/log4j2-config.xml");
+ ConfigurationSource source = ConfigurationSource.fromUri(uri);
+ assertNull(source, "A ConfigurationSource should not have been returned");
+ }
+
+ @Test
+ public void withAuthentication() throws Exception {
+ System.setProperty("log4j2.Configuration.username", "testuser");
+ System.setProperty("log4j2.Configuration.password", "password");
+ System.setProperty("log4j2.Configuration.allowedProtocols", "http");
+ URI uri = new URI("http://localhost:" + port + "/log4j2-config.xml");
+ ConfigurationSource source = ConfigurationSource.fromUri(uri);
+ assertNotNull(source, "No ConfigurationSource returned");
+ InputStream is = source.getInputStream();
+ assertNotNull(is, "No data returned");
+ long lastModified = source.getLastModified();
+ int result = verifyNotModified(uri, lastModified);
+ assertEquals(NOT_MODIFIED, result,"File was modified");
+ File file = new File("target/test-classes/log4j2-config.xml");
+ if (!file.setLastModified(System.currentTimeMillis())) {
+ fail("Unable to set last modified time");
+ }
+ result = verifyNotModified(uri, lastModified);
+ assertEquals(OK, result,"File was not modified");
+ }
+
+ private int verifyNotModified(URI uri, long lastModifiedMillis) throws Exception {
+ final HttpURLConnection urlConnection = UrlConnectionFactory.createConnection(uri.toURL(),
+ lastModifiedMillis, null);
+ urlConnection.connect();
+
+ try {
+ return urlConnection.getResponseCode();
+ } catch (final IOException ioe) {
+ LOGGER.error("Error accessing configuration at {}: {}", uri, ioe.getMessage());
+ return 500;
+ }
+ }
+
+ public static class TestServlet extends DefaultServlet {
+
+ private static final long serialVersionUID = -2885158530511450659L;
+
+ @Override
+ protected void doGet(HttpServletRequest request,
+ HttpServletResponse response) throws ServletException, IOException {
+ Enumeration<String> headers = request.getHeaders(HttpHeader.AUTHORIZATION.toString());
+ if (headers == null) {
+ response.sendError(401, "No Auth header");
+ return;
+ }
+ while (headers.hasMoreElements()) {
+ String authData = headers.nextElement();
+ assertTrue(authData.startsWith(BASIC), "Not a Basic auth header");
+ String credentials = new String(decoder.decode(authData.substring(BASIC.length())));
+ if (!expectedCreds.equals(credentials)) {
+ response.sendError(401, "Invalid credentials");
+ return;
+ }
+ }
+ if (request.getServletPath().equals("/log4j2-config.xml")) {
+ File file = new File("target/test-classes/log4j2-config.xml");
+ long modifiedSince = request.getDateHeader(HttpHeader.IF_MODIFIED_SINCE.toString());
+ long lastModified = (file.lastModified() / 1000) * 1000;
+ LOGGER.debug("LastModified: {}, modifiedSince: {}", lastModified, modifiedSince);
+ if (modifiedSince > 0 && lastModified <= modifiedSince) {
+ response.setStatus(304);
+ return;
+ }
+ response.setDateHeader(HttpHeader.LAST_MODIFIED.toString(), lastModified);
+ response.setContentLengthLong(file.length());
+ Files.copy(file.toPath(), response.getOutputStream());
+ response.getOutputStream().flush();
+ response.setStatus(200);
+ } else {
+ response.sendError(400, "Unsupported request");
+ }
+ }
+ }
+}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java
index 415c8b6..a8af985 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java
@@ -33,6 +33,7 @@ import org.apache.logging.log4j.core.config.ConfigurationScheduler;
import org.apache.logging.log4j.core.config.DefaultConfiguration;
import org.apache.logging.log4j.core.config.HttpWatcher;
import org.apache.logging.log4j.core.config.Reconfigurable;
+import org.apache.logging.log4j.core.net.UrlConnectionFactory;
import org.apache.logging.log4j.core.test.net.ssl.TestConstants;
import org.apache.logging.log4j.core.time.internal.format.FastDateFormat;
import org.apache.logging.log4j.test.junit.StatusLoggerRule;
@@ -69,6 +70,7 @@ public class WatchHttpTest {
@BeforeClass
public static void beforeClass() {
+ System.setProperty(UrlConnectionFactory.ALLOWED_PROTOCOLS, "http,https");
try {
formatter = FastDateFormat.getInstance("EEE, dd MMM yyyy HH:mm:ss", TimeZone.getTimeZone("UTC"));
} catch (Exception ex) {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java
index 930ec9f..edcca05 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java
@@ -99,7 +99,7 @@ public abstract class ConfigurationFactory extends ConfigurationBuilderFactory {
public static final String LOG4J1_EXPERIMENTAL = "log4j1.compatibility";
- public static final String AUTHORIZATION_PROVIDER = "log4j2.authorizationProvider";
+ public static final String AUTHORIZATION_PROVIDER = "authorizationProvider";
/**
* Plugin category used to inject a ConfigurationFactory {@link org.apache.logging.log4j.plugins.Plugin}
@@ -150,6 +150,8 @@ public abstract class ConfigurationFactory extends ConfigurationBuilderFactory {
private static final String HTTPS = "https";
private static final String HTTP = "http";
+ private static final String[] PREFIXES = {"log4j2.", "log4j2.Configuration."};
+
private static volatile AuthorizationProvider authorizationProvider;
/**
@@ -199,7 +201,7 @@ public abstract class ConfigurationFactory extends ConfigurationBuilderFactory {
}
public static AuthorizationProvider authorizationProvider(final PropertiesUtil props) {
- final String authClass = props.getStringProperty(AUTHORIZATION_PROVIDER);
+ final String authClass = props.getStringProperty(PREFIXES, AUTHORIZATION_PROVIDER, null);
AuthorizationProvider provider = null;
if (authClass != null) {
try {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/UrlConnectionFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/UrlConnectionFactory.java
index 63df3d2..5e97030 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/UrlConnectionFactory.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/UrlConnectionFactory.java
@@ -18,8 +18,14 @@ package org.apache.logging.log4j.core.net;
import java.io.IOException;
import java.net.HttpURLConnection;
+import java.net.ProtocolException;
import java.net.URL;
import java.net.URLConnection;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
+import java.util.Properties;
+import java.util.Set;
import javax.net.ssl.HttpsURLConnection;
import org.apache.logging.log4j.core.config.ConfigurationFactory;
@@ -27,6 +33,9 @@ import org.apache.logging.log4j.core.net.ssl.LaxHostnameVerifier;
import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
import org.apache.logging.log4j.core.net.ssl.SslConfigurationFactory;
import org.apache.logging.log4j.core.util.AuthorizationProvider;
+import org.apache.logging.log4j.core.util.BasicAuthorizationProvider;
+import org.apache.logging.log4j.util.PropertiesUtil;
+import org.apache.logging.log4j.util.Strings;
/**
* Constructs an HTTPURLConnection. This class should be considered to be internal
@@ -42,11 +51,26 @@ public class UrlConnectionFactory {
private static final String TEXT = "text/plain";
private static final String HTTP = "http";
private static final String HTTPS = "https";
+ private static final String NO_PROTOCOLS = "_none";
+ public static final String ALLOWED_PROTOCOLS = "log4j2.Configuration.allowedProtocols";
public static HttpURLConnection createConnection(final URL url, final long lastModifiedMillis, final SslConfiguration sslConfiguration)
throws IOException {
+ PropertiesUtil props = PropertiesUtil.getProperties();
+ List<String> allowed = Arrays.asList(Strings.splitList(props
+ .getStringProperty(ALLOWED_PROTOCOLS, HTTPS).toLowerCase(Locale.ROOT)));
+ if (allowed.size() == 1 && NO_PROTOCOLS.equals(allowed.get(0))) {
+ throw new ProtocolException("No external protocols have been enabled");
+ }
+ String protocol = url.getProtocol();
+ if (protocol == null) {
+ throw new ProtocolException("No protocol was specified on " + url.toString());
+ }
+ if (!allowed.contains(protocol)) {
+ throw new ProtocolException("Protocol " + protocol + " has not been enabled as an allowed protocol");
+ }
final HttpURLConnection urlConnection = (HttpURLConnection) url.openConnection();
- final AuthorizationProvider provider = ConfigurationFactory.getAuthorizationProvider();
+ final AuthorizationProvider provider = ConfigurationFactory.authorizationProvider(props);
if (provider != null) {
provider.addAuthorization(urlConnection);
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/BasicAuthorizationProvider.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/BasicAuthorizationProvider.java
index 52df233..61aa2d5 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/BasicAuthorizationProvider.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/BasicAuthorizationProvider.java
@@ -28,7 +28,7 @@ import org.apache.logging.log4j.util.PropertiesUtil;
* Provides the Basic Authorization header to a request.
*/
public class BasicAuthorizationProvider implements AuthorizationProvider {
- private static final String[] PREFIXES = {"log4j2.config.", "logging.auth."};
+ private static final String[] PREFIXES = {"log4j2.config.", "log4j2.Configuration.", "logging.auth."};
private static final String AUTH_USER_NAME = "username";
private static final String AUTH_PASSWORD = "password";
private static final String AUTH_PASSWORD_DECRYPTOR = "passwordDecryptor";
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index c0fca20..2298558 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -303,6 +303,9 @@
Fix log4j-jakarta-web service file #723.
</action>
<!-- ADD -->
+ <action issue="LOG4J2-3297" dev="rgoers" type="add">
+ Limit loading of configuration via a url to https by default.
+ </action>
<action type="add" dev="rgoers" issue="LOG4J2-3341">
Add shorthand syntax for properties configuration format for specifying a logger level and appender refs.
</action>
diff --git a/src/site/asciidoc/manual/configuration.adoc b/src/site/asciidoc/manual/configuration.adoc
index 6c73c85..f586f66 100644
--- a/src/site/asciidoc/manual/configuration.adoc
+++ b/src/site/asciidoc/manual/configuration.adoc
@@ -355,6 +355,28 @@ trace will result in results similar to:
Note that status logging is disabled when the default configuration is
used.
+[#Configuration_From_a_URI]
+== Configuraton From a URI
+
+When `log4j2.configurationFile` references a URL Log4j will first determine if the URL reference
+a file using the file protocol. If it does Log4j will validate that the file url is valid and continue
+processing as previously described. If it contains a protocol other than file then Log4j will inspect
+the value of the `log4j2.Configuration.allowedProtocols` system property. If the provided list
+contains the protocol specified then Log4j will use the URI to locate the specified configuration file. If
+not an exception will be thrown and an error message will be logged. If no value is provided for the
+system property it will default to "https". Use of any protocol other than "file" can be prevented by
+setting the system property value to "_none". This value would be an invalid protocol so cannot conflict
+with any custom protocols that may be present.
+
+Log4j supports access to remote urls that require authentication. Log4j supports basic authentication
+out of the box. If the `log4j2.Configuration.username` and `log4j2.Configuration.password`
+are specified those values will be used to perform the authentication. If the password is encrypted a custom
+password decryptor may be supplied by specifying the fully qualified class name in the
+`log4j2.Configuration.passwordDecryptor` system property. A custom
+`AuthenticationProvider` may be used by setting the
+`log4j2.Configuration.authenticationProvider` system property to the fully qualified class name
+of the provider.
+
[#Additivity]
== Additivity
@@ -1961,6 +1983,37 @@ extending
If specified, an instance of this class is added to the list of
configuration factories.
+|[[configurationAllowedProtocols]]log4j2.Configuration.allowedProtocols +
+([[log4j.configurationAllowedProtocols]]log4j.configurationAllowedProtocols)
+|LOG4J_CONFIGURATION_ALLOWED_PROTOCOLS
+|
+|A comma separated list of the protocols that may be used to load a configuration file. The default is https.
+To completely prevent accessing the configuration via a URL specify a value of "_none".
+
+|[[configurationAuthorizationProvider]]log4j2.Configuration.authorizationProvider +
+([[log4j.configurationAuthorizationProvider]]log4j.configurationAuthorizationProvider)
+|LOG4J_CONFIGURATION_AUTHORIZATION_PROVIDER
+|org.apache.logging.log4j.core.util.BasicAuthorizationProvider
+|The fully qualified class name of the AuthorizationProvider.
+
+|[[configurationPassword]]log4j2.Configuration.password +
+([[log4j.configurationPassword]]log4j.configurationPassword)
+|LOG4J_CONFIGURATION_PASSWORD
+|
+|The password required to access the remote logging configuration file.
+
+|[[configurationPasswordDecryptor]]log4j2.Configuration.passwordDecryptor +
+([[log4j.configurationPasswordDecryptor]]log4j.configurationPasswordDecryptor)
+|LOG4J_CONFIGURATION_DECRYPTOR
+|
+|If the password is encrypted this class will be used to decrypt it.
+
+|[[configurationUsername]]log4j2.Configuration.username+
+([[log4j.configurationUsername]]log4j.configurationUsername)
+|LOG4J_CONFIGURATION_USERNAME
+|
+|The user name required to access the remote logging configuration file.
+
|[[shutdownHookEnabled]]log4j2.shutdownHookEnabled +
([[log4j.shutdownHookEnabled]]log4j.shutdownHookEnabled)
|LOG4J_SHUTDOWN_HOOK_ENABLED