Merge pull request #91745 from Bisnode/gh-87369

Presence of bearer token should cancel exec action

hack/lib/util.sh

@@ -452,6 +452,22 @@ function kube::util::test_openssl_installed {
     OPENSSL_BIN=$(command -v openssl)
 }
 
+# Query the API server for client certificate authentication capabilities
+function kube::util::test_client_certificate_authentication_enabled {
+  local output
+  kube::util::test_openssl_installed
+
+  output=$(echo \
+    | "${OPENSSL_BIN}" s_client -connect "127.0.0.1:${SECURE_API_PORT}" 2> /dev/null \
+    | grep -A3 'Acceptable client certificate CA names')
+
+  if [[ "${output}" != *"/CN=127.0.0.1"* ]] && [[ "${output}" != *"CN = 127.0.0.1"* ]]; then
+    echo "API server not configured for client certificate authentication"
+    echo "Output of from acceptable client certificate check: ${output}"
+    exit 1
+  fi
+}
+
 # creates a client CA, args are sudo, dest-dir, ca-id, purpose
 # purpose is dropped in after "key encipherment", you usually want
 # '"client auth"'

hack/make-rules/test-cmd.sh

@@ -69,6 +69,7 @@ function run_kube_apiserver() {
     --storage-media-type="${KUBE_TEST_API_STORAGE_TYPE-}" \
     --cert-dir="${TMPDIR:-/tmp/}" \
     --service-cluster-ip-range="10.0.0.0/24" \
+    --client-ca-file=hack/testdata/ca.crt \
     --token-auth-file=hack/testdata/auth-tokens.csv 1>&2 &
   export APISERVER_PID=$!
 

hack/testdata/ca.crt

@@ -0,0 +1,17 @@
+-----BEGIN CERTIFICATE-----
+MIICpjCCAY4CCQCZBiNB23olFzANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDDAkx
+MjcuMC4wLjEwIBcNMjAwNjE0MTk0OTM4WhgPMjI5NDAzMzAxOTQ5MzhaMBQxEjAQ
+BgNVBAMMCTEyNy4wLjAuMTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
+AMxIjMd58IhiiyK4VjmuCWBUZksSs1CcQuo5HSpOqogVZ+vR5mdJDZ56Pw/NSM5c
+RqOB3cvjGrxYQe/lKvo9D3UmWLcRKtxdlWxCfPekioJ25/dhGOxtBQcjtp/TSqTM
+txprwT4fvsVwiwaURFoCOivF4xjQFG0K1i3/m7CiMHODy67M1EfJDrM7Vv5XPIuJ
+VF8HhWBH2HiM25ak34XhxVTX8K97k6wO9OZ5GMqbYuVobTZrSRdiv8s95rkmik6P
+jn0ePKqSz6cXNXgXqTl11WtsuoGgjOdB8j/noqTF3m3z17sSBqqG/xBFuSFoNceA
+yBDb9ohbs8oY3NIZzyMrt8MCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAFgcaqRgv
+qylx4ogL5iUr0K2e/8YzsvH7zLHG6xnr7HxpR/p0lQt3dPlppECZMGDKElbCgU8f
+xVDdZ3FOxHTJ51Vnq/U5xJo+UOMJ4sS8fEH8cfNliSsvmSKzjxpPKqbCJ7VTnkW8
+lonedCPRksnhlD1U8CF21rEjKsXcLoX5PsxlS4DX3PtO0+e8aUh9F4XyZagpejq8
+0ttXkWd3IyYrpFRGDlFDxIiKx7pf+mG6JZ/ms6jloBSwwcz/Nkn5FMxiq75bQuOH
+EV+99S2du/X2bRmD1JxCiMDw8cMacIFBr6BYXsvKOlivwfHBWk8U0f+lVi60jWje
+PpKFRd1mYuEZgw==
+-----END CERTIFICATE-----

staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go

@@ -237,6 +237,15 @@ type credentials struct {
 // UpdateTransportConfig updates the transport.Config to use credentials
 // returned by the plugin.
 func (a *Authenticator) UpdateTransportConfig(c *transport.Config) error {
+	// If a bearer token is present in the request - avoid the GetCert callback when
+	// setting up the transport, as that triggers the exec action if the server is
+	// also configured to allow client certificates for authentication. For requests
+	// like "kubectl get --token (token) pods" we should assume the intention is to
+	// use the provided token for authentication.
+	if c.HasTokenAuth() {
+		return nil
+	}
+
 	c.Wrap(func(rt http.RoundTripper) http.RoundTripper {
 		return &roundTripper{a, rt}
 	})

staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go

@@ -651,6 +651,27 @@ func TestRoundTripper(t *testing.T) {
 	get(t, http.StatusOK)
 }
 
+func TestTokenPresentCancelsExecAction(t *testing.T) {
+	a, err := newAuthenticator(newCache(), &api.ExecConfig{
+		Command:    "./testdata/test-plugin.sh",
+		APIVersion: "client.authentication.k8s.io/v1alpha1",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// UpdateTransportConfig returns error on existing TLS certificate callback, unless a bearer token is present in the
+	// transport config, in which case it takes precedence
+	cert := func() (*tls.Certificate, error) {
+		return nil, nil
+	}
+	tc := &transport.Config{BearerToken: "token1", TLS: transport.TLSConfig{Insecure: true, GetCert: cert}}
+
+	if err := a.UpdateTransportConfig(tc); err != nil {
+		t.Error("Expected presence of bearer token in config to cancel exec action")
+	}
+}
+
 func TestTLSCredentials(t *testing.T) {
 	now := time.Now()
 

test/cmd/authentication.sh

@@ -0,0 +1,81 @@
+#!/usr/bin/env bash
+
+# Copyright 2020 The Kubernetes Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+run_exec_credentials_tests() {
+  set -o nounset
+  set -o errexit
+
+  kube::log::status "Testing kubectl with configured exec credentials plugin"
+
+  cat > "${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml << EOF
+apiVersion: v1
+clusters:
+- cluster:
+  name: test
+contexts:
+- context:
+    cluster: test
+    user: invalid_token_user
+  name: test
+current-context: test
+kind: Config
+preferences: {}
+users:
+- name: invalid_token_user
+  user:
+    exec:
+      apiVersion: client.authentication.k8s.io/v1beta1
+      # Any invalid exec credential plugin will do to demonstrate
+      command: ls
+EOF
+
+  ### Provided --token should take precedence, thus not triggering the (invalid) exec credential plugin
+  # Pre-condition: Client certificate authentication enabled on the API server
+  kube::util::test_client_certificate_authentication_enabled
+  # Command
+  output=$(kubectl "${kube_flags_with_token[@]:?}" --kubeconfig="${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml get namespace kube-system -o name || true)
+
+  if [[ "${output}" == "namespace/kube-system" ]]; then
+    kube::log::status "exec credential plugin not triggered since kubectl was called with provided --token"
+  else
+    kube::log::status "Unexpected output when providing --token for authentication - exec credential plugin likely triggered. Output: ${output}"
+    exit 1    
+  fi
+  # Post-condition: None
+
+  ### Without provided --token, the exec credential plugin should be triggered
+  # Pre-condition: Client certificate authentication enabled on the API server - already checked by positive test above
+
+  # Command
+  output2=$(kubectl "${kube_flags_without_token[@]:?}" --kubeconfig="${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml get namespace kube-system -o name 2>&1 || true)
+
+  if [[ "${output2}" =~ "json parse error" ]]; then
+    kube::log::status "exec credential plugin triggered since kubectl was called without provided --token"
+  else
+    kube::log::status "Unexpected output when not providing --token for authentication - exec credential plugin not triggered. Output: ${output2}"
+    exit 1
+  fi
+  # Post-condition: None
+
+  rm "${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml
+
+  set +o nounset
+  set +o errexit
+}

test/cmd/legacy-script.sh

@@ -29,6 +29,7 @@ KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/../..
 # source "${KUBE_ROOT}/hack/lib/test.sh"
 source "${KUBE_ROOT}/test/cmd/apply.sh"
 source "${KUBE_ROOT}/test/cmd/apps.sh"
+source "${KUBE_ROOT}/test/cmd/authentication.sh"
 source "${KUBE_ROOT}/test/cmd/authorization.sh"
 source "${KUBE_ROOT}/test/cmd/batch.sh"
 source "${KUBE_ROOT}/test/cmd/certificate.sh"
@@ -341,11 +342,13 @@ runTests() {
     '-s' "http://127.0.0.1:${API_PORT}"
   )
 
-  # token defined in hack/testdata/auth-tokens.csv
-  kube_flags_with_token=(
-    '-s' "https://127.0.0.1:${SECURE_API_PORT}" '--token=admin-token' '--insecure-skip-tls-verify=true'
+  kube_flags_without_token=(
+    '-s' "https://127.0.0.1:${SECURE_API_PORT}" '--insecure-skip-tls-verify=true'
   )
 
+  # token defined in hack/testdata/auth-tokens.csv
+  kube_flags_with_token=( "${kube_flags_without_token[@]}" '--token=admin-token' )
+
   if [[ -z "${ALLOW_SKEW:-}" ]]; then
     kube_flags+=('--match-server-version')
     kube_flags_with_token+=('--match-server-version')
@@ -760,6 +763,11 @@ runTests() {
     record_command run_nodes_tests
   fi
 
+  ########################
+  # Authentication
+  ########################
+
+  record_command run_exec_credentials_tests
 
   ########################
   # authorization.k8s.io #