You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2022/03/11 08:22:39 UTC
[GitHub] [zookeeper] eolivelli opened a new pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
eolivelli opened a new pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802
- Drop Apache Log4j1
- Replace with https://reload4j.qos.ch/, that is 100% compatible (same package names, it is actually a fork)
- Remove logging implementation dependency from zookeeper server module (that is also used by clients)
Client applications that upgrade to 3.6.4 or 3.7.1 won't have surprises regarding Maven exclusions, they were used to exclude log4j1, this is no more needed, but even if they forget to drop that exclusion it will be harmless, and they will never find reload4j as transitive dependency
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] li4wang commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
li4wang commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1049279741
> The only issues I saw was the removal of the JMX MBean stuff, but ZooKeeper should already handle that fine.
Yeah, I noticed JMX MBean stuff was removed from reload4j-1.2.18.5 and 1.2.19, but I don't know if the removal was intended and permanent. I posted a message to the reload4j distribution list but haven't got response yet.
I replaced log4j1.2.17 with reload4j-1.2.18.3 and slf4j-reload4j-1.7.35 for the legacy Zookeeper 3.4 release and it works great.
How does Zookeeper handle missing JMX package? It looks like it still tries to instantiate the `org.apache.log4j.jmx.HierarchyDynamicMBean ` at the runtime.
https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/jmx/ManagedUtil.java#L68
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1079847903
I will send a new patch for 3.6 thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ctubbsii commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1066159594
> Thanks for looking into this @eolivelli
>
> For the Vulnerability issue reported from slf4j-log4j12, how about updating slf4j version from `1.7.30` to `1.7.35`? Another thing we can try is replacing slf4j-log4j12 with slf4j-reload4j for the binding after upgrading slf4j to 1.7.35.
Both of these should be done. slf4j-log4j12 after 1.7.35 is just a simple redirect to slf4j-reload4j anyway. On the class path, it makes no difference, because the jar it resolves to is the same. It's just a name change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#discussion_r793604523
##########
File path: zookeeper-contrib/zookeeper-contrib-loggraph/pom.xml
##########
@@ -63,14 +63,9 @@
</exclusions>
</dependency>
<dependency>
- <groupId>log4j</groupId>
- <artifactId>log4j</artifactId>
- <exclusions>
- <exclusion>
- <groupId>*</groupId>
- <artifactId>*</artifactId>
- </exclusion>
- </exclusions>
+ <groupId>ch.qos.reload4j</groupId>
+ <artifactId>reload4j</artifactId>
+ </dependency>
Review comment:
thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad closed pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
arshadmohammad closed pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ctubbsii commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1027018960
> > Yes. In general, ZK should exclude any logging runtimes coming from dependencies using , and it should mark any logging runtimes it adds as true. This would be very helpful for downstream Maven builds that depend on ZK.
>
> 100% agreed. The real problem is that we don't have a clean "client package", I started to work on it much time ago, but I never finished.
I'm not sure what you mean by `clean "client package"` or how that applies here.
>
> But this is actually out of the scope of the PR. isn't it ?
I'm not so sure. In order to ensure slf4j binds to reload4j and not to log4j1, you probably need to make sure any log4j1 jars are excluded from the classpath. That means doing the work to set up any exclusions from dependencies. That can be made easier by checking the dependency hierarchy with `mvn dependency:tree`. Declaring reload4j as optional might be considered out of scope, but it's also trivial to do, so I'd include it anyway, so long as it doesn't prevent it from being bundled in the distribution assembly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad commented on a change in pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on a change in pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#discussion_r809303049
##########
File path: zookeeper-server/pom.xml
##########
@@ -77,6 +77,7 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
+ <scope>test</scope>
Review comment:
After changing slf4j-log4j12 scope to test, logging is not working. I can see only following information in log file
`SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
SLF4J: Failed to load class "org.slf4j.impl.StaticMDCBinder".
SLF4J: Defaulting to no-operation MDCAdapter implementation.
SLF4J: See http://www.slf4j.org/codes.html#no_static_mdc_binder for further details.
`
So some how I think we should ensure presence of slf4j implementation lib in tar ball.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1048806212
> Note that reload4j is not 100% compatible with Log4j 1.2.
Thanks for the reminder.
Would you suggest to use log4j2 and activate the compatibility bridge instead of using that fork?
It seemed to me that this move was the less risky.
We are doing very limited use of the capabilities of log4j1 as we are using sl4fj API.
The main problem for us is to to give surprises to users who changed the log configuration file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ctubbsii commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1026971460
> @eolivelli looks good to me, but I have thought something else. What about the dependencies used in ZooKeeper.
>
> e.g. ZooKeeper uses jUnit or commons-cli or anything else and they depend on log4j1, shouldn't we exclude log4j1 when we use them?
Yes. In general, ZK should exclude any logging runtimes coming from dependencies using `<exclusions>`, and it should mark any logging runtimes it adds as `<optional>true</optional>`. This would be very helpful for downstream Maven builds that depend on ZK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1075460926
I have restarted CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1079741855
Merged to branch-3.7. But this PR changes causes conflict in branch-3.6
@eolivelli please raise PR for branch-3.6. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1079238139
I will merge this PR tomorrow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] li4wang commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
li4wang commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1065398600
Thanks for looking into this @eolivelli
For the Vulnerability issue reported from slf4j-log4j12, how about updating slf4j version from `1.7.30` to `1.7.35`? Another thing we can try is replacing slf4j-log4j12 with slf4j-reload4j for the binding after upgrading slf4j to 1.7.35.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] li4wang commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
li4wang commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1080880951
Thanks @eolivelli and others for getting this in. Really appreciated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1026578503
@tamaashu PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1024330714
> Do you think we should discuss this on the dev list too
sure thing.
I created this PR in order to kickstart CI and do testing.
for old branches we cannot change the logging framework, and Reload4j is 100% compatible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] li4wang commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
li4wang commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1075442960
Any updates on this @eolivelli ? We are in the process of deploying 3.7 to production. It would be great if this issue can be addressed. Please let me know if anything I can help with. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1026974069
> Yes. In general, ZK should exclude any logging runtimes coming from dependencies using <exclusions>, and it should mark any logging runtimes it adds as <optional>true</optional>. This would be very helpful for downstream Maven builds that depend on ZK.
100% agreed.
The real problem is that we don't have a clean "client package", I started to work on it much time ago, but I never finished.
But this is actually out of the scope of the PR. isn't it ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ctubbsii commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1023638912
@anmolnar wrote:
> @eolivelli Waiting for the syntax fix to make CI happy again. This patch could render the logback migration pointless to be honest.
If there was interest in staying on log4j1 longer, it does obviate the *need* to move to logback. However, you had some other good changes in that migration that improved using logs for tests, and the changes you did to move to logback increase the confidence that users can swap out the implementation at runtime with any other slf4j runtime implementation jar. I think migrating to reload4j is a temporary hack. Long-term, what you did to get onto logback will be more useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1023211921
I will also have to update the LICENSE files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1076112807
I have updated SLF4J to 1.7.35 as suggested by @ctubbsii
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1078880037
@arshadmohammad I have updated reload4j
PTAL
feel free to merge if you are happy with this patch and CI passes
thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad edited a comment on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
arshadmohammad edited a comment on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1079741855
Merged to branch-3.7. But this PR change causes conflict in branch-3.6
@eolivelli please raise PR for branch-3.6. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1042106392
It is a flaky test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] li4wang commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
li4wang commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1049242463
> Note that reload4j is not 100% compatible with Log4j 1.2.
According to the Reload4j website, reload4j is a fork of log4j1.2 and supposed to be a drop-in replacement for log4j.1.2.17. Any info on what and why it is not 100% compatible with Log4j 1.2?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] anmolnar commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
anmolnar commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1023203679
@eolivelli Waiting for the syntax fix to make CI happy again.
This patch could render the logback migration pointless to be honest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] sonatype-lift[bot] commented on a change in pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#discussion_r824757853
##########
File path: zookeeper-assembly/pom.xml
##########
@@ -64,6 +64,14 @@
<artifactId>zookeeper</artifactId>
<version>${project.version}</version>
</dependency>
+ <dependency>
Review comment:
*Critical OSS Vulnerability:*
### pkg:maven/org.slf4j/slf4j-log4j12@1.7.30
2 Critical, 0 Severe, 1 Moderate, 0 Unknown vulnerabilities have been found across 1 dependencies
<details>
<summary><b>Components</b></summary><br/>
<ul>
<details>
<summary><b>pkg:maven/log4j/log4j@1.2.17</b></summary>
<ul>
<details>
<summary><b>CRITICAL Vulnerabilities (2)</b></summary><br/>
<ul>
<details>
<summary>CVE-2019-17571</summary>
> #### [CVE-2019-17571] Included in Log4j 1.2 is a SocketServer class that is vulnerable to deserializat...
> Included in Log4j 1.2 is a SocketServer class that is vulnerable to deserialization of untrusted data which can be exploited to remotely execute arbitrary code when combined with a deserialization gadget when listening to untrusted network traffic for log data. This affects Log4j versions up to 1.2 up to 1.2.17.
>
> **CVSS Score:** 9.8
>
> **CVSS Vector:** CVSS:3.0/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H
</details>
<details>
<summary>CVE-2021-4104</summary>
> #### [CVE-2021-4104] JMSAppender in Log4j 1.2 is vulnerable to deserialization of untrusted data when...
> JMSAppender in Log4j 1.2 is vulnerable to deserialization of untrusted data when the attacker has write access to the Log4j configuration. The attacker can provide TopicBindingName and TopicConnectionFactoryBindingName configurations causing JMSAppender to perform JNDI requests that result in remote code execution in a similar fashion to CVE-2021-44228. Note this issue only affects Log4j 1.2 when specifically configured to use JMSAppender, which is not the default. Apache Log4j 1.2 reached end of life in August 2015. Users should upgrade to Log4j 2 as it addresses numerous other issues from the previous versions.
===================================================
The following information is provided by Sonatype Nexus Intelligence. Nexus Intelligence is the only security research service that performs "secondary expansion" to determine if newly discovered vulnerabilities are also present in other components.
Learn more about Nexus Intelligence -- https://www.sonatype.com/products/intelligence
===================================================
Explanation
---------------------------------------------------
The `log4j:log4j` package is vulnerable to Deserialization of Untrusted Data. The `lookup()` and `activateOptions()` methods in the `JMSAppender` class allow `JNDI` lookup requests to be made when the `TopicBindingName` and `TopicConnectionFactoryBindingName` specify a trusted host. Lookups made to this host may be used by attackers to request a serialized malicious Java Object that can be deserialized and executed, leading to Remote Code Execution (RCE).
Note that this vulnerability is different from CVE-2021-44228 and requires the attacker to be in control of the third party host that is specified in the configuration, or write access to the Log4j configuration file in order to specify a malicious lookup host directly. This vulnerability also only affects the 1.x.x component of `Log4j` released under the `log4j:log4j` group and artifact IDs.
*Advisory Deviation Notice:* The Sonatype security research team discovered that the root cause of the vulnerability is in all versions of log4j:log4j, not just in the 1.2.x branch as the advisory states.
Detection
---------------------------------------------------
The application is vulnerable by using this component under the following circumstances:
- The configuration file specifies an allowed third-party `JNDI` lookup host for the `JMSAppender`
- the `javax.jms.*` API is included in the application's `CLASSPATH`
Reference: https://bugzilla.redhat.com/show_bug.cgi?id=2031667#c28
Recommendation
---------------------------------------------------
The 1.x.x component has reach `End of Life`, and users should upgrade to a non-vulnerable version of `org.apache.logging.log4j:log4j-core` as this component includes other security vulnerabilities that are not fixed.
References:
- https://github.com/apache/logging-log4j2/pull/608#issuecomment-990494126
- https://logging.apache.org/log4j/1.2/
>
> **CVSS Score:** 8.1
>
> **CVSS Vector:** CVSS:3.0/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:H/A:H
</details>
</ul>
</details>
<details>
<summary><b>MODERATE Vulnerabilities (1)</b></summary><br/>
<ul>
> #### [CVE-2020-9488] Improper validation of certificate with host mismatch in Apache Log4j SMTP appen...
> Improper validation of certificate with host mismatch in Apache Log4j SMTP appender. This could allow an SMTPS connection to be intercepted by a man-in-the-middle attack which could leak any log messages sent through that appender.
>
> **CVSS Score:** 3.7
>
> **CVSS Vector:** CVSS:3.0/AV:N/AC:H/PR:N/UI:N/S:U/C:L/I:N/A:N
</ul>
</details>
</ul>
</details>
</ul>
</details>
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1064881425
I don't know why GH does not allow me to restart CI, trying to close and reopen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ctubbsii commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1027142104
> I have excluded the slf4j binding from zookeeper-server, setting it to "test scope". People that use zookeeper-server as "client" library should not import that binding and reloadj4 into their dependency tree.
>
> but we want it in the binary tarball.
The distribution looks like it's being put together using maven-assembly-plugin using `dependencySets`. Marking it as a test dependency means it might not show up as a transitive dependency for that. So, you can fix that by adding it directly as a dependency to the `zookeeper-assembly/pom.xml`. Here is where you can also add exclusions so logj4 1.2 jars don't make it into the assembly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] tamaashu commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
tamaashu commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1026920900
@eolivelli looks good to me, but I have thought something else. What about the dependencies used in ZooKeeper.
e.g. ZooKeeper uses jUnit or commons-cli or anything else and they depend on log4j1, shouldn't we exclude log4j1 when we use them?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] c3ivodujmovic commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
c3ivodujmovic commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1042066780
Does anyone understand why the make of zookeeper client c is failing? This build code has not changed in a while, so is it could be a real issue? https://github.com/apache/zookeeper/blob/48191b63dfaa4d6d71572608a31338e15f02f0fa/zookeeper-client/zookeeper-client-c/pom.xml#L175
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1027067804
I have excluded the slf4j binding from zookeeper-server, setting it to "test scope".
People that use zookeeper-server as "client" library should not import that binding and reloadj4 into their dependency tree.
but we want it in the binary tarball.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ctubbsii commented on a change in pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#discussion_r792951102
##########
File path: zookeeper-contrib/zookeeper-contrib-loggraph/pom.xml
##########
@@ -63,14 +63,9 @@
</exclusions>
</dependency>
<dependency>
- <groupId>log4j</groupId>
- <artifactId>log4j</artifactId>
- <exclusions>
- <exclusion>
- <groupId>*</groupId>
- <artifactId>*</artifactId>
- </exclusion>
- </exclusions>
+ <groupId>ch.qos.reload4j</groupId>
+ <artifactId>reload4j</artifactId>
+ </dependency>
Review comment:
Extra `</dependency>` here
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] li4wang commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
li4wang commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1064727777
Would it be possible to get this PR merged soon? Please let me know if anything I can help with.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1065140139
I have rebased onto branch-3.7 after cherry-picking https://issues.apache.org/jira/browse/ZOOKEEPER-4479 (to fix C client test flakyness)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] arshadmohammad commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
arshadmohammad commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1077748610
Changes look good to me. Functionality works fine.
As this work started long back and meanwhile reload4j team have come up with new releases. I think it is better to take latest release [1.2.19](https://github.com/qos-ch/reload4j/releases/tag/v_1.2.19)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ctubbsii commented on a change in pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#discussion_r796861081
##########
File path: zookeeper-server/pom.xml
##########
@@ -77,6 +77,7 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-log4j12</artifactId>
+ <scope>test</scope>
Review comment:
slf4j-log4j12-1.7.35 maven coordinates automatically redirect to slf4j-reload4j-1.7.35
So, you could just use the slf4j-reload4j binding directly, if you update the slf4j version to the latest, 1.7.35 (which I would recommend). If you do this, I see this would be done in the root POM, plus the server, plus 3 of the contrib POMs.
See https://search.maven.org/artifact/org.slf4j/slf4j-log4j12/1.7.35/pom
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] tamaashu commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
tamaashu commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1026035628
@eolivelli what happens with the log4j1 jars which come to ZooKeeper transitively? Should we not exclude them everywhere?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1026079175
> @eolivelli what happens with the log4j1 jars which come to ZooKeeper transitively? Should we not exclude them everywhere?
The new version will import reload4j.
Probably we can make this optional, and include it only in the binaries package.
So people won't have to care about excluding reload4j when upgrading.
Good point!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] sudeepknair commented on a change in pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
sudeepknair commented on a change in pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#discussion_r793805739
##########
File path: zookeeper-contrib/zookeeper-contrib-fatjar/pom.xml
##########
@@ -91,9 +91,9 @@
<artifactId>snappy-java</artifactId>
</dependency>
<dependency>
- <groupId>log4j</groupId>
- <artifactId>log4j</artifactId>
- </dependency>
+ <groupId>ch.qos.reload4j</groupId>
+ <artifactId>reload4j</artifactId>
+ </dependency>
Review comment:
nit* proper alignment on closing `</dependency>` is missing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] ctubbsii commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1049263495
> > Note that reload4j is not 100% compatible with Log4j 1.2.
>
> According to the Reload4j website, reload4j is a fork of log4j1.2 and supposed to be a drop-in replacement for log4j.1.2.17. Any info on what and why it is not 100% compatible with Log4j 1.2?
The only issues I saw was the removal of the JMX MBean stuff, but ZooKeeper should already handle that fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli closed pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli closed pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] garydgregory commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1048797467
Note that reload4j is not 100% compatible with Log4j 1.2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] garydgregory commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1048976689
I understand your POV WRT minimum changes. You have a whole team of dedicated Apache developers ready to help :-) we have made a lot of improvements to the 1.2 bridge for the upcoming 2.17.2 release, which hopefully will get a release within a week.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] li4wang edited a comment on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
li4wang edited a comment on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1049279741
> The only issues I saw was the removal of the JMX MBean stuff, but ZooKeeper should already handle that fine.
Yeah, I noticed JMX MBean stuff was removed from reload4j-1.2.18.5 and 1.2.19, but I don't know if the removal was intended and permanent. I posted a message to the reload4j distribution list but haven't got response yet.
I replaced log4j1.2.17 with reload4j-1.2.18.3 and slf4j-reload4j-1.7.35 for the legacy Zookeeper 3.4 release and it works great.
Yes, Zookeeper still tries to instantiate the `org.apache.log4j.jmx.HierarchyDynamicMBean ` in ManagedUtil.registerLog4jMBeans, but the caller catches the JMSException and logs it as warn.
https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/jmx/ManagedUtil.java#L68
https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServerMain.java#L102-L103
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1802: ZOOKEEPER-4455: Move to https://reload4j.qos.ch/ (remove log4j1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1802:
URL: https://github.com/apache/zookeeper/pull/1802#issuecomment-1076823058
I had to move to slf4j-reload4j otherwise I saw the same problem as @arshadmohammad
Now I see the server working correctly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org