Update log4j to 2.17.1 to fix DDOS attacks #780

tsmock · 2021-12-16T21:22:57Z

Description:

See https://nvd.nist.gov/vuln/detail/CVE-2021-44228.

Potential Impact:

~~There should be no significant impact -- the logging packages were shaded.~~

We are using sl4j in code, not log4j, so all we have to do is (a) update log4j and (b) optionally update sl4j.

It does appear that scripts and documents refer to -Dlog4j.configuration, which will make log4j 2.x read log4j 1.x configuration files.

Unit Test Approach:

N/A

Test Results:

Tests did pass.

See also:

jwpgage · 2021-12-16T22:59:10Z

Hey Taylor, have you tested this change in spark? Would like to see a world build before merging.

tsmock · 2021-12-16T23:01:23Z

I'll do that as soon as I get atlas-generator/atlas-checks building again.

Both of them are looking for log4j-1.2.17 in osgeo, and it looks like osgeo removed the jar file for that release and replaced it with a -norce release. So I need to figure out which dependency is using it.

~~EDIT: I'm probably just going to do a general dependency update PR.~~
EDIT2: It turns out osgeo says it has log4j-1.2.17, but doesn't actually have it. Since it was the first repo on the list, it fails.

atiannicelli · 2021-12-20T21:26:11Z

log4j 2.16.0 creates another vulnerability (CVE-2021-45105). So we need to upgrade to 2.17.0.

tsmock · 2021-12-27T16:02:28Z

log4j 2.16.0 creates another vulnerability (CVE-2021-45105). So we need to upgrade to 2.17.0.

Since @jwpgage wants a world run before merging, I've gone ahead and done updated log4j to 2.17.0 in bdc815f, along with updating everything else that did not require source code changes.

Current CVE's in dependencies (most not from checkstyle/osmosis should be fixed in bdc815f, and will most likely require an update to atlas_checkstyle with the updated dependencies once this is merged in order to fully fix the issues):

checkstyle-8.18.jar (pkg:maven/com.puppycrawl.tools/[email protected], cpe:2.3:a:checkstyle:checkstyle:8.18:*:*:*:*:*:*:*) : CVE-2019-10782
commons-beanutils-1.9.3.jar (pkg:maven/commons-beanutils/[email protected], cpe:2.3:a:apache:commons_beanutils:1.9.3:*:*:*:*:*:*:*) : CVE-2019-10086 (checkstyle)
commons-compress-1.9.jar (pkg:maven/org.apache.commons/[email protected], cpe:2.3:a:apache:commons_compress:1.9:*:*:*:*:*:*:*) : CVE-2018-11771, CVE-2021-35515, CVE-2021-35516, CVE-2021-35517, CVE-2021-36090 (osmosis)
commons-io-2.5.jar (pkg:maven/commons-io/[email protected], cpe:2.3:a:apache:commons_io:2.5:*:*:*:*:*:*:*) : CVE-2021-29425
groovy-2.5.4.jar (pkg:maven/org.codehaus.groovy/[email protected], cpe:2.3:a:apache:groovy:2.5.4:*:*:*:*:*:*:*) : CVE-2020-17521
groovy-json-2.5.4.jar (pkg:maven/org.codehaus.groovy/[email protected], cpe:2.3:a:apache:groovy:2.5.4:*:*:*:*:*:*:*) : CVE-2020-17521
guava-27.0.1-jre.jar (pkg:maven/com.google.guava/[email protected], cpe:2.3:a:google:guava:27.0.1:*:*:*:*:*:*:*) : CVE-2020-8908 (checkstyle, may be overridden)
httpclient-4.5.1.jar (pkg:maven/org.apache.httpcomponents/[email protected], cpe:2.3:a:apache:httpclient:4.5.1:*:*:*:*:*:*:*) : CVE-2020-13956 (note: we may want to add httpclient5 so people can slowly migrate)
jackson-databind-2.10.5.jar (pkg:maven/com.fasterxml.jackson.core/[email protected], cpe:2.3:a:fasterxml:jackson-databind:2.10.5:*:*:*:*:*:*:*) : CVE-2020-25649
log4j-api-2.16.0.jar (pkg:maven/org.apache.logging.log4j/[email protected], cpe:2.3:a:apache:log4j:2.16.0:*:*:*:*:*:*:*) : CVE-2021-45105
log4j-core-2.16.0.jar (pkg:maven/org.apache.logging.log4j/[email protected], cpe:2.3:a:apache:log4j:2.16.0:*:*:*:*:*:*:*) : CVE-2021-45105
log4j-slf4j-impl-2.16.0.jar (pkg:maven/org.apache.logging.log4j/[email protected], cpe:2.3:a:apache:log4j:2.16.0:*:*:*:*:*:*:*) : CVE-2021-45105
postgresql-9.4-1201-jdbc4.jar (pkg:maven/org.postgresql/[email protected], cpe:2.3:a:postgresql:postgresql:9.4.0:*:*:*:*:*:*:*, cpe:2.3:a:postgresql:postgresql_jdbc_driver:9.4.1201:*:*:*:*:*:*:*) : CVE-2014-8161, CVE-2015-0241, CVE-2015-0242, CVE-2015-0243, CVE-2015-0244, CVE-2015-3166, CVE-2015-3167, CVE-2018-1053, CVE-2018-1058, CVE-2018-10915, CVE-2018-10925, CVE-2018-10936, CVE-2018-16850, CVE-2019-10127, CVE-2019-10128, CVE-2019-10208, CVE-2019-10210, CVE-2019-10211, CVE-2019-9193, CVE-2020-13692, CVE-2020-25694, CVE-2020-25695, CVE-2021-3393 (osmosis)
xercesImpl-2.9.1.jar (pkg:maven/xerces/[email protected]) : CVE-2009-2625 (osmosis)

Of note, checkstyle has not been updated since it means someone (probably me) will have to update the checkstyle settings (some settings got renamed, some got removed, some got added). Osmosis also has not been updated, since that will require source code changes, and it blocks protobuf.

EDIT: For Osmosis, I've made a PR at openstreetmap/osmosis#106 to update the osmosis dependencies. There are some dependencies that were not updated in that PR, since they would require more significant code rewriting (Netty 3.x to 4.x).

EDIT2: Checkstyle has been updated. Rather unfortunately, due to the way Atlas uses Checkstyle, the build will fail until a new version is released with the updated checkstyle dependency.

tsmock · 2021-12-29T22:17:36Z

For the checkstyle update, we are going to have to do one of two things:

Temporarily ignore the fact that the build is failing
Add the following (temporary) patch until a new release of Atlas is done. See 6e995c9.

index 4d5afdfb..57a64ea7 100644
--- a/build.gradle
+++ b/build.gradle
@@ -84,7 +81,7 @@ dependencies
     testImplementation packages.junit.params
     testImplementation packages.junit.vintage
 
-    checkstyle packages.atlas_checkstyle
+    checkstyle files("build/libs/${project.name}-${project.version}.jar")
     checkstyle packages.checkstyle
 
     shaded packages.log4j.api

tsmock · 2022-01-04T23:35:29Z

The run is taking a bit longer than expected. Both with and without the changes. @atiannicelli : What memory size are you using for your executors?

FTR, a large amount of time (28.7% of a 10+ minute profile run, more like 40% if you discount some native methods) is spent in https://github.com/osmlab/atlas/blob/dev/src/main/java/org/openstreetmap/atlas/utilities/filters/AtlasEntityPolygonsFilter.java#L236 . The amount of time spent with the dependency updates and without the dependency updates appeared to be about the same.

I'm going to do a full NZL run without the dependency updates on my hardware. And then I'll do one with the dependency updates.

tsmock · 2022-01-12T14:13:55Z

OK. I've done a few runs with NZL (./gradlew run, and a standalone spark cluster, I did not save timings from spark cluster)

Timings from ./gradlew run

43956s (original)
48499s (original)
47463s (updates)
49334s (updates)

I'm uncertain what happened with the two original runs (10%+ difference), however, the second non-updated run was about the same as the two runs with updates. It could be a simple as different software running on the laptop used.

atiannicelli · 2022-01-12T15:10:12Z

@jwpgage would you help to get this reviewed? @tsmock seems to have done a thorough update of many of the dependencies for Atlas that would bring it up to a level that would eliminate many CVE exposures.
Please let us know if there is any testing that we can do to help this get through the approval process.

adahn6 · 2022-01-12T22:59:18Z

Due to the significant changes to dependencies here, I want to hold off on merging this for now. Log4j 1.x (our current dependency) is unaffected by this vulnerability, though it is unsupported now and it would be good to migrate soon. I realize the benefits of upgrading seem urgent, but Spark in particular is going to force-downgrade to Log4j 1.x anyway, so this yields a lot of changes to our dependency tree for minimal overall improvements. I know https://issues.apache.org/jira/browse/SPARK-6305 and apache/spark#34895 are scheduled for release in April, so it seems wise to wait until that's released to target testing and ensuring complete compatibility.

adahn6 · 2022-01-12T23:00:11Z

@tsmock there's also a few changes here that seem unrelated to the log4j changes-- do you mind splitting those out into separate PRs? it will help make this one a bit easier to deal with if we need to rollback or debug

adahn6 · 2022-01-12T23:01:47Z

I'll try to keep reviewing this over the next few days-- haven't been ignoring it, but it's a big changeset and there's a lot of backlog from the holidays

tsmock · 2022-01-12T23:15:49Z

@tsmock there's also a few changes here that seem unrelated to the log4j changes-- do you mind splitting those out into separate PRs? it will help make this one a bit easier to deal with if we need to rollback or debug

The problem is that many of the changes are due to to some dependencies needing code updates.
Of specific note is the update of Gradle to 7.3.3 forces known-bad log4j versions out of the dependency tree. The gradle build script was not kept up-to-date (but the changes made ended up being very similar to those already made in atlas-generator, IIRC).

Beyond that, the checkstyle update forced some changes (due to fixed/new checks), and the update for osmosis forced an update to some classes (for the most part, renaming release to close).

With that being said, I can split it out into different changes, but I'll go ahead and rebase so that the commits are (a) in logical order and (b) in logical chunks (in other words, so that the squash doesn't have to happen).

Signed-off-by: Taylor Smock <[email protected]>

tsmock · 2022-01-12T23:39:05Z

OK. I've force-pushed only the log4j changes.

Signed-off-by: Taylor Smock <[email protected]>

sonarqubecloud · 2022-01-13T14:28:11Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

adahn6 · 2022-01-13T16:58:44Z

@tsmock there's also a few changes here that seem unrelated to the log4j changes-- do you mind splitting those out into separate PRs? it will help make this one a bit easier to deal with if we need to rollback or debug

The problem is that many of the changes are due to to some dependencies needing code updates. Of specific note is the update of Gradle to 7.3.3 forces known-bad log4j versions out of the dependency tree. The gradle build script was not kept up-to-date (but the changes made ended up being very similar to those already made in atlas-generator, IIRC).

Beyond that, the checkstyle update forced some changes (due to fixed/new checks), and the update for osmosis forced an update to some classes (for the most part, renaming release to close).

With that being said, I can split it out into different changes, but I'll go ahead and rebase so that the commits are (a) in logical order and (b) in logical chunks (in other words, so that the squash doesn't have to happen).

ah, okay, I didn't realize that, but it makes sense. sorry for the hassle, we've had to rollback issues before so I'm always a little cautious about big changes. let me merge this and do a couple runs myself, then if everything looks good I'll go ahead and release and we can start reviewing the other dependency updates

tsmock · 2022-01-13T17:06:47Z

Fair enough. My test process was with atlas-checks -- I modified settings.gradle to include atlas/atlas-generator builds (which replace the pinned versions of both), and then used a local copy of NZL atlas files for testing.

* Update dependencies This only updates dependencies that did not require code rewriting. Dependencies that require code rewriting include osmosis, which blocks protobuf. Signed-off-by: Taylor Smock <[email protected]> * Fix broken BeanUtils test BeanUtils 1.9.2 added the ability to prevent access to the `class` field. BeanUtils 1.9.4 prevented this by default. The specific test used the `class` field to check for non-instantiable object handling. Signed-off-by: Taylor Smock <[email protected]> * Update to Osmosis 0.48.3 from 0.44.1 This required updating some classes, as one of the primary differences is support for Java 8 AutoCloseable. Signed-off-by: Taylor Smock <[email protected]> * Update checkstyle Signed-off-by: Taylor Smock <[email protected]> * Gradle: add missing dependencies Also, some prework for Gradle 7.x. Signed-off-by: Taylor Smock <[email protected]> * Update Gradle to 7.3.3 Signed-off-by: Taylor Smock <[email protected]> * Dependency updates Signed-off-by: Taylor Smock <[email protected]> Co-authored-by: Samuel Gass <[email protected]>

This was referenced Dec 16, 2021

PersistenceTools: log4j -> slf4j logging osmlab/atlas-generator#196

Merged

Update log4j to 2.17.1 osmlab/atlas-checks#644

Merged

tsmock force-pushed the cve-2021-44228 branch 2 times, most recently from 6f692ee to 437ed9b Compare December 29, 2021 21:41

tsmock changed the title ~~Update log4j to 2.16 to fix DDOS attacks~~ Update log4j to 2.17.1 to fix DDOS attacks Dec 29, 2021

tsmock marked this pull request as draft December 30, 2021 13:30

tsmock marked this pull request as ready for review January 4, 2022 18:09

tsmock force-pushed the cve-2021-44228 branch from 768ec21 to 044121b Compare January 12, 2022 23:00

Update log4j to 2.17.1 to fix DDOS attacks

e36908c

Signed-off-by: Taylor Smock <[email protected]>

tsmock force-pushed the cve-2021-44228 branch from 044121b to 8263223 Compare January 12, 2022 23:36

tsmock force-pushed the cve-2021-44228 branch from 8263223 to e36908c Compare January 12, 2022 23:43

Gradle: Exclude log4j 1.2.17 from osgeo repo

e25cae9

Signed-off-by: Taylor Smock <[email protected]>

adahn6 approved these changes Jan 13, 2022

View reviewed changes

adahn6 merged commit fb8a582 into osmlab:dev Jan 13, 2022

tsmock deleted the cve-2021-44228 branch January 13, 2022 17:00

Update log4j to 2.17.1 to fix DDOS attacks #780

Update log4j to 2.17.1 to fix DDOS attacks #780

Uh oh!

Conversation

tsmock commented Dec 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Potential Impact:

Unit Test Approach:

Test Results:

Uh oh!

jwpgage commented Dec 16, 2021

Uh oh!

tsmock commented Dec 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atiannicelli commented Dec 20, 2021

Uh oh!

tsmock commented Dec 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsmock commented Dec 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsmock commented Jan 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tsmock commented Jan 12, 2022

Uh oh!

atiannicelli commented Jan 12, 2022

Uh oh!

adahn6 commented Jan 12, 2022

Uh oh!

adahn6 commented Jan 12, 2022

Uh oh!

adahn6 commented Jan 12, 2022

Uh oh!

tsmock commented Jan 12, 2022

Uh oh!

tsmock commented Jan 12, 2022

Uh oh!

sonarqubecloud bot commented Jan 13, 2022

Uh oh!

adahn6 commented Jan 13, 2022

Uh oh!

tsmock commented Jan 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tsmock commented Dec 16, 2021 •

edited

Loading

tsmock commented Dec 16, 2021 •

edited

Loading

tsmock commented Dec 27, 2021 •

edited

Loading

tsmock commented Dec 29, 2021 •

edited

Loading

tsmock commented Jan 4, 2022 •

edited

Loading