From 755e2c9d57f0517a73d16bfcaed93cc91969bdee Mon Sep 17 00:00:00 2001 From: Ralph Goers Date: Mon, 29 Nov 2021 22:28:07 -0700 Subject: [PATCH 1/4] Restrict LDAP access via JNDI --- log4j-core/pom.xml | 5 + .../log4j/core/appender/mom/JmsAppender.java | 31 ++++- .../logging/log4j/core/lookup/JndiLookup.java | 5 + .../logging/log4j/core/net/JndiManager.java | 106 ++++++++++++++- .../logging/log4j/core/util/NetUtils.java | 44 +++++++ .../log4j/core/lookup/JndiExploit.java | 36 +++++ .../log4j/core/lookup/JndiLdapLookupTest.java | 124 ++++++++++++++++++ .../src/test/resources/java-import.ldif | 4 + pom.xml | 7 + 9 files changed, 354 insertions(+), 8 deletions(-) create mode 100644 log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiExploit.java create mode 100644 log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java create mode 100644 log4j-core/src/test/resources/java-import.ldif diff --git a/log4j-core/pom.xml b/log4j-core/pom.xml index 9c6cabe7674..01b5e70b4b9 100644 --- a/log4j-core/pom.xml +++ b/log4j-core/pom.xml @@ -349,6 +349,11 @@ awaitility test + + org.zapodot + embedded-ldap-junit + test + diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsAppender.java index ddac6663a16..f2190ca91c7 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsAppender.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsAppender.java @@ -88,6 +88,12 @@ public static class Builder> extends AbstractAppender.Build @PluginBuilderAttribute private boolean immediateFail; + @PluginBuilderAttribute + private String allowedLdapClasses; + + @PluginBuilderAttribute + private String allowedLdapHosts; + // Programmatic access only for now. private JmsManager jmsManager; @@ -100,8 +106,18 @@ public JmsAppender build() { JmsManager actualJmsManager = jmsManager; JmsManagerConfiguration configuration = null; if (actualJmsManager == null) { + Properties additionalProperties = null; + if (allowedLdapClasses != null || allowedLdapHosts != null) { + additionalProperties = new Properties(); + if (allowedLdapHosts != null) { + additionalProperties.put(JndiManager.ALLOWED_HOSTS, allowedLdapHosts); + } + if (allowedLdapClasses != null) { + additionalProperties.put(JndiManager.ALLOWED_CLASSES, allowedLdapClasses); + } + } final Properties jndiProperties = JndiManager.createProperties(factoryName, providerUrl, urlPkgPrefixes, - securityPrincipalName, securityCredentials, null); + securityPrincipalName, securityCredentials, additionalProperties); configuration = new JmsManagerConfiguration(jndiProperties, factoryBindingName, destinationBindingName, userName, password, false, reconnectIntervalMillis); actualJmsManager = AbstractManager.getManager(getName(), JmsManager.FACTORY, configuration); @@ -202,6 +218,16 @@ public Builder setUserName(final String userName) { return this; } + public Builder setAllowedLdapClasses(final String allowedLdapClasses) { + this.allowedLdapClasses = allowedLdapClasses; + return this; + } + + public Builder setAllowedLdapHosts(final String allowedLdapHosts) { + this.allowedLdapHosts = allowedLdapHosts; + return this; + } + /** * Does not include the password. */ @@ -212,7 +238,8 @@ public String toString() { + ", securityCredentials=" + securityCredentials + ", factoryBindingName=" + factoryBindingName + ", destinationBindingName=" + destinationBindingName + ", username=" + userName + ", layout=" + getLayout() + ", filter=" + getFilter() + ", ignoreExceptions=" + isIgnoreExceptions() - + ", jmsManager=" + jmsManager + "]"; + + ", jmsManager=" + jmsManager + ", allowedLdapClasses=" + allowedLdapClasses + + ", allowedLdapHosts=" + allowedLdapHosts + "]"; } } 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 index 30e65ad24f4..e57d0bebba2 100644 --- 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 @@ -16,6 +16,11 @@ */ package org.apache.logging.log4j.core.lookup; +import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.List; import java.util.Objects; import javax.naming.NamingException; 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 index 26708578848..8a0e68dc66d 100644 --- 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,31 +17,76 @@ package org.apache.logging.log4j.core.net; +import java.io.Serializable; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; import java.util.Properties; import java.util.concurrent.TimeUnit; import javax.naming.Context; -import javax.naming.InitialContext; +import javax.naming.NameClassPair; +import javax.naming.NamingEnumeration; import javax.naming.NamingException; +import javax.naming.Referenceable; +import javax.naming.directory.Attribute; +import javax.naming.directory.Attributes; +import javax.naming.directory.DirContext; +import javax.naming.directory.InitialDirContext; 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.core.util.NetUtils; +import org.apache.logging.log4j.util.PropertiesUtil; /** - * Manages a JNDI {@link javax.naming.Context}. + * Manages a JNDI {@link javax.naming.directory.DirContext}. * * @since 2.1 */ public class JndiManager extends AbstractManager { + public static final String ALLOWED_HOSTS = "allowedLdapHosts"; + public static final String ALLOWED_CLASSES = "allowedLdapClasses"; + private static final JndiManagerFactory FACTORY = new JndiManagerFactory(); + private static final String PREFIX = "log4j2."; + private static final List permanentAllowedHosts = new ArrayList<>(); + private static final List permanentAllowedClasses = new ArrayList<>(); + private static final String LDAP = "ldap"; + private static final String SERIALIZED_DATA = "javaserializeddata"; + private static final String CLASS_NAME = "javaclassname"; + private static final String REFERENCE_ADDRESS = "javareferenceaddress"; + private static final String OBJECT_FACTORY = "javafactory"; + private final List allowedHosts; + private final List allowedClasses; + + static { + permanentAllowedHosts.addAll(NetUtils.getLocalIps()); + permanentAllowedClasses.add(Boolean.class.getName()); + permanentAllowedClasses.add(Byte.class.getName()); + permanentAllowedClasses.add(Character.class.getName()); + permanentAllowedClasses.add(Double.class.getName()); + permanentAllowedClasses.add(Float.class.getName()); + permanentAllowedClasses.add(Integer.class.getName()); + permanentAllowedClasses.add(Long.class.getName()); + permanentAllowedClasses.add(Number.class.getName()); + permanentAllowedClasses.add(Short.class.getName()); + permanentAllowedClasses.add(String.class.getName()); + } - private final Context context; - private JndiManager(final String name, final Context context) { + private final DirContext context; + + private JndiManager(final String name, final DirContext context, final List allowedHosts, + final List allowedClasses) { super(null, name); this.context = context; + this.allowedHosts = allowedHosts; + this.allowedClasses = allowedClasses; } /** @@ -168,7 +213,37 @@ protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) { * @throws NamingException if a naming exception is encountered */ @SuppressWarnings("unchecked") - public T lookup(final String name) throws NamingException { + public synchronized T lookup(final String name) throws NamingException { + try { + URI uri = new URI(name); + if (LDAP.equalsIgnoreCase(uri.getScheme())) { + if (!allowedHosts.contains(uri.getHost())) { + LOGGER.warn("Attempt to access ldap server not in allowed list"); + return null; + } + Attributes attributes = this.context.getAttributes(name); + if (attributes != null) { + Attribute classNameAttr = attributes.get(CLASS_NAME); + if (attributes.get(SERIALIZED_DATA) != null) { + if (classNameAttr != null) { + String className = classNameAttr.get().toString(); + if (!allowedClasses.contains(className)) { + LOGGER.warn("Deserialization of {} is not allowed", className); + return null; + } + } else { + LOGGER.warn("No class name provided for {}", name); + return null; + } + } else if (attributes.get(REFERENCE_ADDRESS) != null || attributes.get(OBJECT_FACTORY) != null){ + LOGGER.warn("Referenceable class is not allowed for {}", name); + return null; + } + } + } + } catch (URISyntaxException ex) { + // This is OK. + } return (T) this.context.lookup(name); } @@ -176,13 +251,32 @@ private static class JndiManagerFactory implements ManagerFactory allowedHosts = new ArrayList<>(); + List allowedClasses = new ArrayList<>(); + addAll(hosts, allowedHosts, permanentAllowedHosts, ALLOWED_HOSTS, data); + addAll(classes, allowedClasses, permanentAllowedClasses, ALLOWED_CLASSES, data); try { - return new JndiManager(name, new InitialContext(data)); + return new JndiManager(name, new InitialDirContext(data), allowedHosts, allowedClasses); } catch (final NamingException e) { LOGGER.error("Error creating JNDI InitialContext.", e); return null; } } + + private void addAll(String toSplit, List list, List permanentList, String propertyName, + Properties data) { + if (toSplit != null) { + list.addAll(Arrays.asList(toSplit.split("\\s*,\\s*"))); + data.remove(propertyName); + } + toSplit = PropertiesUtil.getProperties().getStringProperty(PREFIX + propertyName); + if (toSplit != null) { + list.addAll(Arrays.asList(toSplit.split("\\s*,\\s*"))); + } + list.addAll(permanentList); + } } @Override diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java index 8a8353e7f90..ddbe437e246 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java @@ -17,6 +17,8 @@ package org.apache.logging.log4j.core.util; import java.io.File; +import java.net.Inet4Address; +import java.net.Inet6Address; import java.net.InetAddress; import java.net.MalformedURLException; import java.net.NetworkInterface; @@ -25,11 +27,14 @@ import java.net.URISyntaxException; import java.net.URL; import java.net.UnknownHostException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Enumeration; +import java.util.List; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.status.StatusLogger; +import org.apache.logging.log4j.util.Strings; /** * Networking-related convenience methods. @@ -79,6 +84,45 @@ public static String getLocalHostname() { } } + public static List getLocalIps() { + List localIps = new ArrayList<>(); + localIps.add("localhost"); + localIps.add("127.0.0.1"); + try { + final InetAddress addr = Inet4Address.getLocalHost(); + setHostName(addr, localIps); + } catch (final UnknownHostException ex) { + // Ignore this. + } + try { + final Enumeration interfaces = NetworkInterface.getNetworkInterfaces(); + if (interfaces != null) { + while (interfaces.hasMoreElements()) { + final NetworkInterface nic = interfaces.nextElement(); + final Enumeration addresses = nic.getInetAddresses(); + while (addresses.hasMoreElements()) { + final InetAddress address = addresses.nextElement(); + setHostName(address, localIps); + } + } + } + } catch (final SocketException se) { + // ignore. + } + return localIps; + } + + private static void setHostName(InetAddress address, List localIps) { + String[] parts = address.toString().split("\\s*/\\s*"); + if (parts.length > 0) { + for (String part : parts) { + if (Strings.isNotBlank(part) && !localIps.contains(part)) { + localIps.add(part); + } + } + } + } + /** * Returns the local network interface's MAC address if possible. The local network interface is defined here as * the {@link java.net.NetworkInterface} that is both up and not a loopback interface. diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiExploit.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiExploit.java new file mode 100644 index 00000000000..7b0ae4631c3 --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiExploit.java @@ -0,0 +1,36 @@ +/* + * 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.Name; +import javax.naming.spi.ObjectFactory; +import java.util.Hashtable; + +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Test LDAP object + */ +public class JndiExploit implements ObjectFactory { + @Override + public Object getObjectInstance(Object obj, Name name, Context nameCtx, Hashtable environment) + throws Exception { + fail("getObjectInstance must not be allowed"); + return null; + } +} diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java new file mode 100644 index 00000000000..73129f5a911 --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java @@ -0,0 +1,124 @@ +/* + * 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.NamingException; +import javax.naming.Reference; +import javax.naming.Referenceable; +import javax.naming.StringRefAddr; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.message.SimpleMessage; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.zapodot.junit.ldap.EmbeddedLdapRule; +import org.zapodot.junit.ldap.EmbeddedLdapRuleBuilder; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +/** + * JndiLookupTest + */ +public class JndiLdapLookupTest { + + private static final String LDAP_URL = "ldap://127.0.0.1:"; + private static final String RESOURCE = "JndiExploit"; + private static final String TEST_STRING = "TestString"; + private static final String TEST_MESSAGE = "TestMessage"; + private static final String LEVEL = "TestLevel"; + public static final String DOMAIN_DSN = "dc=apache,dc=org"; + + @Rule + public EmbeddedLdapRule embeddedLdapRule = EmbeddedLdapRuleBuilder.newInstance().usingDomainDsn(DOMAIN_DSN) + .importingLdifs("java-import.ldif").build(); + + @BeforeClass + public static void beforeClass() { + System.setProperty("log4j2.allowedLdapClasses", Level.class.getName()); + } + + @Test + public void testReferenceLookup() throws Exception { + int port = embeddedLdapRule.embeddedServerPort(); + Context context = embeddedLdapRule.context(); + context.bind( "cn=" + RESOURCE +"," + DOMAIN_DSN, new Fruit("Test Message")); + final StrLookup lookup = new JndiLookup(); + String result = lookup.lookup(LDAP_URL + port + "/" + "cn=" + RESOURCE + "," + DOMAIN_DSN); + if (result != null) { + fail("Lookup returned an object"); + } + } + + @Test + public void testSerializableLookup() throws Exception { + int port = embeddedLdapRule.embeddedServerPort(); + Context context = embeddedLdapRule.context(); + context.bind( "cn=" + TEST_STRING +"," + DOMAIN_DSN, "Test Message"); + final StrLookup lookup = new JndiLookup(); + String result = lookup.lookup(LDAP_URL + port + "/" + "cn=" + TEST_STRING + "," + DOMAIN_DSN); + if (result == null) { + fail("Lookup failed to return the test string"); + } + assertEquals("Incorrect message returned", "Test Message", result); + } + + @Test + public void testBadSerializableLookup() throws Exception { + int port = embeddedLdapRule.embeddedServerPort(); + Context context = embeddedLdapRule.context(); + context.bind( "cn=" + TEST_MESSAGE +"," + DOMAIN_DSN, new SimpleMessage("Test Message")); + final StrLookup lookup = new JndiLookup(); + String result = lookup.lookup(LDAP_URL + port + "/" + "cn=" + TEST_MESSAGE + "," + DOMAIN_DSN); + if (result != null) { + fail("Lookup returned an object"); + } + } + + @Test + public void testSpecialSerializableLookup() throws Exception { + int port = embeddedLdapRule.embeddedServerPort(); + Context context = embeddedLdapRule.context(); + context.bind( "cn=" + LEVEL +"," + DOMAIN_DSN, Level.ERROR); + final StrLookup lookup = new JndiLookup(); + String result = lookup.lookup(LDAP_URL + port + "/" + "cn=" + LEVEL + "," + DOMAIN_DSN); + if (result == null) { + fail("Lookup failed to return the level"); + } + assertEquals("Incorrect level returned", Level.ERROR.toString(), result); + } + + class Fruit implements Referenceable { + String fruit; + public Fruit(String f) { + fruit = f; + } + + public Reference getReference() throws NamingException { + + return new Reference(Fruit.class.getName(), new StringRefAddr("fruit", + fruit), JndiExploit.class.getName(), null); // factory location + } + + public String toString() { + return fruit; + } + } + +} diff --git a/log4j-core/src/test/resources/java-import.ldif b/log4j-core/src/test/resources/java-import.ldif new file mode 100644 index 00000000000..35daf435c8c --- /dev/null +++ b/log4j-core/src/test/resources/java-import.ldif @@ -0,0 +1,4 @@ +dn: dc=apache,dc=org +objectClass: domain +objectClass: top +dc: apache \ No newline at end of file diff --git a/pom.xml b/pom.xml index 1e7204c7e42..9a55dda8806 100644 --- a/pom.xml +++ b/pom.xml @@ -954,6 +954,13 @@ 3.0.0 test + + + org.zapodot + embedded-ldap-junit + 0.8.1 + test + From fe3d988db456640c96412e6c2a27ea2d40e51615 Mon Sep 17 00:00:00 2001 From: Ralph Goers Date: Tue, 30 Nov 2021 17:20:24 -0700 Subject: [PATCH 2/4] Disable most JNDI protocols --- .../log4j/core/appender/mom/JmsAppender.java | 13 ++++- .../logging/log4j/core/lookup/JndiLookup.java | 5 -- .../logging/log4j/core/net/JndiManager.java | 47 +++++++++---------- .../log4j/core/lookup/JndiLdapLookupTest.java | 22 ++++++++- 4 files changed, 56 insertions(+), 31 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsAppender.java index f2190ca91c7..35f5b10569e 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsAppender.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/mom/JmsAppender.java @@ -94,6 +94,9 @@ public static class Builder> extends AbstractAppender.Build @PluginBuilderAttribute private String allowedLdapHosts; + @PluginBuilderAttribute + private String allowedJndiProtocols; + // Programmatic access only for now. private JmsManager jmsManager; @@ -115,6 +118,9 @@ public JmsAppender build() { if (allowedLdapClasses != null) { additionalProperties.put(JndiManager.ALLOWED_CLASSES, allowedLdapClasses); } + if (allowedJndiProtocols != null) { + additionalProperties.put(JndiManager.ALLOWED_PROTOCOLS, allowedJndiProtocols); + } } final Properties jndiProperties = JndiManager.createProperties(factoryName, providerUrl, urlPkgPrefixes, securityPrincipalName, securityCredentials, additionalProperties); @@ -228,6 +234,11 @@ public Builder setAllowedLdapHosts(final String allowedLdapHosts) { return this; } + public Builder setAllowedJndiProtocols(final String allowedJndiProtocols) { + this.allowedJndiProtocols = allowedJndiProtocols; + return this; + } + /** * Does not include the password. */ @@ -239,7 +250,7 @@ public String toString() { + ", destinationBindingName=" + destinationBindingName + ", username=" + userName + ", layout=" + getLayout() + ", filter=" + getFilter() + ", ignoreExceptions=" + isIgnoreExceptions() + ", jmsManager=" + jmsManager + ", allowedLdapClasses=" + allowedLdapClasses - + ", allowedLdapHosts=" + allowedLdapHosts + "]"; + + ", allowedLdapHosts=" + allowedLdapHosts + ", allowedJndiProtocols=" + allowedJndiProtocols + "]"; } } 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 index e57d0bebba2..30e65ad24f4 100644 --- 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 @@ -16,11 +16,6 @@ */ package org.apache.logging.log4j.core.lookup; -import java.net.MalformedURLException; -import java.net.URI; -import java.net.URISyntaxException; -import java.util.ArrayList; -import java.util.List; import java.util.Objects; import javax.naming.NamingException; 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 index 8a0e68dc66d..613a0551da9 100644 --- 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,20 +17,17 @@ package org.apache.logging.log4j.core.net; -import java.io.Serializable; import java.net.URI; import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Locale; import java.util.Properties; import java.util.concurrent.TimeUnit; import javax.naming.Context; -import javax.naming.NameClassPair; -import javax.naming.NamingEnumeration; import javax.naming.NamingException; -import javax.naming.Referenceable; import javax.naming.directory.Attribute; import javax.naming.directory.Attributes; import javax.naming.directory.DirContext; @@ -51,42 +48,36 @@ public class JndiManager extends AbstractManager { public static final String ALLOWED_HOSTS = "allowedLdapHosts"; public static final String ALLOWED_CLASSES = "allowedLdapClasses"; + public static final String ALLOWED_PROTOCOLS = "allowedJndiProtocols"; private static final JndiManagerFactory FACTORY = new JndiManagerFactory(); private static final String PREFIX = "log4j2."; - private static final List permanentAllowedHosts = new ArrayList<>(); - private static final List permanentAllowedClasses = new ArrayList<>(); private static final String LDAP = "ldap"; + private static final String LDAPS = "ldaps"; + private static final String JAVA = "java"; + private static final List permanentAllowedHosts = NetUtils.getLocalIps(); + private static final List permanentAllowedClasses = Arrays.asList(Boolean.class.getName(), + Byte.class.getName(), Character.class.getName(), Double.class.getName(), Float.class.getName(), + Integer.class.getName(), Long.class.getName(), Number.class.getName(), Short.class.getName(), + String.class.getName()); + private static final List permanentAllowedProtocols = Arrays.asList(JAVA, LDAP, LDAPS); private static final String SERIALIZED_DATA = "javaserializeddata"; private static final String CLASS_NAME = "javaclassname"; private static final String REFERENCE_ADDRESS = "javareferenceaddress"; private static final String OBJECT_FACTORY = "javafactory"; private final List allowedHosts; private final List allowedClasses; - - static { - permanentAllowedHosts.addAll(NetUtils.getLocalIps()); - permanentAllowedClasses.add(Boolean.class.getName()); - permanentAllowedClasses.add(Byte.class.getName()); - permanentAllowedClasses.add(Character.class.getName()); - permanentAllowedClasses.add(Double.class.getName()); - permanentAllowedClasses.add(Float.class.getName()); - permanentAllowedClasses.add(Integer.class.getName()); - permanentAllowedClasses.add(Long.class.getName()); - permanentAllowedClasses.add(Number.class.getName()); - permanentAllowedClasses.add(Short.class.getName()); - permanentAllowedClasses.add(String.class.getName()); - } - + private final List allowedProtocols; private final DirContext context; private JndiManager(final String name, final DirContext context, final List allowedHosts, - final List allowedClasses) { + final List allowedClasses, final List allowedProtocols) { super(null, name); this.context = context; this.allowedHosts = allowedHosts; this.allowedClasses = allowedClasses; + this.allowedProtocols = allowedProtocols; } /** @@ -216,7 +207,11 @@ protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) { public synchronized T lookup(final String name) throws NamingException { try { URI uri = new URI(name); - if (LDAP.equalsIgnoreCase(uri.getScheme())) { + if (!allowedProtocols.contains(uri.getScheme().toLowerCase(Locale.ROOT))) { + LOGGER.warn("Log4j JNDI does not allow protocol {}", uri.getScheme()); + return null; + } + if (LDAP.equalsIgnoreCase(uri.getScheme()) || LDAPS.equalsIgnoreCase(uri.getScheme())) { if (!allowedHosts.contains(uri.getHost())) { LOGGER.warn("Attempt to access ldap server not in allowed list"); return null; @@ -253,12 +248,16 @@ private static class JndiManagerFactory implements ManagerFactory allowedHosts = new ArrayList<>(); List allowedClasses = new ArrayList<>(); + List 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); + return new JndiManager(name, new InitialDirContext(data), allowedHosts, allowedClasses, + allowedProtocols); } catch (final NamingException e) { LOGGER.error("Error creating JNDI InitialContext.", e); return null; diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java index 73129f5a911..a26d927da46 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java @@ -43,7 +43,8 @@ public class JndiLdapLookupTest { private static final String TEST_STRING = "TestString"; private static final String TEST_MESSAGE = "TestMessage"; private static final String LEVEL = "TestLevel"; - public static final String DOMAIN_DSN = "dc=apache,dc=org"; + private static final String DOMAIN_DSN = "dc=apache,dc=org"; + private static final String DOMAIN = "apache.org"; @Rule public EmbeddedLdapRule embeddedLdapRule = EmbeddedLdapRuleBuilder.newInstance().usingDomainDsn(DOMAIN_DSN) @@ -52,6 +53,7 @@ public class JndiLdapLookupTest { @BeforeClass public static void beforeClass() { System.setProperty("log4j2.allowedLdapClasses", Level.class.getName()); + System.setProperty("log4j2.allowedJndiProtocols", "dns"); } @Test @@ -104,6 +106,24 @@ public void testSpecialSerializableLookup() throws Exception { assertEquals("Incorrect level returned", Level.ERROR.toString(), result); } + @Test + public void testDnsLookup() throws Exception { + final StrLookup lookup = new JndiLookup(); + String result = lookup.lookup("dns:/" + DOMAIN); + if (result == null) { + fail("No DNS data returned"); + } + } + + @Test + public void testNisLookup() throws Exception { + final StrLookup lookup = new JndiLookup(); + String result = lookup.lookup("nis:/" + DOMAIN); + if (result != null) { + fail("NIS information should not have been returned"); + } + } + class Fruit implements Referenceable { String fruit; public Fruit(String f) { From 5f81dd218aab36bf1c6a7410c88c29594bb1a0e7 Mon Sep 17 00:00:00 2001 From: Ralph Goers Date: Tue, 30 Nov 2021 22:38:22 -0700 Subject: [PATCH 3/4] Rename test. Various minor fixes --- .../logging/log4j/core/net/JndiManager.java | 31 +++++++++++++------ ...est.java => JndiRestrictedLookupTest.java} | 4 +-- ...-import.ldif => JndiRestrictedLookup.ldif} | 0 3 files changed, 24 insertions(+), 11 deletions(-) rename log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/{JndiLdapLookupTest.java => JndiRestrictedLookupTest.java} (98%) rename log4j-core/src/test/resources/{java-import.ldif => JndiRestrictedLookup.ldif} (100%) 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 index 613a0551da9..b392b938b46 100644 --- 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 @@ -21,12 +21,15 @@ import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Properties; import java.util.concurrent.TimeUnit; import javax.naming.Context; +import javax.naming.NamingEnumeration; import javax.naming.NamingException; import javax.naming.directory.Attribute; import javax.naming.directory.Attributes; @@ -58,13 +61,12 @@ public class JndiManager extends AbstractManager { private static final List permanentAllowedHosts = NetUtils.getLocalIps(); private static final List permanentAllowedClasses = Arrays.asList(Boolean.class.getName(), Byte.class.getName(), Character.class.getName(), Double.class.getName(), Float.class.getName(), - Integer.class.getName(), Long.class.getName(), Number.class.getName(), Short.class.getName(), - String.class.getName()); + Integer.class.getName(), Long.class.getName(), Short.class.getName(), String.class.getName()); private static final List permanentAllowedProtocols = Arrays.asList(JAVA, LDAP, LDAPS); - private static final String SERIALIZED_DATA = "javaserializeddata"; - private static final String CLASS_NAME = "javaclassname"; - private static final String REFERENCE_ADDRESS = "javareferenceaddress"; - private static final String OBJECT_FACTORY = "javafactory"; + private static final String SERIALIZED_DATA = "javaSerializedData"; + private static final String CLASS_NAME = "javaClassName"; + private static final String REFERENCE_ADDRESS = "javaReferenceAddress"; + private static final String OBJECT_FACTORY = "javaFactory"; private final List allowedHosts; private final List allowedClasses; private final List allowedProtocols; @@ -218,8 +220,18 @@ public synchronized T lookup(final String name) throws NamingException { } Attributes attributes = this.context.getAttributes(name); if (attributes != null) { - Attribute classNameAttr = attributes.get(CLASS_NAME); - if (attributes.get(SERIALIZED_DATA) != null) { + // In testing the "key" for attributes seems to be lowercase while the attribute id is + // camelcase, but that may just be true for the test LDAP used here. This copies the Attributes + // to a Map ignoring the "key" and using the Attribute's id as the key in the Map so it matches + // the Java schema. + Map attributeMap = new HashMap<>(); + NamingEnumeration enumeration = attributes.getAll(); + while (enumeration.hasMore()) { + Attribute attribute = enumeration.next(); + attributeMap.put(attribute.getID(), attribute); + } + Attribute classNameAttr = attributeMap.get(CLASS_NAME); + if (attributeMap.get(SERIALIZED_DATA) != null) { if (classNameAttr != null) { String className = classNameAttr.get().toString(); if (!allowedClasses.contains(className)) { @@ -230,7 +242,8 @@ public synchronized T lookup(final String name) throws NamingException { LOGGER.warn("No class name provided for {}", name); return null; } - } else if (attributes.get(REFERENCE_ADDRESS) != null || attributes.get(OBJECT_FACTORY) != null){ + } else if (attributeMap.get(REFERENCE_ADDRESS) != null + || attributeMap.get(OBJECT_FACTORY) != null) { LOGGER.warn("Referenceable class is not allowed for {}", name); return null; } diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiRestrictedLookupTest.java similarity index 98% rename from log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java rename to log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiRestrictedLookupTest.java index a26d927da46..032c9c4d852 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiRestrictedLookupTest.java @@ -36,7 +36,7 @@ /** * JndiLookupTest */ -public class JndiLdapLookupTest { +public class JndiRestrictedLookupTest { private static final String LDAP_URL = "ldap://127.0.0.1:"; private static final String RESOURCE = "JndiExploit"; @@ -48,7 +48,7 @@ public class JndiLdapLookupTest { @Rule public EmbeddedLdapRule embeddedLdapRule = EmbeddedLdapRuleBuilder.newInstance().usingDomainDsn(DOMAIN_DSN) - .importingLdifs("java-import.ldif").build(); + .importingLdifs("JndiRestrictedLookup.ldif").build(); @BeforeClass public static void beforeClass() { diff --git a/log4j-core/src/test/resources/java-import.ldif b/log4j-core/src/test/resources/JndiRestrictedLookup.ldif similarity index 100% rename from log4j-core/src/test/resources/java-import.ldif rename to log4j-core/src/test/resources/JndiRestrictedLookup.ldif From 7fe72d62fcb9246be792b946e405e1d40d402780 Mon Sep 17 00:00:00 2001 From: Ralph Goers Date: Sat, 4 Dec 2021 20:59:39 -0700 Subject: [PATCH 4/4] LOG4J2-3201 - Limit the protocols JNDI can use by default. Limit the servers and classes that can be accessed via LDAP. --- .../logging/log4j/core/net/JndiManager.java | 62 ++++++++++--------- .../logging/log4j/core/util/NetUtils.java | 4 ++ src/changes/changes.xml | 3 + src/site/xdoc/manual/appenders.xml | 27 ++++++++ src/site/xdoc/manual/configuration.xml.vm | 26 ++++++++ src/site/xdoc/manual/extending.xml | 5 +- src/site/xdoc/manual/lookups.xml | 7 +++ 7 files changed, 103 insertions(+), 31 deletions(-) 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 index b392b938b46..2d7604fee68 100644 --- 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 @@ -209,43 +209,45 @@ protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) { public synchronized T lookup(final String name) throws NamingException { try { URI uri = new URI(name); - if (!allowedProtocols.contains(uri.getScheme().toLowerCase(Locale.ROOT))) { - LOGGER.warn("Log4j JNDI does not allow protocol {}", uri.getScheme()); - return null; - } - if (LDAP.equalsIgnoreCase(uri.getScheme()) || LDAPS.equalsIgnoreCase(uri.getScheme())) { - if (!allowedHosts.contains(uri.getHost())) { - LOGGER.warn("Attempt to access ldap server not in allowed list"); + if (uri.getScheme() != null) { + if (!allowedProtocols.contains(uri.getScheme().toLowerCase(Locale.ROOT))) { + LOGGER.warn("Log4j JNDI does not allow protocol {}", uri.getScheme()); return null; } - Attributes attributes = this.context.getAttributes(name); - if (attributes != null) { - // In testing the "key" for attributes seems to be lowercase while the attribute id is - // camelcase, but that may just be true for the test LDAP used here. This copies the Attributes - // to a Map ignoring the "key" and using the Attribute's id as the key in the Map so it matches - // the Java schema. - Map attributeMap = new HashMap<>(); - NamingEnumeration enumeration = attributes.getAll(); - while (enumeration.hasMore()) { - Attribute attribute = enumeration.next(); - attributeMap.put(attribute.getID(), attribute); + if (LDAP.equalsIgnoreCase(uri.getScheme()) || LDAPS.equalsIgnoreCase(uri.getScheme())) { + if (!allowedHosts.contains(uri.getHost())) { + LOGGER.warn("Attempt to access ldap server not in allowed list"); + return null; } - Attribute classNameAttr = attributeMap.get(CLASS_NAME); - if (attributeMap.get(SERIALIZED_DATA) != null) { - if (classNameAttr != null) { - String className = classNameAttr.get().toString(); - if (!allowedClasses.contains(className)) { - LOGGER.warn("Deserialization of {} is not allowed", className); + Attributes attributes = this.context.getAttributes(name); + if (attributes != null) { + // In testing the "key" for attributes seems to be lowercase while the attribute id is + // camelcase, but that may just be true for the test LDAP used here. This copies the Attributes + // to a Map ignoring the "key" and using the Attribute's id as the key in the Map so it matches + // the Java schema. + Map attributeMap = new HashMap<>(); + NamingEnumeration enumeration = attributes.getAll(); + while (enumeration.hasMore()) { + Attribute attribute = enumeration.next(); + attributeMap.put(attribute.getID(), attribute); + } + Attribute classNameAttr = attributeMap.get(CLASS_NAME); + if (attributeMap.get(SERIALIZED_DATA) != null) { + if (classNameAttr != null) { + String className = classNameAttr.get().toString(); + if (!allowedClasses.contains(className)) { + LOGGER.warn("Deserialization of {} is not allowed", className); + return null; + } + } else { + LOGGER.warn("No class name provided for {}", name); return null; } - } else { - LOGGER.warn("No class name provided for {}", name); + } else if (attributeMap.get(REFERENCE_ADDRESS) != null + || attributeMap.get(OBJECT_FACTORY) != null) { + LOGGER.warn("Referenceable class is not allowed for {}", name); return null; } - } else if (attributeMap.get(REFERENCE_ADDRESS) != null - || attributeMap.get(OBJECT_FACTORY) != null) { - LOGGER.warn("Referenceable class is not allowed for {}", name); - return null; } } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java index ddbe437e246..661f74f90b1 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java @@ -84,6 +84,10 @@ public static String getLocalHostname() { } } + /** + * Returns all the local host names and ip addresses. + * @return The local host names and ip addresses. + */ public static List getLocalIps() { List localIps = new ArrayList<>(); localIps.add("localhost"); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 247face4e56..50893bfd204 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -110,6 +110,9 @@ Improve PatternLayout performance by reducing unnecessary indirection and branching. + + Limit the protocols JNDI can use by default. Limit the servers and classes that can be accessed via LDAP. + Enable immediate flush on RollingFileAppender when buffered i/o is not enabled. diff --git a/src/site/xdoc/manual/appenders.xml b/src/site/xdoc/manual/appenders.xml index 72497446009..d634b8a9e26 100644 --- a/src/site/xdoc/manual/appenders.xml +++ b/src/site/xdoc/manual/appenders.xml @@ -1555,6 +1555,33 @@ public class ConnectionFactory { Default Description + + allowdLdapClasses + String + null + + A comma separated list of fully qualified class names that may be accessed by LDAP. The classes + must implement Serializable. Only applies when the JMS Appender By default only Java primative classes are allowed. + + + + allowdLdapHosts + String + null + + A comma separated list of host names or ip addresses that may be accessed by LDAP. By default only + the local host names and ip addresses are allowed. + + + + allowdJndiProtocols + String + null + + A comma separated list of protocol names that JNDI will allow. By default only java, ldap, and ldaps + are the only allowed protocols. + + factoryBindingName String diff --git a/src/site/xdoc/manual/configuration.xml.vm b/src/site/xdoc/manual/configuration.xml.vm index e298599e79f..5393fed53c1 100644 --- a/src/site/xdoc/manual/configuration.xml.vm +++ b/src/site/xdoc/manual/configuration.xml.vm @@ -2146,6 +2146,32 @@ public class AwesomeTest { before falling back to the default class loader. + + log4j2.allowedLdapClasses + LOG4J_ALLOWED_LDAP_CLASSES +   + + 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. + + + + log4j2.allowedLdapHosts + LOG4J_ALLOWED_LDAP_HOSTS +   + + 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. + + + + log4j2.allowedJndiProtocols + LOG4J_ALLOWED_JNDI_PROTOCOLS +   + + System property that adds protocol names that JNDI will allow. By default it only allows java, ldap, and ldaps. + + log4j2.uuidSequence
diff --git a/src/site/xdoc/manual/extending.xml b/src/site/xdoc/manual/extending.xml index ba04d68e00e..04c742aa808 100644 --- a/src/site/xdoc/manual/extending.xml +++ b/src/site/xdoc/manual/extending.xml @@ -92,7 +92,10 @@
Associates LoggerContexts with the ClassLoader that created the caller of the getLogger call. This is the default ContextSelector.
JndiContextSelector
-
Locates the LoggerContext by querying JNDI.
+
Locates the LoggerContext by querying JNDI. Please see log4j2.allowedJndiProtocols, + log4j2.allowedLdapClasses, and + log4j2.allowedLdapHosts for restrictions on using JNDI + with Log4j.
AsyncLoggerContextSelector
Creates a LoggerContext that ensures that all loggers are AsyncLoggers.
BundleContextSelector
diff --git a/src/site/xdoc/manual/lookups.xml b/src/site/xdoc/manual/lookups.xml index d699e784ce7..cc6a66f0a6f 100644 --- a/src/site/xdoc/manual/lookups.xml +++ b/src/site/xdoc/manual/lookups.xml @@ -270,6 +270,13 @@ 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.

+

By default the JDNI Lookup only supports the java, ldap, and ldaps protocols or no protocol. Additional + protocols may be supported by specifying them on the log4j2.allowedJndiProtocols property. + When using LDAP Java classes that implement the Referenceable interface are not supported for security + reasons. Only the Java primative classes are supported by default as well as any classes specified by the + log4j2.allowedLdapClasses property. When using LDAP only references to the local host name + or ip address are supported along with any hosts or ip addresses listed in the + log4j2.allowedLdapHosts property.