Skip to content

Commit

Permalink
Restrict LDAP access via JNDI (#608)
Browse files Browse the repository at this point in the history
* Restrict LDAP access via JNDI

* Disable most JNDI protocols

* Rename test. Various minor fixes

* LOG4J2-3201 - Limit the protocols JNDI can use by default. Limit the servers and classes that can be accessed via LDAP.
  • Loading branch information
rgoers committed Dec 5, 2021
1 parent 2315969 commit c77b3cb
Show file tree
Hide file tree
Showing 13 changed files with 465 additions and 9 deletions.
5 changes: 5 additions & 0 deletions log4j-core/pom.xml
Expand Up @@ -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>
Expand Down
Expand Up @@ -88,6 +88,15 @@ public static class Builder<B extends Builder<B>> extends AbstractAppender.Build
@PluginBuilderAttribute
private boolean immediateFail;

@PluginBuilderAttribute
private String allowedLdapClasses;

@PluginBuilderAttribute
private String allowedLdapHosts;

@PluginBuilderAttribute
private String allowedJndiProtocols;

// Programmatic access only for now.
private JmsManager jmsManager;

Expand All @@ -100,8 +109,21 @@ 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);
}
if (allowedJndiProtocols != null) {
additionalProperties.put(JndiManager.ALLOWED_PROTOCOLS, allowedJndiProtocols);
}
}
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);
Expand Down Expand Up @@ -202,6 +224,21 @@ 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;
}

public Builder setAllowedJndiProtocols(final String allowedJndiProtocols) {
this.allowedJndiProtocols = allowedJndiProtocols;
return this;
}

/**
* Does not include the password.
*/
Expand All @@ -212,7 +249,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 + ", allowedJndiProtocols=" + allowedJndiProtocols + "]";
}

}
Expand Down
Expand Up @@ -17,31 +17,69 @@

package org.apache.logging.log4j.core.net;

import java.net.URI;
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.InitialContext;
import javax.naming.NamingEnumeration;
import javax.naming.NamingException;
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";
public static final String ALLOWED_PROTOCOLS = "allowedJndiProtocols";

private static final JndiManagerFactory FACTORY = new JndiManagerFactory();
private static final String PREFIX = "log4j2.";
private static final String LDAP = "ldap";
private static final String LDAPS = "ldaps";
private static final String JAVA = "java";
private static final List<String> permanentAllowedHosts = NetUtils.getLocalIps();
private static final List<String> permanentAllowedClasses = Arrays.asList(Boolean.class.getName(),
Byte.class.getName(), Character.class.getName(), Double.class.getName(), Float.class.getName(),
Integer.class.getName(), Long.class.getName(), Short.class.getName(), String.class.getName());
private static final List<String> 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<String> allowedHosts;
private final List<String> allowedClasses;
private final List<String> allowedProtocols;

private final Context context;
private final DirContext context;

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

/**
Expand Down Expand Up @@ -168,21 +206,91 @@ protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) {
* @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 (uri.getScheme() != null) {
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;
}
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<String, Attribute> attributeMap = new HashMap<>();
NamingEnumeration<? extends Attribute> 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 if (attributeMap.get(REFERENCE_ADDRESS) != null
|| attributeMap.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);
}

private static class JndiManagerFactory implements ManagerFactory<JndiManager, Properties> {

@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 InitialContext(data));
return new JndiManager(name, new InitialDirContext(data), allowedHosts, allowedClasses,
allowedProtocols);
} 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
Expand Down
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -79,6 +84,49 @@ public static String getLocalHostname() {
}
}

/**
* Returns all the local host names and ip addresses.
* @return The local host names and ip addresses.
*/
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.
Expand Down
@@ -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;
}
}

5 comments on commit c77b3cb

@elknot
Copy link

@elknot elknot commented on c77b3cb Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, is there any patches for the older branches of log4j such as 2.14.x?

@ascmove
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My ES use log4j-2.11.x, is it helpful using jvm.options as "-Dlog4j2.formatMsgNoLookups=true"?

@vy
Copy link
Member

@vy vy commented on c77b3cb Dec 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My ES use log4j-2.11.x, is it helpful using jvm.options as "-Dlog4j2.formatMsgNoLookups=true"?

@ascmove, yes, please add -Dlog4j2.formatMsgNoLookups=true as a system property.

@garydgregory
Copy link
Member

@garydgregory garydgregory commented on c77b3cb Dec 10, 2021 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ecki
Copy link

@ecki ecki commented on c77b3cb Dec 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should turning the lookup off be the default, why would younger trigger that from a untrusted payload (even when it is the format string)?

Please sign in to comment.