LOG4J2-3242 - Limit JNDI to the java protocol

rgoers · rgoers · commit f6564bb993d5 · 2021-12-20T12:56:16.000-07:00
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java
@@ -63,6 +63,9 @@ protected AbstractManager(final String name) {
      */
     public static <M extends AbstractManager, T> M getManager(final String name, final ManagerFactory<M, T> factory,
                                                               final T data) {
+        if (factory == null) {
+            throw new IllegalArgumentException("factory cannot be null");
+        }
         LOCK.lock();
         try {
             @SuppressWarnings("unchecked")
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsManager.java
@@ -156,12 +156,17 @@ private static class JmsManagerFactory implements ManagerFactory<JmsManager, Jms
 
         @Override
         public JmsManager createManager(final String name, final JmsConfiguration data) {
-            try {
-                return new JmsManager(name, data.jndiManager, data.connectionFactoryName, data.destinationName,
-                    data.username, data.password);
-            } catch (final Exception e) {
-                LOGGER.error("Error creating JmsManager using ConnectionFactory [{}] and Destination [{}].",
-                    data.connectionFactoryName, data.destinationName, e);
+            if (JndiManager.isJndiJmsEnabled()) {
+                try {
+                    return new JmsManager(name, data.jndiManager, data.connectionFactoryName, data.destinationName,
+                        data.username, data.password);
+                } catch (final Exception e) {
+                    LOGGER.error("Error creating JmsManager using ConnectionFactory [{}] and Destination [{}].",
+                        data.connectionFactoryName, data.destinationName, e);
+                    return null;
+                }
+            } else {
+                LOGGER.error("JNDI must be enabled by setting log4j2.enableJndiJms=true");
                 return null;
             }
         }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/Interpolator.java
@@ -24,6 +24,7 @@
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.config.plugins.util.PluginManager;
 import org.apache.logging.log4j.core.config.plugins.util.PluginType;
+import org.apache.logging.log4j.core.net.JndiManager;
 import org.apache.logging.log4j.core.util.Loader;
 import org.apache.logging.log4j.core.util.ReflectionUtil;
 import org.apache.logging.log4j.status.StatusLogger;
@@ -62,7 +63,10 @@ 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);
-                lookups.put(entry.getKey(), ReflectionUtil.instantiate(clazz));
+                if (!clazz.getName().equals("org.apache.logging.log4j.core.lookup.JndiLookup")
+                    || JndiManager.isJndiLookupEnabled()) {
+                    lookups.put(entry.getKey().toLowerCase(), ReflectionUtil.instantiate(clazz));
+                }
             } catch (final Exception ex) {
                 LOGGER.error("Unable to create Lookup for {}", entry.getKey(), ex);
             }
@@ -87,15 +91,16 @@ public Interpolator(final Map<String, String> properties) {
         lookups.put("main", MapLookup.MAIN_SINGLETON);
         lookups.put("java", new JavaLookup());
         // JNDI
-        try {
-            // [LOG4J2-703] We might be on Android
-            lookups.put("jndi",
-                Loader.newCheckedInstanceOf("org.apache.logging.log4j.core.lookup.JndiLookup", StrLookup.class));
-        } catch (final Throwable e) {
-            // java.lang.VerifyError: org/apache/logging/log4j/core/lookup/JndiLookup
-            LOGGER.warn(
+        if (JndiManager.isJndiLookupEnabled()) {
+            try {
+                // [LOG4J2-703] We might be on Android
+                lookups.put("jndi", Loader.newCheckedInstanceOf("org.apache.logging.log4j.core.lookup.JndiLookup", StrLookup.class));
+            } catch (final Throwable e) {
+                // java.lang.VerifyError: org/apache/logging/log4j/core/lookup/JndiLookup
+                LOGGER.warn(
                     "JNDI lookup class is not available because this JRE does not support JNDI. JNDI string lookups will not be available, continuing configuration.",
                     e);
+            }
         }
         // JMX input args
         try {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/JndiLookup.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/lookup/JndiLookup.java
@@ -38,6 +38,15 @@ public class JndiLookup extends AbstractLookup {
     /** JNDI resource path prefix used in a J2EE container */
     static final String CONTAINER_JNDI_RESOURCE_PATH_PREFIX = "java:comp/env/";
 
+    /**
+     * Constructs a new instance or throw IllegalStateException if this feature is disabled.
+     */
+    public JndiLookup() {
+        if (!JndiManager.isJndiLookupEnabled()) {
+            throw new IllegalStateException("JNDI must be enabled by setting log4j2.enableJndiLookup=true");
+        }
+    }
+
     /**
      * Looks up the value of the JNDI resource.
      * @param event The current LogEvent (is ignored by this StrLookup).
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java
@@ -17,6 +17,8 @@
 
 package org.apache.logging.log4j.core.net;
 
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.Properties;
 
 import javax.naming.Context;
@@ -26,6 +28,7 @@
 import org.apache.logging.log4j.core.appender.AbstractManager;
 import org.apache.logging.log4j.core.appender.ManagerFactory;
 import org.apache.logging.log4j.core.util.JndiCloser;
+import org.apache.logging.log4j.util.PropertiesUtil;
 
 /**
  * JNDI {@link javax.naming.Context} manager.
@@ -35,9 +38,31 @@
 public class JndiManager extends AbstractManager {
 
     private static final JndiManagerFactory FACTORY = new JndiManagerFactory();
+    private static final String PREFIX = "log4j2.enableJndi";
+    private static final String JAVA_SCHEME = "java";
 
     private final Context context;
 
+    private static boolean isJndiEnabled(final String subKey) {
+        return PropertiesUtil.getProperties().getBooleanProperty(PREFIX + subKey, false);
+    }
+
+    public static boolean isJndiEnabled() {
+        return isJndiContextSelectorEnabled() || isJndiJmsEnabled() || isJndiLookupEnabled();
+    }
+
+    public static boolean isJndiContextSelectorEnabled() {
+        return isJndiEnabled("ContextSelector");
+    }
+
+    public static boolean isJndiJmsEnabled() {
+        return isJndiEnabled("Jms");
+    }
+
+    public static boolean isJndiLookupEnabled() {
+        return isJndiEnabled("Lookup");
+    }
+
     private JndiManager(final String name, final Context context) {
         super(name);
         this.context = context;
@@ -125,13 +150,28 @@ protected void releaseSub() {
      */
     @SuppressWarnings("unchecked")
     public <T> T lookup(final String name) throws NamingException {
-        return (T) this.context.lookup(name);
+        if (context == null) {
+            return null;
+        }
+        try {
+            URI uri = new URI(name);
+            if (uri.getScheme() == null || uri.getScheme().equals(JAVA_SCHEME)) {
+                return (T) this.context.lookup(name);
+            }
+            LOGGER.warn("Unsupported JNDI URI - {}", name);
+        } catch (URISyntaxException ex) {
+            LOGGER.warn("Invalid  JNDI URI - {}", name);
+        }
+        return null;
     }
 
     private static class JndiManagerFactory implements ManagerFactory<JndiManager, Properties> {
 
         @Override
         public JndiManager createManager(final String name, final Properties data) {
+            if (!isJndiEnabled()) {
+                throw new IllegalStateException(String.format("JNDI must be enabled by setting one of the %s* properties to true", PREFIX));
+            }
             try {
                 return new JndiManager(name, new InitialContext(data));
             } catch (final NamingException e) {
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/JndiContextSelector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/selector/JndiContextSelector.java
@@ -92,6 +92,12 @@ public class JndiContextSelector implements NamedContextSelector {
 
     private static final StatusLogger LOGGER = StatusLogger.getLogger();
 
+    public JndiContextSelector() {
+        if (!JndiManager.isJndiContextSelectorEnabled()) {
+            throw new IllegalStateException("JNDI must be enabled by setting log4j2.enableJndiContextSelector=true");
+        }
+    }
+
     @Override
     public LoggerContext getContext(final String fqcn, final ClassLoader loader, final boolean currentContext) {
         return getContext(fqcn, loader, currentContext, null);
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java
@@ -59,6 +59,7 @@ public class JmsAppenderTest {
 
     @BeforeClass
     public static void setUpClass() throws Exception {
+        System.setProperty("log4j2.enableJndiJms", "true");
         MockContextFactory.setAsInitial();
         context = new InitialContext();
         context.rebind(CONNECTION_FACTORY_NAME, new QueueConnectionFactoryImpl());
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppenderWithJndiTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/routing/RoutingAppenderWithJndiTest.java
@@ -28,6 +28,7 @@
 import org.apache.logging.log4j.test.appender.ListAppender;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.mockejb.jndi.MockContextFactory;
@@ -46,6 +47,11 @@ public class RoutingAppenderWithJndiTest {
     @Rule
     public InitialLoggerContext init = new InitialLoggerContext("log4j-routing-by-jndi.xml");
 
+    @BeforeClass
+    public static void beforeClass() {
+        System.setProperty("log4j2.enableJndiLookup", "true");
+    }
+
     @Before
     public void before() throws NamingException {
         MockContextFactory.setAsInitial();
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/InterpolatorTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/InterpolatorTest.java
@@ -49,7 +49,7 @@ public class InterpolatorTest {
     @BeforeClass
     public static void before() throws NamingException {
         System.setProperty(TESTKEY, TESTVAL);
-
+        System.setProperty("log4j2.enableJndiLookup", "true");
         MockContextFactory.setAsInitial();
         context = new InitialContext();
         context.bind(JndiLookup.CONTAINER_JNDI_RESOURCE_PATH_PREFIX + TEST_CONTEXT_RESOURCE_NAME, TEST_CONTEXT_NAME);
@@ -62,7 +62,7 @@ public static void after() {
         } catch (final NamingException ignored) {
         }
         MockContextFactory.revertSetAsInitial();
-
+        System.clearProperty("log4j2.enableJndiLookup");
         System.clearProperty(TESTKEY);
     }
 
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiDisabledLookupTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiDisabledLookupTest.java
@@ -0,0 +1,63 @@
+/*
+ * 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 javax.naming.Context;
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.mockejb.jndi.MockContextFactory;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+/**
+ * JndiLookupTest
+ */
+public class JndiDisabledLookupTest {
+
+    private static final String TEST_CONTEXT_RESOURCE_NAME = "logging/context-name";
+    private static final String TEST_CONTEXT_NAME = "app-1";
+
+    private Context context;
+
+    @Before
+    public void before() throws NamingException {
+        MockContextFactory.setAsInitial();
+        context = new InitialContext();
+        context.bind(JndiLookup.CONTAINER_JNDI_RESOURCE_PATH_PREFIX + TEST_CONTEXT_RESOURCE_NAME, TEST_CONTEXT_NAME);
+    }
+
+    @After
+    public void after() {
+        try {
+            context.close();
+        } catch (final NamingException ignored) {
+        }
+        MockContextFactory.revertSetAsInitial();
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testLookup() {
+        final StrLookup lookup = new JndiLookup();
+    }
+}
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLookupTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLookupTest.java
@@ -21,7 +21,9 @@
 import javax.naming.NamingException;
 
 import org.junit.After;
+import org.junit.AfterClass;
 import org.junit.Before;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.mockejb.jndi.MockContextFactory;
 
@@ -37,6 +39,16 @@ public class JndiLookupTest {
 
     private Context context;
 
+    @AfterClass
+    public static void afterClass() {
+        System.clearProperty("log4j2.enableJndiLookup");
+    }
+
+    @BeforeClass
+    public static void beforeClass() {
+        System.setProperty("log4j2.enableJndiLookup", "true");
+    }
+
     @Before
     public void before() throws NamingException {
         MockContextFactory.setAsInitial();
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiRestrictedLookupTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiRestrictedLookupTest.java
@@ -0,0 +1,43 @@
+/*
+ * 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 org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.junit.Assert.assertNull;
+
+/**
+ * JndiLookupTest
+ */
+public class JndiRestrictedLookupTest {
+
+    private static final String DOMAIN = "apache.org";
+
+    @BeforeClass
+    public static void beforeClass() {
+        System.setProperty("log4j2.enableJndiLookup", "true");
+    }
+
+    @Test
+    public void testDnsLookup() throws Exception {
+        final StrLookup lookup = new JndiLookup();
+        String result = lookup.lookup("dns:/" + DOMAIN);
+        assertNull("DNS data returend", result);
+
+    }
+}
diff --git a/pom.xml b/pom.xml
@@ -158,6 +158,7 @@
     <Log4jReleaseCount>eighteenth</Log4jReleaseCount>
     <Log4jReleaseManager>Ralph Goers</Log4jReleaseManager>
     <Log4jReleaseKey>B3D8E1BA</Log4jReleaseKey>
+    <Log4jSigningUserName>rgoers@apache.org</Log4jSigningUserName>
     <!-- <Log4jReleaseManager>Matt Sicker</Log4jReleaseManager>
     <Log4jReleaseKey>FA1C814D</Log4jReleaseKey> -->
     <!-- note that any properties you want available in velocity templates must not use periods! -->
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
@@ -24,6 +24,10 @@
   </properties>
   <body>
     <release version="2.3.1" date="2021-12-xx" description="GA Release 2.3.1">
+      <action issue="LOG4J2-3242" dev="rgoers, ggregory" type="fix">
+        Limit JNDI to the java protocol only. JNDI will remain disabled by default. Rename JNDI enablement property from
+        'log4j2.enableJndi' to 'log4j2.enableJndiLookup', 'log4j2.enableJndiJms', and 'log4j2.enableJndiContextSelector'.
+      </action>
       <action issue="LOG4J2-3230" dev="ckozak" type="fix">
         Fix string substitution recursion.
       </action>
diff --git a/src/site/xdoc/manual/appenders.xml b/src/site/xdoc/manual/appenders.xml
@@ -917,6 +917,9 @@ public class ConnectionFactory {
         <a name="JMSTopicAppender"/>
         <subsection name="JMSAppender">
           <p>The JMSAppender sends the formatted log event to a JMS Destination.</p>
+          <p>The JMS Appender requires JNDI support so as of release 2.3.1 this appender will not function unless
+             <code>log4j2.enableJndiJms=true</code> is configured as a system property or environment
+             variable. See the <a href="./configuration.html#enableJndiJms">enableJndiJms</a> system property.</p>
           <p>
             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 JMSAppender which makes no distinction between queues
diff --git a/src/site/xdoc/manual/configuration.xml.vm b/src/site/xdoc/manual/configuration.xml.vm
