Skip to content

Security review: Fix SSRF vulnerability and non-security bugs in CIMD executor#1477

Draft
Copilot wants to merge 3 commits intoISSUE-45284from
copilot/review-issue-45284
Draft

Security review: Fix SSRF vulnerability and non-security bugs in CIMD executor#1477
Copilot wants to merge 3 commits intoISSUE-45284from
copilot/review-issue-45284

Conversation

Copy link

Copilot AI commented Feb 28, 2026

Security review of the CIMD (Client ID Metadata Document) feature on branch ISSUE-45284. Found missing SSRF protection in verifyUri() and six non-security bugs.

Description

SSRF Vulnerability (Critical)

The code comments claim loopback/private IP checks exist, but verifyUri() only validates trusted domain names — no DNS resolution check. An attacker with a trusted domain resolving to an internal IP (DNS rebinding) could trigger server-side requests to internal services via fetchClientMetadata().

Fix: Added verifyNotInternalAddress() which resolves hostnames and blocks loopback (127.0.0.0/8, ::1), private/site-local (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16), link-local (169.254.0.0/16, fe80::/10), and wildcard addresses. Gated by allowHttpScheme config — enforced in production (default), skipped in dev mode. Gracefully handles UnknownHostException (no SSRF risk if fetch also fails).

// Production: SSRF check enforced after trusted domain check
if (!getConfiguration().isAllowHttpScheme()) {
    verifyNotInternalAddress(uri.getHost());
}

Non-Security Bug Fixes

  • Swapped comments in process(): CREATE/UPDATE labels were reversed on createClientMetadata/updateClientMetadata calls
  • Unused variable: paContext assigned then context re-cast on next line in ClientIdUriSchemeCondition
  • Wrong cache time comment: 259200s = 3 days, not "30 days"
  • Wrong HTTP status in javadoc: "307 Not Modified" → "304 Not Modified"
  • Misleading config docs: Said "regex" but checkTrustedDomain() uses *. wildcard matching
  • Typo: "mandagory" → "mandatory"

Test Updates

Added setAllowHttpScheme(true) to all test executor configs that use localhost for the full CIMD flow, since localhost resolves to a loopback address now blocked by SSRF protection.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • repo.gradle.org
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.12/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.12/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.12 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.12/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/keycloak/keycloak org.codehaus.plexus.classworlds.launcher.Launcher compile -pl services -nsu -q (dns block)
  • repository.jboss.org
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.12/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.12/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.12 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.12/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/keycloak/keycloak org.codehaus.plexus.classworlds.launcher.Launcher compile -pl services -nsu -q (dns block)
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.12/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.12/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.12 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.12/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/keycloak/keycloak org.codehaus.plexus.classworlds.launcher.Launcher install -DskipTests -nsu -q (dns block)
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --enable-native-access=ALL-UNNAMED -classpath /usr/share/apache-maven-3.9.12/boot/plexus-classworlds-2.9.0.jar -Dclassworlds.conf=/usr/share/apache-maven-3.9.12/bin/m2.conf -Dmaven.home=/usr/share/apache-maven-3.9.12 -Dlibrary.jansi.path=/usr/share/apache-maven-3.9.12/lib/jansi-native -Dmaven.multiModuleProjectDirectory=/home/REDACTED/work/keycloak/keycloak org.codehaus.plexus.classworlds.launcher.Launcher dependency:tree -Dverbose (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 28, 2026 11:24
Non-security fixes:
- Fix swapped CREATE/UPDATE comments in process()
- Fix unused paContext variable in ClientIdUriSchemeCondition
- Fix incorrect max cache time comment (259200s = 3 days, not 30 days)
- Fix HTTP 307 -> 304 Not Modified in javadoc
- Fix misleading regex documentation (code uses wildcard matching)
- Fix typo mandagory -> mandatory

Security fixes:
- Add SSRF protection: verifyNotInternalAddress() blocks loopback,
  private (site-local), link-local, and wildcard addresses
- SSRF check is enforced when allowHttpScheme is false (production)
- SSRF check is skipped when allowHttpScheme is true (dev only)
- Add ERR_SSRF_BLOCKED error constant
- Update tests to use allowHttpScheme=true for dev/test scenarios

Co-authored-by: tnorimat <25092005+tnorimat@users.noreply.github.com>
Co-authored-by: tnorimat <25092005+tnorimat@users.noreply.github.com>
Copilot AI changed the title [WIP] Review issue 45284 for bugs and security improvements Security review: Fix SSRF vulnerability and non-security bugs in CIMD executor Feb 28, 2026
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