From b423216a3b781009fb4ec4d5974eeb3f882f9d2d Mon Sep 17 00:00:00 2001 From: Anders Eknert Date: Thu, 4 Jun 2020 00:12:05 +0200 Subject: [PATCH 1/2] 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 #87369 for further details. Signed-off-by: Anders Eknert --- hack/lib/util.sh | 16 ++++ hack/make-rules/test-cmd.sh | 1 + hack/testdata/ca.crt | 17 ++++ .../plugin/pkg/client/auth/exec/exec.go | 9 ++ .../plugin/pkg/client/auth/exec/exec_test.go | 21 +++++ test/cmd/authentication.sh | 83 +++++++++++++++++++ test/cmd/legacy-script.sh | 6 ++ 7 files changed, 153 insertions(+) create mode 100644 hack/testdata/ca.crt create mode 100644 test/cmd/authentication.sh diff --git a/hack/lib/util.sh b/hack/lib/util.sh index 80a7b003912..f72ffaabacd 100755 --- a/hack/lib/util.sh +++ b/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"' diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index efd5cb5fbc7..de62051ba73 100755 --- a/hack/make-rules/test-cmd.sh +++ b/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=$! diff --git a/hack/testdata/ca.crt b/hack/testdata/ca.crt new file mode 100644 index 00000000000..6060bec2bda --- /dev/null +++ b/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----- diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go index 6d3544bd1ff..9a2c360d468 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go +++ b/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} }) diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go index 9bef7090fb7..ab81460eaf4 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec_test.go +++ b/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() diff --git a/test/cmd/authentication.sh b/test/cmd/authentication.sh new file mode 100644 index 00000000000..4cd94483595 --- /dev/null +++ b/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 +} diff --git a/test/cmd/legacy-script.sh b/test/cmd/legacy-script.sh index cb117a0d2ab..ee6a9d97038 100755 --- a/test/cmd/legacy-script.sh +++ b/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 # From c595f983fa6b53383557ddbdf698873a38f2275d Mon Sep 17 00:00:00 2001 From: Anders Eknert Date: Mon, 6 Jul 2020 21:55:23 +0200 Subject: [PATCH 2/2] Move kube_flags_without_token creation --- test/cmd/authentication.sh | 2 -- test/cmd/legacy-script.sh | 8 +++++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/cmd/authentication.sh b/test/cmd/authentication.sh index 4cd94483595..c5592a857d5 100644 --- a/test/cmd/authentication.sh +++ b/test/cmd/authentication.sh @@ -62,8 +62,6 @@ EOF ### 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) diff --git a/test/cmd/legacy-script.sh b/test/cmd/legacy-script.sh index ee6a9d97038..9f63b836b5a 100755 --- a/test/cmd/legacy-script.sh +++ b/test/cmd/legacy-script.sh @@ -342,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')