Config order issue - environment protocol proxy overrides system all.proxy

[GrosQuildu]

Description

The README says Fabric8 client auto-configuration should prefer sources in this order: system properties, environment variables, kube config file, then service account token and mounted CA certificate. It also states that system properties are preferred over environment variables.

That invariant is broken for proxy configuration when a system all.proxy and environment HTTP_PROXY are both present.

Utils.getSystemPropertyOrEnvVar correctly prefers a system property over the matching environment variable for one key. However, Config.configFromSysPropsOrEnvVars reads proxy settings in two independent steps: it first assigns all.proxy, then assigns http.proxy. If the caller sets higher-priority -Dall.proxy=http://system-all-proxy:8080 and the process environment contains lower-priority HTTP_PROXY=http://env-proxy:8080, the later http.proxy lookup sees no -Dhttp.proxy and overwrites the system all.proxy value with HTTP_PROXY.

Exploit Scenario

A build agent, shell profile, wrapper script, or container image sets HTTP_PROXY to an attacker-controlled proxy. The victim launches a Fabric8 program with -Dall.proxy=http://system-all-proxy:8080, expecting the higher priority system property to force traffic through the trusted proxy. Fabric8 instead uses the lower-priority environment proxy for HTTP API servers. The proxy can observe request metadata, block traffic, or become part of a larger credential-interception chain if TLS verification is also weakened.

Add the following test as kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/utils/ProxyConfigOrderInvariantPoCTest.java:

package io.fabric8.kubernetes.client.utils;

import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.ConfigBuilder;
import org.junit.jupiter.api.Test;

import java.nio.charset.StandardCharsets;
import java.nio.file.Path;

import static org.assertj.core.api.Assertions.assertThat;

class ProxyConfigOrderInvariantPoCTest {

  @Test
  void environmentHttpProxyOverridesSystemAllProxy() throws Exception {
    assertThat(runChild()).isEqualTo("http://env-proxy:8080");
  }

  private static String runChild() throws Exception {
    String java = Path.of(System.getProperty("java.home"), "bin", "java").toString();
    ProcessBuilder processBuilder = new ProcessBuilder(
        java,
        "-cp",
        System.getProperty("java.class.path"),
        Child.class.getName());
    processBuilder.environment().put("HTTP_PROXY", "http://env-proxy:8080");
    processBuilder.environment().put("HTTPS_PROXY", "");
    processBuilder.environment().put("NO_PROXY", "");
    processBuilder.environment().put("ALL_PROXY", "");
    processBuilder.redirectErrorStream(true);

    Process process = processBuilder.start();
    String output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim();

    assertThat(process.waitFor()).isEqualTo(0);
    return output;
  }

  public static final class Child {
    public static void main(String[] args) {
      System.setProperty("kubeconfig", "/dev/null");
      System.setProperty("kubernetes.master", "http://api.example");
      System.setProperty("all.proxy", "http://system-all-proxy:8080");

      Config config = new ConfigBuilder().build();

      System.out.print(config.getHttpProxy());
    }
  }
}

The proxy PoCs passed on an unpatched disposable worktree:

./mvnw -pl kubernetes-client-api -Dtest=ProxyConfigOrderInvariantPoCTest test
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

After applying the patch below, the unchanged PoC fails because the child JVM prints http://system-all-proxy:8080. Updating the regression expectation to that system-property proxy passes.

Threat Model

Seems like a technical issue that violates the config order invariant. But requires careful consideration to not break users.

Fix

Select proxy values by source rank instead of by sequential assignment. Preserve the existing kubeconfig proxy-url behavior by leaving a pre-existing proxy value in place when one was already set before system and environment overlay.

diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java
index 1f71105aa0..da48151a20 100644
--- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java
+++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java
@@ -536,14 +536,14 @@ public class Config extends SundrioConfig {
     // Only set http(s) proxy fields if they're not set. This is done in order to align behavior of
     // KubernetesClient with kubectl / client-go . Please see https://github.com/fabric8io/kubernetes-client/issues/6150
     // Precedence is given to proxy-url read from kubeconfig .
+    ConfigValue httpProxy = proxyConfigValue(KUBERNETES_HTTP_PROXY, config.getHttpProxy());
     if (Utils.isNullOrEmpty(config.getHttpProxy())) {
-      config.setHttpProxy(Utils.getSystemPropertyOrEnvVar(KUBERNETES_ALL_PROXY, config.getHttpProxy()));
-      config.setHttpProxy(Utils.getSystemPropertyOrEnvVar(KUBERNETES_HTTP_PROXY, config.getHttpProxy()));
+      config.setHttpProxy(httpProxy.value);
     }
+    ConfigValue httpsProxy = proxyConfigValue(KUBERNETES_HTTPS_PROXY, config.getHttpsProxy());
     if (Utils.isNullOrEmpty(config.getHttpsProxy())) {
-      config.setHttpsProxy(Utils.getSystemPropertyOrEnvVar(KUBERNETES_ALL_PROXY, config.getHttpsProxy()));
-      config.setHttpsProxy(Utils.getSystemPropertyOrEnvVar(KUBERNETES_HTTPS_PROXY, config.getHttpsProxy()));
+      config.setHttpsProxy(httpsProxy.value);
     }

     config.setProxyUsername(Utils.getSystemPropertyOrEnvVar(KUBERNETES_PROXY_USERNAME, config.getProxyUsername()));
@@ -564,6 +564,41 @@ public class Config extends SundrioConfig {
     }
   }

+  private static ConfigValue proxyConfigValue(String protocolKey, String existing) {
+    String value = System.getProperty(protocolKey);
+    if (Utils.isNotNullOrEmpty(value)) {
+      return new ConfigValue(value, 4);
+    }
+    value = System.getProperty(KUBERNETES_ALL_PROXY);
+    if (Utils.isNotNullOrEmpty(value)) {
+      return new ConfigValue(value, 3);
+    }
+    value = System.getenv(Utils.convertSystemPropertyNameToEnvVar(protocolKey));
+    if (Utils.isNotNullOrEmpty(value)) {
+      return new ConfigValue(value, 2);
+    }
+    value = System.getenv(Utils.convertSystemPropertyNameToEnvVar(KUBERNETES_ALL_PROXY));
+    if (Utils.isNotNullOrEmpty(value)) {
+      return new ConfigValue(value, 1);
+    }
+    return new ConfigValue(existing, Utils.isNotNullOrEmpty(existing) ? 5 : 0);
+  }
+
+  private static final class ConfigValue {
+    private final String value;
+    private final int rank;
+
+    private ConfigValue(String value, int rank) {
+      this.value = value;
+      this.rank = rank;
+    }
+  }
+

---

Paweł Płatek from Trail of Bits in collaboration with OpenAI.