From fe3d988db456640c96412e6c2a27ea2d40e51615 Mon Sep 17 00:00:00 2001 From: Ralph Goers 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 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 @@ 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 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 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) {