233 lines
11 KiB
Diff
233 lines
11 KiB
Diff
From fe3d988db456640c96412e6c2a27ea2d40e51615 Mon Sep 17 00:00:00 2001
|
|
From: Ralph Goers <rgoers@apache.org>
|
|
Date: Tue, 30 Nov 2021 17:20:24 -0700
|
|
Subject: [PATCH] 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 f2190ca91c..35f5b10569 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 @@
|
|
@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 e57d0bebba..30e65ad24f 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 8a0e68dc66..613a0551da 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 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<String> permanentAllowedHosts = new ArrayList<>();
|
|
- private static final List<String> 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<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(), Number.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;
|
|
-
|
|
- 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<String> allowedProtocols;
|
|
|
|
private final DirContext context;
|
|
|
|
private JndiManager(final String name, final DirContext context, final List<String> allowedHosts,
|
|
- final List<String> allowedClasses) {
|
|
+ final List<String> allowedClasses, final List<String> 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> 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 @@ protected boolean releaseSub(final long timeout, final TimeUnit timeUnit) {
|
|
public JndiManager createManager(final String name, final Properties data) {
|
|
String hosts = data != null ? data.getProperty(ALLOWED_HOSTS) : null;
|
|
String classes = data != null ? data.getProperty(ALLOWED_CLASSES) : null;
|
|
+ String protocols = data != null ? data.getProperty(ALLOWED_PROTOCOLS) : null;
|
|
List<String> allowedHosts = new ArrayList<>();
|
|
List<String> allowedClasses = new ArrayList<>();
|
|
+ List<String> allowedProtocols = new ArrayList<>();
|
|
addAll(hosts, allowedHosts, permanentAllowedHosts, ALLOWED_HOSTS, data);
|
|
addAll(classes, allowedClasses, permanentAllowedClasses, ALLOWED_CLASSES, data);
|
|
+ addAll(protocols, allowedProtocols, permanentAllowedProtocols, ALLOWED_PROTOCOLS, data);
|
|
try {
|
|
- return new JndiManager(name, new InitialDirContext(data), allowedHosts, allowedClasses);
|
|
+ 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 73129f5a91..a26d927da4 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 @@
|
|
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 @@
|
|
@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) {
|