Repository navigation
Stop trusting compromised eMudhra roots in the exploration-tests image - #13026
eliottness wants to merge 1 commit into
Conversation
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
Upgrade ca-certificates in the debian:bookworm-slim base and delete emSign ECC Root CA - G3 / AAA Certificate Services from every SDKMAN and Gradle-provisioned JDK cacerts in the exploration-tests image (IR-62343). The image is published by the manual build-exploration-tests-image job; re-run it after merge. Jira: APPSEC-71277
626140e to
e253125
Compare
eliottness
added
tag: no release notes
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 5 unstable metrics. See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (353.737 µs) : 326, 381
. : milestone, 354,
basic (325.55 µs) : 318, 333
. : milestone, 326,
loop (7.582 ms) : 7575, 7588
. : milestone, 7582,
section candidate
noprobe (351.636 µs) : 314, 389
. : milestone, 352,
basic (324.815 µs) : 319, 331
. : milestone, 325,
loop (7.604 ms) : 7537, 7671
. : milestone, 7604,
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e25312578b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| RUN set -eu; \ | ||
| mkdir -p "$HOME/.gradle/jdks"; \ | ||
| keytool="$HOME/.sdkman/candidates/java/17.0.10-tem/bin/keytool"; \ | ||
| find "$HOME/.sdkman/candidates/java" "$HOME/.gradle/jdks" -type f -path '*/lib/security/cacerts' | \ |
There was a problem hiding this comment.
In the OkHttp job, .gitlab/exploration-tests.yml:148-150 requests a Java 11 test launcher, while this image installs only SDKMAN Java 8 and 17. Gradle provisions missing toolchains on demand, so the Java 11 store is created under $HOME/.gradle/jdks only when jvmTest runs, after this build-time find has completed; that test JVM consequently retains both affected roots. Pre-provision Java 11 before this cleanup or scrub newly provisioned stores at job startup.
Useful? React with 👍 / 👎.
| find "$HOME/.sdkman/candidates/java" "$HOME/.gradle/jdks" -type f -path '*/lib/security/cacerts' | \ | ||
| while IFS= read -r store; do \ | ||
| "$keytool" -list -v -keystore "$store" -storepass changeit | \ | ||
| awk '/^Alias name: / { alias = substr($0, 13) } /^Owner: .*([eE]mSign ECC Root CA - G3|AAA Certificate Services)/ { print alias }' | \ |
There was a problem hiding this comment.
For the pinned Java 17.0.10 installation, this predicate leaves emSign Root CA - G1 and emSign Root CA - G2 in cacerts; that JDK release explicitly added all three eMudhra roots, and Oracle's incident mitigation requires removing G1, G2, ECC G3, and AAA. Consequently the image's existing Java 17 runtime remains only partially mitigated even when this loop succeeds. Match the G1 and G2 owner subjects as well, or replace the pinned runtimes with fixed JDK builds.
Useful? React with 👍 / 👎.
| # Remove compromised CA entries from every SDKMAN and Gradle-provisioned JDK. | ||
| RUN set -eu; \ |
There was a problem hiding this comment.
This cleanup runs only after the Maven dependency resolutions at lines 31, 39, and 44 and the Gradle resolution at line 58, so those downloads use the newly installed standalone JDK stores before the required root removals. In the compromised-CA attack scenario this change addresses, a forged repository certificate can therefore inject artifacts into the image's warmed caches, and deleting the trust anchors afterward does not remove those artifacts. Scrub the SDKMAN JDKs immediately after installation, then separately repeat the cleanup after any later toolchain provisioning.
Useful? React with 👍 / 👎.
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deleting Gradle's generated API jar leaves AAA Certificate Services in its source google.p12 truststore. Gradle can continue trusting that root independently of JDK cacerts and regenerate the removed snapshot.
🤖 Bits Code Review · Commit e253125
|
|
||
| # Do not publish Gradle's generated API jar with its old embedded CA snapshot. | ||
| RUN if [ -d "$HOME/.gradle/caches" ]; then \ | ||
| find "$HOME/.gradle/caches" -type f -path '*/generated-gradle-jars/gradle-api-*.jar' -delete; \ |
There was a problem hiding this comment.
After the okhttp warm-up, Gradle's cached distribution retains google-api-client-*.jar, whose embedded google.p12 truststore contains AAA Certificate Services. Gradle's GCS transport uses this store independently of JDK cacerts. Deleting the generated API jar therefore leaves the root trusted and permits regeneration of the same CA snapshot. Sanitize the source truststore as well as removing generated copies.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
What Does This Do
Stops the debugger exploration-tests image from trusting emSign ECC Root CA - G3 and AAA Certificate Services.
dd-java-agent/agent-debugger/exploration-tests/Dockerfile.exploration-tests(basedebian:bookworm-slim):ca-certificatesand re-runsupdate-ca-certificates -f;cacerts;gradle-api-*.jarfiles, which embed an old CA snapshot. The./gradlew dependencieswarm-up step is unchanged; the CA cleanup runs after it.The APPSEC-71273 ledger lists 17 impacted image references in this repository: 0 fixed here, 1 fixed at their in-repo image source (clean once that image is republished), 16 follow-ups.
Motivation
Incident IR-62343: the compromised eMudhra intermediates chain to emSign ECC Root CA - G3 and AAA Certificate Services, and every CI image whose CA stores still trust them keeps the exposure.
Additional Notes
registry.ddbuild.io/ci/dd-trace-java:exploration-testsimage, so this change takes effect only after someone runs the manualbuild-exploration-tests-imagejob once it is merged. That tag has no clean published version yet.exploration-tests-method-jsoupfailed on this PR. The failure cannot come from this Dockerfile, because the job used the unchanged prebuilt image; touchingdd-java-agent/agent-debugger/**only made the job run. Other PRs touching the debugger passed the same job that day (Require explicit locales and charsets in tracer code #13020 and Close tracers built by debugger tests (quick fix) #13040), so this PR was rebased onto master to re-run the pipeline.Follow-ups (not in this PR):
registry.ddbuild.io/images/mirror/amazon/aws-cli:2.4.29,486234852809.dkr.ecr.us-east-1.amazonaws.com/ci/benchmarking-platform:java-dsm-kafka,486234852809.dkr.ecr.us-east-1.amazonaws.com/ci/benchmarking-platform:java-debugger,registry.ddbuild.io/images/mirror/dd-apm-test-agent/ddapm-test-agent:v1.64.1,registry.ddbuild.io/images/mirror/datadog/agent:7.40.1,registry.ddbuild.io/images/mirror/sqlserver:latest,registry.ddbuild.io/images/mirror/dd-trace-java-docker-build:ci-base,registry.ddbuild.io/images/mirror/dd-trace-java-docker-build:ci-7,registry.ddbuild.io/images/mirror/dd-trace-java-docker-build:ci-8,registry.ddbuild.io/images/mirror/dd-trace-java-docker-build:ci-11,registry.ddbuild.io/images/mirror/dd-trace-java-docker-build:ci-17,registry.ddbuild.io/images/mirror/dd-trace-java-docker-build:ci-21,registry.ddbuild.io/images/mirror/dd-trace-java-docker-build:ci-25.gcr.io/google.com/cloudsdktool/google-cloud-cli:emulators,mcr.microsoft.com/mssql/server:latest.486234852809.dkr.ecr.us-east-1.amazonaws.com/ci/agent-key-management-tools/gpg:1.Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: APPSEC-71277 (parent APPSEC-71273)