You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rg...@apache.org on 2021/11/30 05:28:51 UTC
[logging-log4j2] branch ldap-controls created (now 755e2c9)
This is an automated email from the ASF dual-hosted git repository.
rgoers pushed a change to branch ldap-controls
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.
at 755e2c9 Restrict LDAP access via JNDI
This branch includes the following new commits:
new 755e2c9 Restrict LDAP access via JNDI
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[logging-log4j2] 01/01: Restrict LDAP access via JNDI
Posted by rg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rgoers pushed a commit to branch ldap-controls
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 755e2c9d57f0517a73d16bfcaed93cc91969bdee
Author: Ralph Goers <rg...@apache.org>
AuthorDate: Mon Nov 29 22:28:07 2021 -0700
Restrict LDAP access via JNDI
---
log4j-core/pom.xml | 5 +
.../log4j/core/appender/mom/JmsAppender.java | 31 +++++-
.../logging/log4j/core/lookup/JndiLookup.java | 5 +
.../apache/logging/log4j/core/net/JndiManager.java | 106 +++++++++++++++++-
.../apache/logging/log4j/core/util/NetUtils.java | 44 ++++++++
.../logging/log4j/core/lookup/JndiExploit.java | 36 ++++++
.../log4j/core/lookup/JndiLdapLookupTest.java | 124 +++++++++++++++++++++
log4j-core/src/test/resources/java-import.ldif | 4 +
pom.xml | 7 ++
9 files changed, 354 insertions(+), 8 deletions(-)
diff --git a/log4j-core/pom.xml b/log4j-core/pom.xml
index 9c6cabe..01b5e70 100644
--- a/log4j-core/pom.xml
+++ b/log4j-core/pom.xml
@@ -349,6 +349,11 @@
<artifactId>awaitility</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.zapodot</groupId>
+ <artifactId>embedded-ldap-junit</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
<plugins>
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 ddac666..f2190ca 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 class JmsAppender extends AbstractAppender {
@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 class JmsAppender extends AbstractAppender {
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 class JmsAppender extends AbstractAppender {
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 class JmsAppender extends AbstractAppender {
+ ", 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 30e65ad..e57d0be 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 2670857..8a0e68d 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<String> permanentAllowedHosts = new ArrayList<>();
+ private static final List<String> 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<String> allowedHosts;
+ private final List<String> 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<String> allowedHosts,
+ final List<String> allowedClasses) {
super(null, name);
this.context = context;
+ this.allowedHosts = allowedHosts;
+ this.allowedClasses = allowedClasses;
}
/**
@@ -168,7 +213,37 @@ public class JndiManager extends AbstractManager {
* @throws NamingException if a naming exception is encountered
*/
@SuppressWarnings("unchecked")
- public <T> T lookup(final String name) throws NamingException {
+ public synchronized <T> 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 @@ public class JndiManager extends AbstractManager {
@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;
+ List<String> allowedHosts = new ArrayList<>();
+ List<String> 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<String> list, List<String> 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 8a8353e..ddbe437 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.URI;
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 final class NetUtils {
}
}
+ public static List<String> getLocalIps() {
+ List<String> 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<NetworkInterface> interfaces = NetworkInterface.getNetworkInterfaces();
+ if (interfaces != null) {
+ while (interfaces.hasMoreElements()) {
+ final NetworkInterface nic = interfaces.nextElement();
+ final Enumeration<InetAddress> 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<String> 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 0000000..7b0ae46
--- /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 0000000..73129f5
--- /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 0000000..35daf43
--- /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 1e7204c..9a55dda 100644
--- a/pom.xml
+++ b/pom.xml
@@ -954,6 +954,13 @@
<version>3.0.0</version>
<scope>test</scope>
</dependency>
+ <!-- Testing LDAP -->
+ <dependency>
+ <groupId>org.zapodot</groupId>
+ <artifactId>embedded-ldap-junit</artifactId>
+ <version>0.8.1</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</dependencyManagement>
<build>