Presence of bearer token should cancel exec action

If a bearer token is present in a request, the exec credential plugin should accept that as the chosen method of authentication. Judging by an [earlier comment in exec.go](https://github.com/kubernetes/kubernetes/blob/c18bc7e9f7a27d7d197053d98c52896e1dc3bb3e/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go#L217), this was already intended. This would however not work since UpdateTransportConfig would set the GetCert callback which would then get called by the transport, triggering the exec plugin action even with a token present in the request. See linked issue for further details.

See kubernetes#87369 for further details.

Signed-off-by: Anders Eknert <[email protected]>

Author: anderseknert

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

@@ -178,6 +178,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

@@ -620,6 +620,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,83 @@
+#!/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
+  
+  kube_flags_without_token=('-s' "https://127.0.0.1:${SECURE_API_PORT}" '--insecure-skip-tls-verify=true')
+
+  # 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"
@@ -760,6 +761,11 @@ runTests() {
     record_command run_nodes_tests
   fi
 
+  ########################
+  # Authentication
+  ########################
+
+  record_command run_exec_credentials_tests
 
   ########################
   # authorization.k8s.io #