Restrict LDAP access via JNDI #608

rgoers · 2021-11-30T05:31:06Z

Restricts access to LDAP via JNDI.

carterkozak · 2021-11-30T10:14:35Z

log4j-core/src/main/java/org/apache/logging/log4j/core/util/NetUtils.java

+    public static List<String> getLocalIps() {
+        List<String> localIps = new ArrayList<>();
+        localIps.add("localhost");
+        localIps.add("127.0.0.1");


Perhaps IPv6 as well [::1]? (brackets may or may not be necessary depending how we extract the host)

I could have but this pre-seeding really isn't necessary as they show up in the while loop anyway.

vy

Great job @rgoers! Thanks so much for such a prompt action!

I guess you're gonna incorporate the suggested allowed schemes configuration knob too.

I am also inclined to add a property (log4j2.jndiLookupEnabled?) for toggling JNDI lookups and disable it by default. What do you think?

vy · 2021-11-30T11:38:25Z

log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java

+        permanentAllowedClasses.add(Float.class.getName());
+        permanentAllowedClasses.add(Integer.class.getName());
+        permanentAllowedClasses.add(Long.class.getName());
+        permanentAllowedClasses.add(Number.class.getName());


The rest of the classes are final, though Number isn't. I would keep Number out of this list.

This comment is on the first commit. The second commit added filtering by protocol.
I haven't added a complete kill switch and wasn't planning to. Rather than doing that I think it would be better to figure out a generic way to "hide" plugins.
More importantly, the class is abstract so can never appear here. I am not inspecting sub-classes, only the actual class being instantiated. I will remove Number.

vy · 2021-11-30T11:38:49Z

log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java

+    private static final List<String> permanentAllowedHosts = new ArrayList<>();
+    private static final List<String> permanentAllowedClasses = new ArrayList<>();


Mind upper-casing these two statics, please?

I could go either way on casing here since the included values aren't constant (e.g. NetUtils.getLocalIps() is dynamic).

I agree with @carterkozak that lower-case feels more appropriate since these are not immutable lists.

vy · 2021-11-30T11:46:25Z

log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiExploit.java

+import static org.junit.jupiter.api.Assertions.fail;
+
+/**
+ * Test LDAP object


Suggested change

* Test LDAP object

* Malicious LDAP object that shouldn't be deserialized.

*

* @see JndiLdapLookupTest

vy · 2021-11-30T11:50:57Z

log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java

+import static org.junit.Assert.fail;
+
+/**
+ * JndiLookupTest


Suggested change

* JndiLookupTest

* LDAP-specialized tests for {@link JndiLookup}.

Rats. You just reminded me I need to rename this test as it is no longer specific to LDAP.

yes need to write a more generic test

log4j-core/src/test/resources/java-import.ldif

vy · 2021-11-30T11:53:32Z

log4j-core/src/test/java/org/apache/logging/log4j/core/lookup/JndiLdapLookupTest.java

+/**
+ * JndiLookupTest
+ */
+public class JndiLdapLookupTest {


@rgoers, I cannot see the test where the exploit indeed works when the prevention mechanism is not in place. Am I missing something?

I tested that before I wrote any code. Once the code was added it isn't possible for it to happen any more as we no longer support Referenceable objects and there is no way to re-enable them. They are simply too dangerous.

log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java

carterkozak · 2021-12-01T11:10:42Z

log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java

+                    LOGGER.warn("Attempt to access ldap server not in allowed list");
+                    return null;
+                }
+                Attributes attributes = this.context.getAttributes(name);


Are we confident the attributes resolved here are equivalent to the results of context.lookup? I imagine the attributes could change over time.

I assume you mean the attributes associated with the returned item. All the attributes returned from LDAP for Java objects are specified in https://datatracker.ietf.org/doc/html/rfc2713.

I want to make sure that context.getAttributes(name) verifies against the same data (cached without any sort of refresh or network call) as the subsequent call to context.lookup(name), otherwise the verification isn't very helpful.

Well, yeah. That is how DirContext is documented to work. https://docs.oracle.com/javase/8/docs/api/javax/naming/directory/DirContext.html#getAttributes-java.lang.String-

Perfect, thank you for the explanation, I'm not very familiar with these components :-)

…servers and classes that can be accessed via LDAP.

wcc526 · 2021-12-09T11:22:02Z

Is it a security vulnerability?

Glavo · 2021-12-09T17:31:28Z

Is it a security vulnerability?

I think it is.

It is very surprising that this critical security issue does not seem to have received due attention. It was reported to Apache half a month ago, but it was not fixed until five days ago. Even today, it has not released a new stable version to solve it.

garydgregory · 2021-12-09T17:37:45Z

Is it a security vulnerability?

I think it is.

It is very surprising that this critical security issue does not seem to have received due attention. It was reported to Apache half a month ago, but it was not fixed until five days ago. Even today, it has not released a new stable version to solve it.

Oh so glad you show such appreciation for the work of volunteers...

Glavo · 2021-12-09T18:05:44Z

Is it a security vulnerability?

I think it is.
It is very surprising that this critical security issue does not seem to have received due attention. It was reported to Apache half a month ago, but it was not fixed until five days ago. Even today, it has not released a new stable version to solve it.

Oh so glad you show such appreciation for the work of volunteers...

@garydgregory

I wonder when log4j 2.15 will be officially released? It's hard to imagine that the craziest vulnerability this year has not been solved in the release half a month after it was reported.

Its impact is unimaginable. Countless services using log4j2 are exposed to the risk of being attacked, and the way to attack them is surprisingly simple. Even now I dare not open my minecraft server, because any member can attack it if they want - he/she can easily control my server by sending a text through the chat bar.

Is there anyone dealing with this matter urgently? It's really incomprehensible that I didn't see Apache give any emergency warning under such a serious problem.

GalvinGao · 2021-12-09T18:20:41Z

Is it a security vulnerability?

I think it is.
It is very surprising that this critical security issue does not seem to have received due attention. It was reported to Apache half a month ago, but it was not fixed until five days ago. Even today, it has not released a new stable version to solve it.

Oh so glad you show such appreciation for the work of volunteers...

@garydgregory

I wonder when log4j 2.15 will be officially released? It's hard to imagine that the craziest vulnerability this year has not been solved in the release half a month after it was reported.

Its impact is unimaginable. Countless services using log4j2 are exposed to the risk of being attacked, and the way to attack them is surprisingly simple. Even now I dare not open my minecraft server, because any member can attack it if they want - he/she can easily control my server by sending a text through the chat bar.

Is there anyone dealing with this matter urgently? It's really incomprehensible that I didn't see Apache give any emergency warning under such a serious problem.

+1, and we are in desperate need of a CVE and security advisory to be announced asap. This could affect hundreds of thousands, if not millions, of services actively running on the internet.

We of course appreciate the efforts from contributors, but overall this is a major security issue that needs a new version release and a security advisory.

garydgregory · 2021-12-09T20:17:40Z

Your patience will soon be rewarded...

garydgregory · 2021-12-09T20:22:54Z

Also, if this matters to you so much, why not show it with a donation to the Apache Software Foundation https://www.apache.org/foundation/contributing.html or this project's main contributor https://github.com/sponsors/rgoers ?

remkop · 2021-12-09T21:18:03Z

Is it a security vulnerability?

I think it is.
It is very surprising that this critical security issue does not seem to have received due attention. It was reported to Apache half a month ago, but it was not fixed until five days ago. Even today, it has not released a new stable version to solve it.

Oh so glad you show such appreciation for the work of volunteers...

@garydgregory
I wonder when log4j 2.15 will be officially released? It's hard to imagine that the craziest vulnerability this year has not been solved in the release half a month after it was reported.
Its impact is unimaginable. Countless services using log4j2 are exposed to the risk of being attacked, and the way to attack them is surprisingly simple. Even now I dare not open my minecraft server, because any member can attack it if they want - he/she can easily control my server by sending a text through the chat bar.
Is there anyone dealing with this matter urgently? It's really incomprehensible that I didn't see Apache give any emergency warning under such a serious problem.

+1, and we are in desperate need of a CVE and security advisory to be announced asap. This could affect hundreds of thousands, if not millions, of services actively running on the internet.

We of course appreciate the efforts from contributors, but overall this is a major security issue that needs a new version release and a security advisory.

My understanding is that the procedure is to hold off on announcing the vulnerability until a patch is available. (See https://www.apache.org/security/).

For background:

The team is taking it seriously. As Gary said, we are all volunteers working on this in our spare time. We are also in different time zones so communication is not instantaneous. If you think things can be improved, that's great! We need more people like you and I would encourage you to get involved!

We are in the process of getting a release out with the fix. During review, some security experts found a new vulnerability in our fix (a way to bypass the fix). This has been addressed and we are now in the process of reviewing the updated 2nd release candidate.

Usually (as per ASF rules) the team should wait 72 hours after creating a release candidate before publishing the release to give the community enough time to review and cast their votes. We are building consensus to shorten that window for this particular release, given its urgency.

zhangyoufu · 2021-12-09T21:46:18Z

You can't ask everybody to upgrade to 2.15 at once. And the formatMsgNoLookups option is available to log4j ≥ 2.10 only.

Thanks to LOG4J2-703, I think it's quite safe to remove org/apache/logging/log4j/core/lookup/JndiLookup.class from log4j-core-*.jar as a workaround. Just zip -q -d log4j-core-*.jar org/apache/logging/log4j/core/lookup/JndiLookup.class to disable ${jndi:...} functionality completely.

I posted all log4j-core jar with JndiLookup.class removed at https://github.com/zhangyoufu/log4j2-without-jndi for reference. Simple local test looks promising.

remkop · 2021-12-10T01:08:03Z

Update: the vote for log4j-2.15.0 passed and the release is in progress.

I can see the log4j web site reflecting the log4j 2.15.0 release, but I cannot see the 2.15.0 artifacts on Maven Central yet at this moment. It may take a few hours before mirror servers are synchronized and the artifacts become available for you.

An announcement email for the release will be sent out soon (within 24 hours - we usually wait some time for the mirror servers to catch up).

Thank you @zhangyoufu for the suggested workaround for older versions of log4j to remove the JndiLookup.class class! The team likes your idea and we will include the workaround you suggested in the release notes and announcement email. Many thanks!

moonming · 2021-12-10T01:18:45Z

@remkop thanks for your great work 👍
I come from the Apache APISIX community, and we can intercept this security vulnerability at the API gateway level to provide some help. I wonder if there are such regular expression rules? thanks

yuezk · 2021-12-10T01:18:56Z

Hi @rgoers, is log4j 1.x vulnerable?

remkop · 2021-12-10T01:26:00Z

Hi @rgoers, is log4j 1.x vulnerable?

Hi @yuezk, as far as I can tell, log4j 1.x does not support lookups. ~~I also could not find any other reference to JNDI in the log4j 1.x source code. So, no guarantees but it looks like 1.x is not impacted by this vulnerability.~~ CORRECTION: log4j 1.x contains a JMS Appender which can use JNDI. So I would say that, yes, log4j 1.x is also impacted by this vulnerability (Thank you @garydgregory for pointing this out).

Update (2021-12-11 09:09 JST): according to this analysis by @ceki (the author of log4j 1.x), Log4j 1.x is not impacted, since it does not have lookups, and the JMS Appender only loads Strings from the remote server, not serialized objects.

Update (2021-12-12 10:09 JST): according to this analysis by @TopStreamsNet, strictly speaking, applications using Log4j 1.x may be impacted if their configuration uses JNDI. However, the risk is much lower.

Note that log4j 1.x is End of Life and has other security vulnerabilities that will not be fixed. ~~So we do not recommend using log4j 1.x as a way to avoid this security vulnerability.~~ The recommendation is to upgrade to 2.15.0.

garydgregory · 2021-12-10T01:30:47Z

We need to look at the log4j 1 JMS Appender which I thought had at least programmatic support for JNDI. Gary

…

On Thu, Dec 9, 2021, 20:26 Remko Popma ***@***.***> wrote: Hi @rgoers <https://github.com/rgoers>, is log4j 1.x vulnerable? Hi @yuezk <https://github.com/yuezk>, as far as I can tell, log4j 1.x does not support lookups. I also could not find any other reference to JNDI in the log4j 1.x source code <http://svn.apache.org/viewvc/logging/log4j/trunk/>. So, no guarantees but it looks like 1.x is not impacted by this vulnerability. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#608 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJB6NYD7R35WOHFKBO3ILLUQFJLHANCNFSM5JA3ZEGA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

garydgregory · 2021-12-10T01:34:28Z

As documented here: https://logging.apache.org/log4j/1.2/apidocs/org/apache/log4j/net/JMSAppender.html Gary

…

On Thu, Dec 9, 2021, 20:30 Gary Gregory ***@***.***> wrote: We need to look at the log4j 1 JMS Appender which I thought had at least programmatic support for JNDI. Gary On Thu, Dec 9, 2021, 20:26 Remko Popma ***@***.***> wrote: > Hi @rgoers <https://github.com/rgoers>, is log4j 1.x vulnerable? > > Hi @yuezk <https://github.com/yuezk>, as far as I can tell, log4j 1.x > does not support lookups. I also could not find any other reference to JNDI > in the log4j 1.x source code > <http://svn.apache.org/viewvc/logging/log4j/trunk/>. So, no guarantees > but it looks like 1.x is not impacted by this vulnerability. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#608 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAJB6NYD7R35WOHFKBO3ILLUQFJLHANCNFSM5JA3ZEGA> > . > Triage notifications on the go with GitHub Mobile for iOS > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> > or Android > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. > >

CVE-2021-44228: apache/logging-log4j2#608

alphawoodexec

log4j-core/pom.xml

Restrict LDAP access via JNDI

755e2c9

carterkozak reviewed Nov 30, 2021

View reviewed changes

vy requested changes Nov 30, 2021

View reviewed changes

carterkozak reviewed Nov 30, 2021

View reviewed changes

log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java Outdated Show resolved Hide resolved

rgoers added 2 commits November 30, 2021 17:20

Disable most JNDI protocols

fe3d988

Rename test. Various minor fixes

5f81dd2

carterkozak reviewed Dec 1, 2021

View reviewed changes

carterkozak approved these changes Dec 1, 2021

View reviewed changes

LOG4J2-3201 - Limit the protocols JNDI can use by default. Limit the …

7fe72d6

…servers and classes that can be accessed via LDAP.

rgoers merged commit c77b3cb into release-2.x Dec 5, 2021

hh-hunter mentioned this pull request Dec 9, 2021

PRP: Request CVE-2021-44228 Apache Log4j2 <=2.14.1 JNDI RCE google/tsunami-security-scanner-plugins#219

Closed

hh-hunter mentioned this pull request Dec 9, 2021

PRP: Request Apereo CAS RCE google/tsunami-security-scanner-plugins#220

Closed

Glavo mentioned this pull request Dec 9, 2021

默认情况下禁止 Minecraft 日志记录中使用 Message Pattern Lookup HMCL-dev/HMCL#1209

Merged

ThiemoBoven mentioned this pull request Dec 9, 2021

log4j version update discord-jda/JDA#1943

Closed

3 tasks

berezovskyi mentioned this pull request Dec 10, 2021

Get rid of log4j in oslc4j-registry eclipse-lyo/lyo#219

Closed

cx-liza mentioned this pull request May 16, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cx-liza/astlab2#7

Closed

hirendgithub mentioned this pull request May 16, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 hirendgithub/astlab2#16

Closed

darwantep mentioned this pull request May 16, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 darwantep/AST_Training#16

Open

danielgerb mentioned this pull request May 18, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 danielgerb/astlab2#18

Closed

taogeaidd approved these changes May 31, 2022

View reviewed changes

jbrotsos mentioned this pull request Jun 16, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 CxOneOrg/msft#35

Open

jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jul 17, 2022

sbt: 1.5.5 -> 1.5.6

fcb24ae

CVE-2021-44228: apache/logging-log4j2#608

jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jul 18, 2022

sbt: 1.5.5 -> 1.5.6

c339909

CVE-2021-44228: apache/logging-log4j2#608

alphawoodexec reviewed Jul 24, 2022

View reviewed changes

amitshalemCx mentioned this pull request Aug 10, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 amitshalemCx/ast-advanced-lab#28

Closed

This was referenced Sep 20, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cxronen/BookStore_VSCode#57

Open

CVE-2021-44228 @ Maven-org.apache.logging.log4j:log4j-core-2.11.2 cxronen/BookStore_VSCode#112

Closed

skyler-kim mentioned this pull request Oct 11, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 skyler-kim/astlab2#43

Open

cx-tatianab mentioned this pull request Nov 7, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cx-tatianab/astlab_tatiana#16

Open

veredshahar mentioned this pull request Feb 21, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 veredshahar/astlab2#14

Closed

margaritalm mentioned this pull request Dec 25, 2022

CVE-2021-44228 @ Maven-org.apache.logging.log4j:log4j-core-2.3 margaritalm/BookStore_Small_CLI_small#45

Open

joaoreis-cx mentioned this pull request Nov 16, 2022

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 joaoreis-cx/test1#768

Open

cx-tatianab mentioned this pull request Feb 7, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cx-tatianab/cxone-advanced-lab#41

Closed

dorohayon mentioned this pull request Feb 7, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 dorohayon/cxone-advanced-lab#14

Closed

cxronen mentioned this pull request Feb 17, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cxronen/BookStore#474

Open

Vadim-Sedletsky mentioned this pull request Feb 9, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 Vadim-Sedletsky/astlab2#15

Open

SamHeadrickCx mentioned this pull request Mar 14, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 SamHeadrickCx/easybuggy4sb#44

Open

oscardatastream mentioned this pull request Mar 14, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 oscardatastream/slackDatastream#18

Open

cxronen mentioned this pull request Mar 2, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cxronen/AST_BookStore#420

Open

This was referenced Feb 15, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cxronen/BookStore_VSCode#275

Open

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cxronen/BookStore_VSCode#514

Closed

cxronen mentioned this pull request May 25, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cxronen/BookStore_VSCode#642

Open

diogopcx mentioned this pull request Jun 27, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 diogopcx/CheckmarxDemo#61

Open

cxronen mentioned this pull request May 25, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 cxronen/BookStore_VSCode#907

Open

apcxtest mentioned this pull request Aug 17, 2023

CVE-2021-4104 @ Maven-log4j:log4j-1.2.17 apcxtest/test-repo-pub#65

Closed

		private static final List<String> permanentAllowedHosts = new ArrayList<>();
		private static final List<String> permanentAllowedClasses = new ArrayList<>();

- * Test LDAP object
+ * Malicious LDAP object that shouldn't be deserialized.
+ *
+ * @see JndiLdapLookupTest

	* JndiLookupTest
	* LDAP-specialized tests for {@link JndiLookup}.

Restrict LDAP access via JNDI #608

Restrict LDAP access via JNDI #608

Conversation

rgoers commented Nov 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcc526 commented Dec 9, 2021 • edited Loading

Glavo commented Dec 9, 2021

garydgregory commented Dec 9, 2021

Glavo commented Dec 9, 2021 • edited Loading

GalvinGao commented Dec 9, 2021 • edited Loading

garydgregory commented Dec 9, 2021

garydgregory commented Dec 9, 2021

remkop commented Dec 9, 2021

zhangyoufu commented Dec 9, 2021 • edited Loading

remkop commented Dec 10, 2021 • edited Loading

moonming commented Dec 10, 2021

yuezk commented Dec 10, 2021

remkop commented Dec 10, 2021 • edited Loading

garydgregory commented Dec 10, 2021 via email

garydgregory commented Dec 10, 2021 via email

alphawoodexec left a comment

Choose a reason for hiding this comment

wcc526 commented Dec 9, 2021 •

edited

Loading

Glavo commented Dec 9, 2021 •

edited

Loading

GalvinGao commented Dec 9, 2021 •

edited

Loading

zhangyoufu commented Dec 9, 2021 •

edited

Loading

remkop commented Dec 10, 2021 •

edited

Loading

remkop commented Dec 10, 2021 •

edited

Loading