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 snake CVE #4837

Merged
merged 16 commits into from
Aug 15, 2024
Merged

Fix snake CVE #4837

merged 16 commits into from
Aug 15, 2024

Conversation

rkovalik-raft
Copy link
Contributor

Updates Kubernetes api client-java version to 21.0.0-legacy, which fixes CVE issue related to snake.yaml. I chose the 21.0.0-legacy version because we would need to make significant code changes if we used the 21.0.0 version, since that version makes a method we rely on private instead of public.

Compiles and passes unit tests for hmda-platform and check-digit services, working on testing other services.

@rkovalik-raft rkovalik-raft marked this pull request as draft July 8, 2024 17:29
@rkovalik-raft
Copy link
Contributor Author

Note that the higher s3mock version requires Java 17, and tests/builds still fail due to cassandra issues

@rkovalik-raft
Copy link
Contributor Author

Note: to run this with sbt, need to use this command:
env JDK_JAVA_OPTIONS="--add-opens java.base/java.util=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.io=ALL-UNNAMED --add-opens=java.base/sun.nio.ch=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED" sbt

There are still some errors in hmda-platform when running locally, but it passes tests and the docker image builds fine.

Also ratespread-calculator still has cassandra related errors that cause a test to fail.

@@ -162,6 +162,7 @@ lazy val `hmda-platform` = (project in file("hmda"))
case "logback.xml" => MergeStrategy.concat
case "META-INF/MANIFEST.MF" => MergeStrategy.discard
case PathList("META-INF", xs@_*) => MergeStrategy.concat
case PathList("org", "bouncycastle", xs @_*) => MergeStrategy.first
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes merge strategy errors, added for all services

@@ -18,7 +18,7 @@ class RealTimeConfig(val cmName: String, val ns: String) {
val factory = new SharedInformerFactory(client)
val informer = factory.sharedIndexInformerFor((params: CallGeneratorParams) => {
api.listNamespacedConfigMapCall(
ns, null, null, null, s"metadata.name=$cmName", null, null, params.resourceVersion, null, params.timeoutSeconds, params.watch, null)
ns, null, null, null, s"metadata.name=$cmName", null, null, params.resourceVersion, null, null, params.timeoutSeconds, params.watch, null)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to the kubernetes client-java dependency upgrade, this method now has another parameter, so I added a null parameter value here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deleted since it was not used and relied on S3Mock dependency, which I removed to avoid some big dependency version issues

@@ -17,7 +17,7 @@ import io.lettuce.core.{ ClientOptions, RedisClient }
import monix.eval.Task
import slick.basic.DatabaseConfig
import slick.jdbc.JdbcProfile
import ch.megard.akka.http.cors.scaladsl.CorsDirectives._
import ch.megard.akka.http.cors.scaladsl.CorsDirectives.cors
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the cors related changes in these files fix the "ambiguous reference errors" from the new akka dependency version.

@@ -160,38 +160,6 @@ spec:
key: aws-region
- name: BROWSER_LOG_LEVEL
value: {{.Values.databrowser.loglevel}}
- name: DATABROWSER_PG_TABLE_2017
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed because when I tried to deploy data-browser in hmda dev with a helm command I ran into errors related to missing mlartable tables in the configmap

//package hmda.publication.lar.publication
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commenting out these tests that rely on S3Mock, which is removed in this PR. Leaving the test files here so we can look into re-implementing them without S3Mock somehow in the future.

lazy val slick = "com.typesafe.slick" %% "slick" % Version.slick
lazy val slickHikariCP = "com.typesafe.slick" %% "slick-hikaricp" % Version.slick
lazy val alpakkaSlick = "com.lightbend.akka" %% "akka-stream-alpakka-slick" % Version.alpakka
lazy val postgres = "org.postgresql" % "postgresql" % Version.postgres
lazy val h2 = "com.h2database" % "h2" % Version.h2 % Test
lazy val testContainers = "org.testcontainers" % "testcontainers" % Version.testContainers % "test"
lazy val s3Mock = "com.adobe.testing" % "s3mock" % "2.1.19" % Test
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving the version numbers into Version.scala

@@ -37,6 +40,6 @@ object Version {
val scalacheckShapeless = "1.2.5"
val diffx = "0.4.0"
val log4j = "2.15.0"
val kubernetesApi = "15.0.1"
val kubernetesApi = "21.0.0-legacy"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upgrading this past version 17 will fix the snake.yaml cve issue. I'm using 21.0.0-legacy version here because we would need to make significant code changes if we used the 21.0.0 version, since that version makes a method we rely on private instead of public.

addSbtPlugin("org.scoverage" % "sbt-scoverage" % "2.0.10")
addSbtPlugin("net.virtual-void" % "sbt-dependency-graph" % "0.10.0-RC1")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allows us to generate dependency graphs for our services

//package hmda.calculator.scheduler
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another test that is commented out because it relies on S3Mock

@rkovalik-raft rkovalik-raft changed the title (WIP) Fix snake CVE Fix snake CVE Aug 13, 2024
@rkovalik-raft rkovalik-raft marked this pull request as ready for review August 13, 2024 17:02
Copy link
Contributor

@PatrickGoRaft PatrickGoRaft left a comment

Choose a reason for hiding this comment

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

These changes LGTM.

Pending verification in Dev environment smoke testing

@rkovalik-raft
Copy link
Contributor Author

Confirmed with @Michaeldremy that the Cypress DEV tests looked good

@rkovalik-raft rkovalik-raft merged commit a1b358f into cfpb:master Aug 15, 2024
2 of 3 checks passed
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.

2 participants