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

fix(security): set platform restriction on log4j #912

Merged
merged 1 commit into from
Dec 14, 2021

Conversation

jasonmcintosh
Copy link
Member

Bumps log4j platform restriction. Though the core lib is NOT included in dependencies, better safe than sorry...

Bumps log4j platform restriction.  Though the core lib is NOT included in dependencies, better safe than sorry...
// Log4shell safeguard. Per analysis, log4j-core is not included in dependencies, but this would prevent transitive inclusion of it by extension
// platforms. Doing 2.16.0 which completely removes message lookups AND sets jndi to disabled by default

api(platform("org.apache.logging.log4j:log4j-bom:2.16.0"))
Copy link
Contributor

@j-sandy j-sandy Dec 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior is also disabled by default in 2.15.0 as mentioned here . I hope we should have a valid reason to bump up for 2.16.0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. It's good to be on latest releases as much as possible or makes sense.
  2. 2.16 makes jdni lookups disabled by default entirely vs. just fixing the parsing in log messages. As such it's MUCH more secure and recommended.

Either of the above reasons would be justification for bumping to 2.16.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was another CVE published today (a medium one) for 2.15.0, so using 2.16.0 makes sense.

Copy link
Contributor

@dbyron-sf dbyron-sf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @jasonmcintosh . Planning to tag kork after this goes in.

@dbyron-sf dbyron-sf added the ready to merge Approved and ready for merge label Dec 14, 2021
@mergify mergify bot merged commit 554c5ed into spinnaker:master Dec 14, 2021
@mergify mergify bot added the auto merged label Dec 14, 2021
ylebedeva pushed a commit to ylebedeva/kork that referenced this pull request May 3, 2022
Bumps log4j platform restriction.  Though the core lib is NOT included in dependencies, better safe than sorry...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants