This is an automated email from the ASF dual-hosted git repository. rgoers pushed a commit to branch release-2.x in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit e8dab8c5d652437ff61d2e797bfbe4e4e6cdd1bb Author: Ralph Goers <[email protected]> AuthorDate: Tue Feb 15 22:44:34 2022 -0700 LOG4J2-3297 - Limit loading of configuration via a url to https by default --- .../org/apache/logging/log4j/util/Strings.java | 2 +- .../org/apache/logging/log4j/util/StringsTest.java | 10 ++ log4j-core/revapi.json | 6 + .../log4j/core/config/ConfigurationFactory.java | 6 +- .../log4j/core/net/UrlConnectionFactory.java | 27 +++- .../core/util/BasicAuthorizationProvider.java | 4 +- .../log4j/core/net/UrlConnectionFactoryTest.java | 177 +++++++++++++++++++++ .../logging/log4j/core/util/WatchHttpTest.java | 2 + src/changes/changes.xml | 3 + src/site/xdoc/manual/configuration.xml.vm | 82 ++++++++++ 10 files changed, 314 insertions(+), 5 deletions(-) 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 fa1cd9c..c11b9c9 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 @@ -201,7 +201,7 @@ public final class 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-api/src/test/java/org/apache/logging/log4j/util/StringsTest.java b/log4j-api/src/test/java/org/apache/logging/log4j/util/StringsTest.java index c44fa40..6273869 100644 --- a/log4j-api/src/test/java/org/apache/logging/log4j/util/StringsTest.java +++ b/log4j-api/src/test/java/org/apache/logging/log4j/util/StringsTest.java @@ -84,6 +84,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-core/revapi.json b/log4j-core/revapi.json index 9c7f874..b19da41 100644 --- a/log4j-core/revapi.json +++ b/log4j-core/revapi.json @@ -118,6 +118,12 @@ "new": "method org.apache.logging.log4j.core.config.Property org.apache.logging.log4j.core.config.Property::createProperty(java.lang.String, java.lang.String)", "annotation": "@org.apache.logging.log4j.core.config.plugins.PluginFactory", "justification": "LOG4J2-3317: Fix RoutingAppender backcompat while improving lookup security" + }, + { + "code": "java.field.constantValueChanged", + "old": "field org.apache.logging.log4j.core.config.ConfigurationFactory.AUTHORIZATION_PROVIDER", + "new": "field org.apache.logging.log4j.core.config.ConfigurationFactory.AUTHORIZATION_PROVIDER", + "justification": "Added support for a second prefix" } ] } 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 ea905dd..1322f85 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 @@ -98,7 +98,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.core.config.plugins.Plugin} @@ -149,6 +149,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; /** @@ -198,7 +200,7 @@ public abstract class ConfigurationFactory extends ConfigurationBuilderFactory { } public static AuthorizationProvider authorizationProvider(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 7f17869..e813170 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,27 @@ 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(URL url, long lastModifiedMillis, 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(); - AuthorizationProvider provider = ConfigurationFactory.getAuthorizationProvider(); + + 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 eb3d9f3..ec381aa 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 @@ -17,6 +17,8 @@ package org.apache.logging.log4j.core.util; import java.net.URLConnection; +import java.util.Properties; + import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.Base64Util; @@ -27,7 +29,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/log4j-core/src/test/java/org/apache/logging/log4j/core/net/UrlConnectionFactoryTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/UrlConnectionFactoryTest.java new file mode 100644 index 0000000..07cd754 --- /dev/null +++ b/log4j-core/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/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java index 6d06eed..7dd9db9 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java @@ -32,6 +32,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.net.ssl.TestConstants; import org.apache.logging.log4j.core.util.datetime.FastDateFormat; import org.apache.logging.log4j.util.PropertiesUtil; @@ -66,6 +67,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/src/changes/changes.xml b/src/changes/changes.xml index 580c6ea..015b444 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -215,6 +215,9 @@ Possible NullPointerException in MongoDb4DocumentObject, MongoDbDocumentObject, DefaultNoSqlObject. </action> <!-- ADD --> + <action issue="LOG4J2-3297" dev="rgoers" type="add"> + Limit loading of configuration via a url to https by default. + </action> <action issue="LOG4J2-2486" dev="rgoers" type="add"> Require log4j2.Script.enableLanguages to be specified to enable scripting for specific languages. </action> diff --git a/src/site/xdoc/manual/configuration.xml.vm b/src/site/xdoc/manual/configuration.xml.vm index fa73922..fcfde25 100644 --- a/src/site/xdoc/manual/configuration.xml.vm +++ b/src/site/xdoc/manual/configuration.xml.vm @@ -367,6 +367,31 @@ public class Bar { Note that status logging is disabled when the default configuration is used. </p> </subsection> + <a name="Configuration From a URI"/> + <subsection name="Configuraton From a URI"> + <p> + When <code>log4j2.configurationFile</code> 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 <code>log4j2.Configuration.allowedProtocols</code> 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. + </p> + <p> + Log4j supports access to remote urls that require authentication. Log4j supports basic authentication + out of the box. If the <code>log4j2.Configuration.username</code> and <code>log4j2.Configuration.password</code> + 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 + <code>log4j2.Configuration.passwordDecryptor</code> system property. A custom + <code>AuthenticationProvider</code> may be used by setting the + <code>log4j2.Configuration.authenticationProvider</code> system property to the fully qualified class name + of the provider. + </p> + + </subsection> <a name="Additivity"/> <subsection name="Additivity"> <p> @@ -1990,6 +2015,63 @@ public class AwesomeTest { </td> </tr> <tr> + <td><a name="configurationAllowedProtocols"/>log4j2.Configuration.allowedProtocols + <br /> + (<a name="log4j.configurationAllowedProtocols"/>log4j.configurationAllowedProtocols) + </td> + <td>LOG4J_CONFIGURATION_ALLOWED_PROTOCOLS</td> + <td> </td> + <td> + 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". + </td> + </tr> + <tr> + <td><a name="configurationAuthenticationProvider"/>log4j2.Configuration.authenticationProvider + <br /> + (<a name="log4j.configurationAuthenticationProvider"/>log4j.configurationAuthenticationProvider + </td> + <td>LOG4J_CONFIGURATION_ALLOWED_PROTOCOLS</td> + <td> </td> + <td> + 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". + </td> + </tr> + <tr> + <td><a name="configurationPassword"/>log4j2.Configuration.password + <br /> + (<a name="log4j.configurationPassword"/>log4j.configurationPassword + </td> + <td>LOG4J_CONFIGURATION_PASSWORD</td> + <td> </td> + <td> + The password required to access the remote logging configuration file. + </td> + </tr> + <tr> + <td><a name="configurationPasswordDecryptor"/>log4j2.Configuration.passwordDecryptor + <br /> + (<a name="log4j.configurationPasswordDecryptor"/>log4j.configurationPasswordDecryptor + </td> + <td>LOG4J_CONFIGURATION_PASSWORD_DECRYPTOR</td> + <td> </td> + <td> + If the password is encrypted this class will be used to decrypt it. + </td> + </tr> + <tr> + <td><a name="configurationUsername"/>log4j2.Configuration.username + <br /> + (<a name="log4j.configurationUsername"/>log4j.configurationUsername + </td> + <td>LOG4J_CONFIGURATION_USERNAME</td> + <td> </td> + <td> + The user name required to access the remote logging configuration file. + </td> + </tr> + <tr> <td><a name="shutdownHookEnabled"/>log4j2.shutdownHookEnabled <br /> (<a name="log4j.shutdownHookEnabled"/>log4j.shutdownHookEnabled)
