Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect Log4J 1.x to Log 2.x #11264

Merged
merged 1 commit into from May 18, 2021
Merged

Redirect Log4J 1.x to Log 2.x #11264

merged 1 commit into from May 18, 2021

Conversation

Stwissel
Copy link
Contributor

Removes flag by Whitesource vulnerability scanner

Motivation:

WhiteSource vulnerability scan flags the Log4J 1.x stream as vulnerable.

Modification:

Replaced reference to log4j with log4j-1.2-api
Ran mvn test (on a Mac) successfully

Result:

Fixes #11263

Removes flag by Whitesource vulnerability scanner
@Stwissel
Copy link
Contributor Author

Alternate approach: #11265
Close one or both if inappropriate

@normanmaurer
Copy link
Member

@NiteshKant @chrisvest thoughts ?

@normanmaurer
Copy link
Member

Also /cc @trustin

@trustin
Copy link
Member

trustin commented May 18, 2021

How about this one + adding Log4J2 internal logger implementation?

@normanmaurer
Copy link
Member

@trustin we already have one ;)

@trustin
Copy link
Member

trustin commented May 18, 2021

Haha, then +1 for just accepting this one 😅

@chrisvest
Copy link
Contributor

I would think that the reason we have a log4j 1.2 integration is to support deployments that wish to use log4j 1.2 for logging, the API we use to do that is an implementation detail. I couldn't find the original CVE (oh wait, here it is), but it sounds like all versions of log4j 1.2 are vulnerable, which means the ideal solution - upgrading to a non-vulnerable version - isn't possible.

So it sounds like we are dropping log4j 1.2 support entirely (btw. log4j 1.2 was declared EOL in 2015). In that case, instead of using this bridge API, we could make Log4JLoggerFactory (which we have to keep for backwards compatibility) return the log4j2 logger instead of the 1.2 class (which is hidden so we can remove it).

@normanmaurer
Copy link
Member

@chrisvest so like this #11265 ?

@Stwissel
Copy link
Contributor Author

Removing libraries that have been EOL for more than half a decade sounds reasonable. Any idea how many projects downstream would get into trouble doing that?

@chrisvest
Copy link
Contributor

@normanmaurer Yeah, added a comment on that one.

I don't know how much trouble it would cause for downstream to remove log4j 1.2. As far as I can tell, both solutions effectively do that, though.

@normanmaurer
Copy link
Member

@chrisvest i think this Solution will still work for people that had put log4j 1.2 on there classpath as there are no class changes

@chrisvest
Copy link
Contributor

@normanmaurer Because you'd have the real log4 1.2 on the class path at runtime instead of the bridge API? I suppose if people to it like that and make sure to not depend on the bridge. Ok.

@normanmaurer
Copy link
Member

@chrisvest exactly.. as the our dependency is marked as optional I think this should work.

@normanmaurer
Copy link
Member

@Stwissel can you please sign our ICLA: https://netty.io/s/icla and let me know once done ?

@Stwissel
Copy link
Contributor Author

@normanmaurer - signed

@normanmaurer normanmaurer added this to the 4.1.65.Final milestone May 18, 2021
@normanmaurer normanmaurer merged commit 7c955a1 into netty:4.1 May 18, 2021
@normanmaurer
Copy link
Member

@Stwissel thanks a lot!

normanmaurer pushed a commit that referenced this pull request Dec 15, 2021
Removes flag by Whitesource vulnerability scanner

Motivation:

WhiteSource vulnerability scan flags the Log4J 1.x stream as vulnerable.

Modification:

Replaced reference to `log4j` with `log4j-1.2-api`
Ran `mvn test` (on a Mac) successfully

Result:

Fixes #11263
raidyue pushed a commit to raidyue/netty that referenced this pull request Jul 8, 2022
Removes flag by Whitesource vulnerability scanner

Motivation:

WhiteSource vulnerability scan flags the Log4J 1.x stream as vulnerable.

Modification:

Replaced reference to `log4j` with `log4j-1.2-api`
Ran `mvn test` (on a Mac) successfully

Result:

Fixes netty#11263
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove dependency on log4J 1.x branch
4 participants