Skip to content

Commit

Permalink
LOG4J2-3208 - Disable JNDI by default
Browse files Browse the repository at this point in the history
  • Loading branch information
rgoers committed Dec 11, 2021
1 parent 3a5836b commit 4456909
Show file tree
Hide file tree
Showing 16 changed files with 191 additions and 29 deletions.
Expand Up @@ -24,6 +24,7 @@
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.config.ConfigurationAware;
import org.apache.logging.log4j.core.net.JndiManager;
import org.apache.logging.log4j.core.util.Loader;
import org.apache.logging.log4j.util.ReflectionUtil;
import org.apache.logging.log4j.plugins.util.PluginManager;
Expand Down Expand Up @@ -77,7 +78,9 @@ public Interpolator(final StrLookup defaultLookup, final List<String> pluginPack
for (final Map.Entry<String, PluginType<?>> entry : plugins.entrySet()) {
try {
final Class<? extends StrLookup> clazz = entry.getValue().getPluginClass().asSubclass(StrLookup.class);
strLookupMap.put(entry.getKey().toLowerCase(), ReflectionUtil.instantiate(clazz));
if (!clazz.getName().equals(JndiLookup.class.getName()) || JndiManager.isIsJndiEnabled()) {

This comment has been minimized.

Copy link
@karaatanassov

karaatanassov Dec 14, 2021

Would not JndiLookup.class.getName() cause regression of [LOG4J2-703] mentioned below?

As well this will conflict with the guides recommending the deletion of the JndiLookup.class from the jar files.

This comment has been minimized.

Copy link
@karaatanassov

karaatanassov Dec 14, 2021

The code below uses a string constant that does not depend on class loading.

This comment has been minimized.

Copy link
@dmitankin

dmitankin Dec 15, 2021

Just verified that removing the JndiLookup class on 2.16 causes a failure:

ERROR Unable to create Lookup for bundle java.lang.NoClassDefFoundError: org/apache/logging/log4j/core/lookup/JndiLookup
        at org.apache.logging.log4j.core.lookup.Interpolator.<init>(Interpolator.java:81)
        at org.apache.logging.log4j.core.config.AbstractConfiguration.doConfigure(AbstractConfiguration.java:631)
        at org.apache.logging.log4j.core.config.AbstractConfiguration.initialize(AbstractConfiguration.java:243)
        at org.apache.logging.log4j.core.config.AbstractConfiguration.start(AbstractConfiguration.java:289)
        at org.apache.logging.log4j.core.LoggerContext.setConfiguration(LoggerContext.java:626)
        at org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:699)
        at org.apache.logging.log4j.core.LoggerContext.reconfigure(LoggerContext.java:716)
        at org.apache.logging.log4j.core.LoggerContext.start(LoggerContext.java:270)
        at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:155)
        at org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:47)
        at org.apache.logging.log4j.LogManager.getContext(LogManager.java:196)
        at org.apache.logging.log4j.spi.AbstractLoggerAdapter.getContext(AbstractLoggerAdapter.java:137)
        at org.apache.logging.slf4j.Log4jLoggerFactory.getContext(Log4jLoggerFactory.java:55)
        at org.apache.logging.log4j.spi.AbstractLoggerAdapter.getLogger(AbstractLoggerAdapter.java:47)
        at org.apache.logging.slf4j.Log4jLoggerFactory.getLogger(Log4jLoggerFactory.java:33)
        at org.slf4j.LoggerFactory.getLogger(LoggerFactory.java:363)
        <snip>
Caused by: java.lang.ClassNotFoundException: org.apache.logging.log4j.core.lookup.JndiLookup
        at java.net.URLClassLoader.findClass(URLClassLoader.java:387)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
        at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:365)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
        ... 18 more

The same is not true on 2.13.1. This change seems to regress LOG4J2-703 indeed.

This comment has been minimized.

Copy link
@garydgregory

garydgregory via email Dec 15, 2021

Member

This comment has been minimized.

This comment has been minimized.

Copy link
@pgomulka

pgomulka Dec 17, 2021

@garydgregory just wanted to find out when do you plan the next release with this change? Do you have a rough estimate or a date?

This comment has been minimized.

Copy link
@garydgregory

garydgregory Dec 18, 2021

Member

@garydgregory just wanted to find out when do you plan the next release with this change? Do you have a rough estimate or a date?

We are working on it, please be patient... :-)

strLookupMap.put(entry.getKey().toLowerCase(), ReflectionUtil.instantiate(clazz));
}
} catch (final Throwable t) {
handleError(entry.getKey(), t);
}
Expand Down Expand Up @@ -107,12 +110,14 @@ public Interpolator(final Map<String, String> properties) {
strLookupMap.put("lower", new LowerLookup());
strLookupMap.put("upper", new UpperLookup());
// JNDI
try {
// [LOG4J2-703] We might be on Android
strLookupMap.put(LOOKUP_KEY_JNDI,
Loader.newCheckedInstanceOf("org.apache.logging.log4j.core.lookup.JndiLookup", StrLookup.class));
} catch (final LinkageError | Exception e) {
handleError(LOOKUP_KEY_JNDI, e);
if (JndiManager.isIsJndiEnabled()) {
try {
// [LOG4J2-703] We might be on Android
strLookupMap.put(LOOKUP_KEY_JNDI,
Loader.newCheckedInstanceOf("org.apache.logging.log4j.core.lookup.JndiLookup", StrLookup.class));
} catch (final LinkageError | Exception e) {
handleError(LOOKUP_KEY_JNDI, e);
}
}
// JMX input args
try {
Expand Down
Expand Up @@ -73,6 +73,10 @@ public class JndiManager extends AbstractManager {

private final DirContext context;

public static boolean isIsJndiEnabled() {
return PropertiesUtil.getProperties().getBooleanProperty("log4j2.enableJndi", false);
}

private JndiManager(final String name, final DirContext context, final List<String> allowedHosts,
final List<String> allowedClasses, final List<String> allowedProtocols) {
super(null, name);
Expand All @@ -82,6 +86,14 @@ private JndiManager(final String name, final DirContext context, final List<Stri
this.allowedProtocols = allowedProtocols;
}

private JndiManager(final String name) {
super(null, name);
this.context = null;
this.allowedProtocols = null;
this.allowedClasses = null;
this.allowedHosts = null;
}

/**
* Gets the default JndiManager using the default {@link javax.naming.InitialContext}.
*
Expand Down Expand Up @@ -194,6 +206,9 @@ public static Properties createProperties(final String initialContextFactoryName

@Override
protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) {
if (context != null) {
return JndiCloser.closeSilently(this.context);
}
return JndiCloser.closeSilently(this.context);

This comment has been minimized.

Copy link
@MrVoltz

MrVoltz Dec 11, 2021

Is this correct? Both branches are the same.

This comment has been minimized.

Copy link
@rgoers

rgoers Dec 12, 2021

Author Member

Are you meaning that the same change was made to both the release-2.x and master branches? That is intended as the master branch will become Log4j 2 3.0 someday.

This comment has been minimized.

Copy link
@MrVoltz

MrVoltz Dec 12, 2021

I meant that JndiCloser.closeSilently(this.context); is called no matter if context is or isn't null. The if condition is useless.

}

Expand All @@ -207,6 +222,9 @@ protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) {
*/
@SuppressWarnings("unchecked")
public synchronized <T> T lookup(final String name) throws NamingException {
if (context == null) {
return null;
}
try {
URI uri = new URI(name);
if (uri.getScheme() != null) {
Expand Down Expand Up @@ -262,21 +280,25 @@ private static class JndiManagerFactory implements ManagerFactory<JndiManager, P

@Override
public JndiManager createManager(final String name, final Properties data) {
String hosts = data != null ? data.getProperty(ALLOWED_HOSTS) : null;
String classes = data != null ? data.getProperty(ALLOWED_CLASSES) : null;
String protocols = data != null ? data.getProperty(ALLOWED_PROTOCOLS) : null;
List<String> allowedHosts = new ArrayList<>();
List<String> allowedClasses = new ArrayList<>();
List<String> allowedProtocols = new ArrayList<>();
addAll(hosts, allowedHosts, permanentAllowedHosts, ALLOWED_HOSTS, data);
addAll(classes, allowedClasses, permanentAllowedClasses, ALLOWED_CLASSES, data);
addAll(protocols, allowedProtocols, permanentAllowedProtocols, ALLOWED_PROTOCOLS, data);
try {
return new JndiManager(name, new InitialDirContext(data), allowedHosts, allowedClasses,
allowedProtocols);
} catch (final NamingException e) {
LOGGER.error("Error creating JNDI InitialContext.", e);
return null;
if (isIsJndiEnabled()) {
String hosts = data != null ? data.getProperty(ALLOWED_HOSTS) : null;
String classes = data != null ? data.getProperty(ALLOWED_CLASSES) : null;
String protocols = data != null ? data.getProperty(ALLOWED_PROTOCOLS) : null;
List<String> allowedHosts = new ArrayList<>();
List<String> allowedClasses = new ArrayList<>();
List<String> allowedProtocols = new ArrayList<>();
addAll(hosts, allowedHosts, permanentAllowedHosts, ALLOWED_HOSTS, data);
addAll(classes, allowedClasses, permanentAllowedClasses, ALLOWED_CLASSES, data);
addAll(protocols, allowedProtocols, permanentAllowedProtocols, ALLOWED_PROTOCOLS, data);
try {
return new JndiManager(name, new InitialDirContext(data), allowedHosts, allowedClasses,
allowedProtocols);
} catch (final NamingException e) {
LOGGER.error("Error creating JNDI InitialContext.", e);
return null;
}
} else {
return new JndiManager(name);
}
}

Expand Down
Expand Up @@ -93,6 +93,12 @@ public class JndiContextSelector implements NamedContextSelector {

private static final StatusLogger LOGGER = StatusLogger.getLogger();

public JndiContextSelector() {
if (!JndiManager.isIsJndiEnabled()) {
throw new IllegalStateException("JNDI must be enabled by setting log4j2.enableJndi=true");
}
}

@Override
public void shutdown(final String fqcn, final ClassLoader loader, final boolean currentContext, final boolean allContexts) {
LoggerContext ctx = ContextAnchor.THREAD_CONTEXT.get();
Expand Down
Expand Up @@ -18,6 +18,7 @@

import java.io.File;
import java.util.Collections;
import java.util.Map;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;
Expand Down Expand Up @@ -47,9 +48,14 @@ public class RoutingAppenderWithJndiTest {
public static LoggerContextRule loggerContextRule = new LoggerContextRule("log4j-routing-by-jndi.xml");

@ClassRule
public static RuleChain rules = RuleChain.outerRule(new JndiRule(Collections.<String, Object>emptyMap()))
public static RuleChain rules = RuleChain.outerRule(new JndiRule(initBindings()))
.around(loggerContextRule);

private static Map<String, Object> initBindings() {
System.setProperty("log4j2.enableJndi", "true");
return Collections.emptyMap();
}

@Before
public void before() throws NamingException {
listAppender1 = RoutingAppenderWithJndiTest.loggerContextRule.getListAppender("List1");
Expand Down
Expand Up @@ -48,12 +48,14 @@ public class InterpolatorTest {
protected void before() throws Throwable {
System.setProperty(TESTKEY, TESTVAL);
System.setProperty(TESTKEY2, TESTVAL);
System.setProperty("log4j2.enableJndi", "true");
}

@Override
protected void after() {
System.clearProperty(TESTKEY);
System.clearProperty(TESTKEY2);
System.clearProperty("log4j2.enableJndi");
}
}).around(new JndiRule(
JndiLookup.CONTAINER_JNDI_RESOURCE_PATH_PREFIX + TEST_CONTEXT_RESOURCE_NAME, TEST_CONTEXT_NAME));
Expand Down
@@ -0,0 +1,62 @@
/*
* 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.lookup;

import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;

import org.apache.logging.log4j.core.test.junit.JndiRule;
import org.junit.Rule;
import org.junit.Test;

import static org.junit.Assert.assertNull;

/**
* JndiDisabledLookupTest
*
* Verifies the Lookups are disabled without the log4j2.enableJndi property set to true.
*/
public class JndiDisabledLookupTest {

private static final String TEST_CONTEXT_RESOURCE_NAME = "logging/context-name";
private static final String TEST_CONTEXT_NAME = "app-1";
private static final String TEST_INTEGRAL_NAME = "int-value";
private static final int TEST_INTEGRAL_VALUE = 42;
private static final String TEST_STRINGS_NAME = "string-collection";
private static final Collection<String> TEST_STRINGS_COLLECTION = Arrays.asList("one", "two", "three");

@Rule
public JndiRule jndiRule = new JndiRule(createBindings());

private Map<String, Object> createBindings() {
final Map<String, Object> map = new HashMap<>();
map.put(JndiLookup.CONTAINER_JNDI_RESOURCE_PATH_PREFIX + TEST_CONTEXT_RESOURCE_NAME, TEST_CONTEXT_NAME);
map.put(JndiLookup.CONTAINER_JNDI_RESOURCE_PATH_PREFIX + TEST_INTEGRAL_NAME, TEST_INTEGRAL_VALUE);
map.put(JndiLookup.CONTAINER_JNDI_RESOURCE_PATH_PREFIX + TEST_STRINGS_NAME, TEST_STRINGS_COLLECTION);
return map;
}

@Test
public void testLookup() {
final StrLookup lookup = new JndiLookup();

String contextName = lookup.lookup(TEST_CONTEXT_RESOURCE_NAME);
assertNull(contextName);
}
}
Expand Up @@ -22,6 +22,7 @@
import java.util.Map;

import org.apache.logging.log4j.core.test.junit.JndiRule;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;

Expand All @@ -42,6 +43,11 @@ public class JndiLookupTest {
@Rule
public JndiRule jndiRule = new JndiRule(createBindings());

@BeforeClass
public static void beforeClass() {
System.setProperty("log4j2.enableJndi", "true");
}

private Map<String, Object> createBindings() {
final Map<String, Object> map = new HashMap<>();
map.put(JndiLookup.CONTAINER_JNDI_RESOURCE_PATH_PREFIX + TEST_CONTEXT_RESOURCE_NAME, TEST_CONTEXT_NAME);
Expand Down
Expand Up @@ -54,6 +54,7 @@ public class JndiRestrictedLookupTest {
public static void beforeClass() {
System.setProperty("log4j2.allowedLdapClasses", Level.class.getName());
System.setProperty("log4j2.allowedJndiProtocols", "dns");
System.setProperty("log4j2.enableJndi", "true");
}

@Test
Expand Down
Expand Up @@ -125,10 +125,15 @@ private static class JmsManagerFactory implements ManagerFactory<JmsManager, Jms

@Override
public JmsManager createManager(final String name, final JmsManagerConfiguration data) {
try {
return new JmsManager(name, data);
} catch (final Exception e) {
logger().error("Error creating JmsManager using JmsManagerConfiguration [{}]", data, e);
if (JndiManager.isIsJndiEnabled()) {
try {
return new JmsManager(name, data);
} catch (final Exception e) {
logger().error("Error creating JmsManager using JmsManagerConfiguration [{}]", data, e);
return null;
}
} else {
logger().error("JNDI has not been enabled. The log4j2.enableJndi property must be set to true");
return null;
}
}
Expand Down
Expand Up @@ -49,6 +49,7 @@
import org.apache.logging.log4j.message.SimpleMessage;
import org.apache.logging.log4j.message.StringMapMessage;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
Expand Down Expand Up @@ -83,6 +84,11 @@ public class JmsAppenderTest {
@Rule
public RuleChain rules = RuleChain.outerRule(jndiRule).around(ctx);

@BeforeClass
public static void beforeClass() throws Exception {
System.setProperty("log4j2.enableJndi", "true");
}

public JmsAppenderTest() throws Exception {
// this needs to set up before LoggerContextRule
given(connectionFactory.createConnection()).willReturn(connection);
Expand Down
5 changes: 4 additions & 1 deletion src/changes/changes.xml
Expand Up @@ -168,7 +168,10 @@
Fixes incorrect constructor call in LocalizedMessageFactory.
</action>
</release>
<release version="2.15.1" date="2021-12-XX" description="GA Release 2.15.0">
<release version="2.15.1" date="2021-12-XX" description="GA Release 2.15.1">
<action issue="LOG4J2-3208" dev="rgoers" type="fix">
Disable JNDI by default. Require log4j2.enableJndi to be set to true to allow JNDI.
</action>
</release>
<release version="2.15.0" date="2021-12-06" description="GA Release 2.15.0">
<!-- ADDS -->
Expand Down
4 changes: 4 additions & 0 deletions src/site/asciidoc/manual/appenders.adoc
Expand Up @@ -1259,6 +1259,10 @@ As of Log4j 2.11.0, JPA support has moved from the existing module
The JMS Appender sends the formatted log event to a JMS Destination.
The JMS Appender requires JNDI support so as of release 2.15.1 this appender will not function unless
`log4j2.enableJndi=true` is configured as a system property or environment variable. See the
link:./configuration.html#enableJndi[log4j2.enableJndi] system property.
Note that in Log4j 2.0, this appender was split into a JMSQueueAppender
and a JMSTopicAppender. Starting in Log4j 2.1, these appenders were
combined into the JMS Appender which makes no distinction between queues
Expand Down
26 changes: 26 additions & 0 deletions src/site/asciidoc/manual/configuration.adoc
Expand Up @@ -2073,6 +2073,32 @@ context class loader before falling back to the default class loader.
|If `true`, classes and configuration are only loaded with the default context class loader.
Otherwise, log4j also uses the log4j classloader, parent classloaders and the system classloader.
|[[enableJndi]]log4j2.enableJndi +
([[log4j.enableJndi]]log4j.enableJndi)
|LOG4J_ENABLE_JNDI
|false
| When true, Log4j components that use JNDI are enabled. When false, the default, they are disabled.
|[[allowedLdapClasses]]log4j2.allowedLdapClasses +
([[log4j.allowedLdapClasses]]log4j.allowedLdapClasses)
|LOG4J_ALLOWED_LDAP_CLASSES
|&nbsp;
| System property that specifies fully qualified class names that may be accessed by LDAP. The classes
must implement Serializable. By default only Java primative classes are allowed.
|[[allowedLdapHosts]]log4j2.allowedLdapHosts +
([[log4j.allowedLdapHosts]]log4j.allowedLdapHosts)
|LOG4J_ALLOWED_LDAP_HOSTS
|&nbsp;
| System property that adds host names or ip addresses that may be access by LDAP. By default it only allows
the local host names and ip addresses.
|[[allowedJndiProtocols]]log4j2.allowedJndiProtocols +
([[log4j.allowedJndiProtocols]]log4j.allowedJndiProtocols)
|LOG4J_ALLOWED_JNDI_PROTOCOLS
|&nbsp;
| System property that adds protocol names that JNDI will allow. By default it only allows java, ldap, and ldaps.
|[[uuidSequence]]log4j2.uuidSequence +
([[org.apache.logging.log4j.uuidSequence]]org.apache.logging.log4j.uuidSequence)
|LOG4J_UUID_SEQUENCE
Expand Down
4 changes: 4 additions & 0 deletions src/site/asciidoc/manual/logsep.adoc
Expand Up @@ -116,6 +116,10 @@ cause the container to use JNDI to locate each web application's
context parameter to true and also set the `log4jContextName` and
`log4jConfiguration` context parameters.
The `JndiContextSelector` will not work unless `log4j2.enableJndi=true` is set as a system property
or environment variable. See the
link:./configuration.html#enableJndi[log4j2.enableJndi] system property.
The exact method for setting system properties depends on the container.
For Tomcat, edit `$CATALINA_HOME/conf/catalina.properties`. Consult the
documentation for other web containers.
4 changes: 4 additions & 0 deletions src/site/asciidoc/manual/lookups.adoc
Expand Up @@ -267,6 +267,10 @@ For example:
[#JndiLookup]
== JNDI Lookup
As of Log4j 2.15.1 JNDI operations require that `log4j2.enableJndi=tru`e be set as a system property or the

This comment has been minimized.

Copy link
@jschauma

jschauma Dec 12, 2021

s/tru`e/true`/

This comment has been minimized.

Copy link
@remkop

remkop Dec 14, 2021

Contributor

Looking at it now. Thanks!

This comment has been minimized.

Copy link
@remkop

remkop Dec 14, 2021

Contributor

This is the master branch. This is not published.
In the 2.x branch, the lookups page is generated from an XML file that does not have this typo.

I will fix the typo in master later, working on the published site first.

corresponding environment variable for this lookup to function. See the
link:./configuration.html#enableJndi[log4j2.enableJndi] system property.
The JndiLookup allows variables to be retrieved via JNDI. By default the
key will be prefixed with java:comp/env/, however if the key contains a
":" no prefix will be added.
Expand Down
2 changes: 1 addition & 1 deletion src/site/asciidoc/security.adoc
Expand Up @@ -63,7 +63,7 @@ substitution is enabled. From log4j 2.15.0, this behavior has been disabled by d
Mitigation: In releases &gt;= 2.10, this behavior can be mitigated by setting either the system property
`log4j2.formatMsgNoLookups` or the environment variable `LOG4J_FORMAT_MSG_NO_LOOKUPS` to `true`.
For releases from 2.7 through 2.14.1 all PatternLayout patterns can be modified to specify the message converter as
`%m{nnolookups}` instead of just `%m`.
`%m{nolookups}` instead of just `%m`.

This comment has been minimized.

Copy link
@guusdk

guusdk Dec 14, 2021

At the time of writing (two days after this commit was made), this fix has not yet made it to the public website at https://logging.apache.org/log4j/2.x/ - that's now showing the typo - that's easy for people to copy/paste without realizing that they're introducing a typo. If at all possible, can this change be pushed?

This comment has been minimized.

Copy link
@remkop

remkop Dec 14, 2021

Contributor

Working on it...
Changes are being reviewed as we speak.

This comment has been minimized.

Copy link
@e3k

e3k Dec 15, 2021

what about building a dummy jindilookup.class and replace the original one. from what i have heard tomcat does not like the remove.

For releases from 2.0-beta9 to 2.7, the only mitigation is to remove the `JndiLookup` class from the classpath:
`zip -q -d log4j-core-*.jar org/apache/logging/log4j/core/lookup/JndiLookup.class`.
Expand Down

0 comments on commit 4456909

Please sign in to comment.