Skip to content

fix(kubernetes-client-api): honor source precedence between CA materi… - #8012

Open
aloktomarr wants to merge 1 commit into
fabric8io:mainfrom
aloktomarr:fix/config-tls-source-precedence-7957
Open

fix(kubernetes-client-api): honor source precedence between CA materi…#8012
aloktomarr wants to merge 1 commit into
fabric8io:mainfrom
aloktomarr:fix/config-tls-source-precedence-7957

Conversation

@aloktomarr

Copy link
Copy Markdown

Description

insecure-skip-tls-verify: true in a kube config maps to trustCerts=true (trust-all). When a higher-priority source — a system property or environment variable, per the documented precedence — supplies CA cert material (kubernetes.certs.ca.file / kubernetes.certs.ca.data), that explicit CA was silently ignored: SSLUtils#trustManagers checks isTrustCerts() before the CA material, so trust-all stayed in effect and the connection was left without certificate verification.

Fixes #7957.

Root cause

Config.configFromSysPropsOrEnvVars set trustCerts and the CA file/data independently, with no notion of which source each value came from. A low-priority kube-config trustCerts=true could therefore coexist with a high-priority CA file, and SSLUtils resolved the ambiguity in favor of trust-all.

Fix

configFromSysPropsOrEnvVars now resolves the CA file/data together with the trustCerts and disableHostnameVerification flags, tagging each value with a source rank (system property > environment variable > existing/kube-config value). When CA material comes from a strictly higher-priority source than the insecure flag, the insecure flag is cleared, so the explicitly configured CA is enforced — matching the documented configuration precedence. Configurations where the insecure flag is at the same or higher priority than the CA are unchanged.

Note (flagged by the reporter as "discussable"): this resolves the conflict by letting the higher-priority CA win. The alternative would be to fail fast on contradictory trust configuration. I chose the precedence-consistent behavior since it matches how every other field in this method is resolved — happy to switch to an explicit error if maintainers prefer.

Testing

  • New regression test ConfigTlsSourcePrecedenceTest: writes a temp kube config with insecure-skip-tls-verify: true, sets a higher-priority -Dkubernetes.certs.ca.file, and asserts the CA file is applied and isTrustCerts() is false. Confirmed it fails before the fix and passes after.
  • Full kubernetes-client-api module suite: 667 tests, 0 failures, 0 errors — no regressions.

Reported by @GrosQuildu (Trail of Bits), who also provided a proof-of-concept and a suggested fix.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase; test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CH## Description

insecure-skip-tls-verify: true in a kube config maps to trustCerts=true (trust-all). When a higher-priority source — a system property or environment variable, per the documented precedence — supplies CA cert material (kubernetes.certs.ca.file / kubernetes.certs.ca.data), that explicit CA was silently ignored: SSLUtils#trustManagers checks isTrustCerts() before the CA material, so trust-all stayed in effect and the connection was left without certificate verification.

Fixes #7957.

Root cause

Config.configFromSysPropsOrEnvVars set trustCerts and the CA file/data independently, with no notion of which source each value came from. A low-priority kube-config trustCerts=true could therefore coexist with a high-priority CA file, and SSLUtils resolved the ambiguity in favor of trust-all.

Fix

configFromSysPropsOrEnvVars now resolves the CA file/data together with the trustCerts and disableHostnameVerification flags, tagging each value with a source rank (system property > environment variable > existing/kube-config value). When CA material comes from a strictly higher-priority source than the insecure flag, the insecure flag is cleared, so the explicitly configured CA is enforced — matching the documented configuration precedence. Configurations where the insecure flag is at the same or higher priority than the CA are unchanged.

Note (flagged by the reporter as "discussable"): this resolves the conflict by letting the higher-priority CA win. The alternative would be to fail fast on contradictory trust configuration. I chose the precedence-consistent behavior since it matches how every other field in this method is resolved — happy to switch to an explicit error if maintainers prefer.

Testing

  • New regression test ConfigTlsSourcePrecedenceTest: writes a temp kube config with insecure-skip-tls-verify: true, sets a higher-priority -Dkubernetes.certs.ca.file, and asserts the CA file is applied and isTrustCerts() is false. Confirmed it fails before the fix and passes after.
  • Full kubernetes-client-api module suite: 667 tests, 0 failures, 0 errors — no regressions.

Reported by @GrosQuildu (Trail of Bits), who also provided a proof-of-concept and a suggested fix.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase; test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CH

…al and insecure TLS flags

When kube config set insecure-skip-tls-verify (trustCerts=true) but a higher-priority source (system property or environment variable) supplied a CA cert file/data, SSLUtils evaluated isTrustCerts() before the CA material and left trust-all in effect, so the explicitly pinned CA was silently ignored - a TLS verification bypass.

configFromSysPropsOrEnvVars now resolves the CA file/data together with the trust-all and hostname-verification flags using source ranks (system property > environment variable > existing/kube config value). When CA material comes from a strictly higher-priority source than the insecure flag, the insecure flag is cleared so the explicitly configured CA is enforced, matching the documented configuration precedence.

Reported by @GrosQuildu (Trail of Bits), who provided a proof-of-concept and a suggested fix.

Fixes fabric8io#7957
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.

Config order issue - kubeconfig insecure-skip-tls-verify overrides higher-priority CA material

1 participant