Skip to content

Commit fe43b10

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#91745 from Bisnode/kubernetesgh-87369
Presence of bearer token should cancel exec action
2 parents 6dd8d71 + c595f98 commit fe43b10

File tree

7 files changed

+156
-3
lines changed

7 files changed

+156
-3
lines changed

‎hack/lib/util.sh‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,22 @@ function kube::util::test_openssl_installed {
452452
OPENSSL_BIN=$(command -v openssl)
453453
}
454454

455+
# Query the API server for client certificate authentication capabilities
456+
function kube::util::test_client_certificate_authentication_enabled {
457+
local output
458+
kube::util::test_openssl_installed
459+
460+
output=$(echo \
461+
| "${OPENSSL_BIN}" s_client -connect "127.0.0.1:${SECURE_API_PORT}" 2> /dev/null \
462+
| grep -A3 'Acceptable client certificate CA names')
463+
464+
if [[ "${output}" != *"/CN=127.0.0.1"* ]] && [[ "${output}" != *"CN = 127.0.0.1"* ]]; then
465+
echo "API server not configured for client certificate authentication"
466+
echo "Output of from acceptable client certificate check: ${output}"
467+
exit 1
468+
fi
469+
}
470+
455471
# creates a client CA, args are sudo, dest-dir, ca-id, purpose
456472
# purpose is dropped in after "key encipherment", you usually want
457473
# '"client auth"'

‎hack/make-rules/test-cmd.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ function run_kube_apiserver() {
6969
--storage-media-type="${KUBE_TEST_API_STORAGE_TYPE-}" \
7070
--cert-dir="${TMPDIR:-/tmp/}" \
7171
--service-cluster-ip-range="10.0.0.0/24" \
72+
--client-ca-file=hack/testdata/ca.crt \
7273
--token-auth-file=hack/testdata/auth-tokens.csv 1>&2 &
7374
export APISERVER_PID=$!
7475

‎hack/testdata/ca.crt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICpjCCAY4CCQCZBiNB23olFzANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDDAkx
3+
MjcuMC4wLjEwIBcNMjAwNjE0MTk0OTM4WhgPMjI5NDAzMzAxOTQ5MzhaMBQxEjAQ
4+
BgNVBAMMCTEyNy4wLjAuMTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB
5+
AMxIjMd58IhiiyK4VjmuCWBUZksSs1CcQuo5HSpOqogVZ+vR5mdJDZ56Pw/NSM5c
6+
RqOB3cvjGrxYQe/lKvo9D3UmWLcRKtxdlWxCfPekioJ25/dhGOxtBQcjtp/TSqTM
7+
txprwT4fvsVwiwaURFoCOivF4xjQFG0K1i3/m7CiMHODy67M1EfJDrM7Vv5XPIuJ
8+
VF8HhWBH2HiM25ak34XhxVTX8K97k6wO9OZ5GMqbYuVobTZrSRdiv8s95rkmik6P
9+
jn0ePKqSz6cXNXgXqTl11WtsuoGgjOdB8j/noqTF3m3z17sSBqqG/xBFuSFoNceA
10+
yBDb9ohbs8oY3NIZzyMrt8MCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAFgcaqRgv
11+
qylx4ogL5iUr0K2e/8YzsvH7zLHG6xnr7HxpR/p0lQt3dPlppECZMGDKElbCgU8f
12+
xVDdZ3FOxHTJ51Vnq/U5xJo+UOMJ4sS8fEH8cfNliSsvmSKzjxpPKqbCJ7VTnkW8
13+
lonedCPRksnhlD1U8CF21rEjKsXcLoX5PsxlS4DX3PtO0+e8aUh9F4XyZagpejq8
14+
0ttXkWd3IyYrpFRGDlFDxIiKx7pf+mG6JZ/ms6jloBSwwcz/Nkn5FMxiq75bQuOH
15+
EV+99S2du/X2bRmD1JxCiMDw8cMacIFBr6BYXsvKOlivwfHBWk8U0f+lVi60jWje
16+
PpKFRd1mYuEZgw==
17+
-----END CERTIFICATE-----

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,15 @@ type credentials struct {
237237
// UpdateTransportConfig updates the transport.Config to use credentials
238238
// returned by the plugin.
239239
func (a *Authenticator) UpdateTransportConfig(c *transport.Config) error {
240+
// If a bearer token is present in the request - avoid the GetCert callback when
241+
// setting up the transport, as that triggers the exec action if the server is
242+
// also configured to allow client certificates for authentication. For requests
243+
// like "kubectl get --token (token) pods" we should assume the intention is to
244+
// use the provided token for authentication.
245+
if c.HasTokenAuth() {
246+
return nil
247+
}
248+
240249
c.Wrap(func(rt http.RoundTripper) http.RoundTripper {
241250
return &roundTripper{a, rt}
242251
})

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,27 @@ func TestRoundTripper(t *testing.T) {
651651
get(t, http.StatusOK)
652652
}
653653

654+
func TestTokenPresentCancelsExecAction(t *testing.T) {
655+
a, err := newAuthenticator(newCache(), &api.ExecConfig{
656+
Command: "./testdata/test-plugin.sh",
657+
APIVersion: "client.authentication.k8s.io/v1alpha1",
658+
})
659+
if err != nil {
660+
t.Fatal(err)
661+
}
662+
663+
// UpdateTransportConfig returns error on existing TLS certificate callback, unless a bearer token is present in the
664+
// transport config, in which case it takes precedence
665+
cert := func() (*tls.Certificate, error) {
666+
return nil, nil
667+
}
668+
tc := &transport.Config{BearerToken: "token1", TLS: transport.TLSConfig{Insecure: true, GetCert: cert}}
669+
670+
if err := a.UpdateTransportConfig(tc); err != nil {
671+
t.Error("Expected presence of bearer token in config to cancel exec action")
672+
}
673+
}
674+
654675
func TestTLSCredentials(t *testing.T) {
655676
now := time.Now()
656677

‎test/cmd/authentication.sh‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
#!/usr/bin/env bash
2+
3+
# Copyright 2020 The Kubernetes Authors.
4+
#
5+
# Licensed under the Apache License, Version 2.0 (the "License");
6+
# you may not use this file except in compliance with the License.
7+
# You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
set -o errexit
18+
set -o nounset
19+
set -o pipefail
20+
21+
run_exec_credentials_tests() {
22+
set -o nounset
23+
set -o errexit
24+
25+
kube::log::status "Testing kubectl with configured exec credentials plugin"
26+
27+
cat > "${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml << EOF
28+
apiVersion: v1
29+
clusters:
30+
- cluster:
31+
name: test
32+
contexts:
33+
- context:
34+
cluster: test
35+
user: invalid_token_user
36+
name: test
37+
current-context: test
38+
kind: Config
39+
preferences: {}
40+
users:
41+
- name: invalid_token_user
42+
user:
43+
exec:
44+
apiVersion: client.authentication.k8s.io/v1beta1
45+
# Any invalid exec credential plugin will do to demonstrate
46+
command: ls
47+
EOF
48+
49+
### Provided --token should take precedence, thus not triggering the (invalid) exec credential plugin
50+
# Pre-condition: Client certificate authentication enabled on the API server
51+
kube::util::test_client_certificate_authentication_enabled
52+
# Command
53+
output=$(kubectl "${kube_flags_with_token[@]:?}" --kubeconfig="${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml get namespace kube-system -o name || true)
54+
55+
if [[ "${output}" == "namespace/kube-system" ]]; then
56+
kube::log::status "exec credential plugin not triggered since kubectl was called with provided --token"
57+
else
58+
kube::log::status "Unexpected output when providing --token for authentication - exec credential plugin likely triggered. Output: ${output}"
59+
exit 1
60+
fi
61+
# Post-condition: None
62+
63+
### Without provided --token, the exec credential plugin should be triggered
64+
# Pre-condition: Client certificate authentication enabled on the API server - already checked by positive test above
65+
66+
# Command
67+
output2=$(kubectl "${kube_flags_without_token[@]:?}" --kubeconfig="${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml get namespace kube-system -o name 2>&1 || true)
68+
69+
if [[ "${output2}" =~ "json parse error" ]]; then
70+
kube::log::status "exec credential plugin triggered since kubectl was called without provided --token"
71+
else
72+
kube::log::status "Unexpected output when not providing --token for authentication - exec credential plugin not triggered. Output: ${output2}"
73+
exit 1
74+
fi
75+
# Post-condition: None
76+
77+
rm "${TMPDIR:-/tmp}"/invalid_exec_plugin.yaml
78+
79+
set +o nounset
80+
set +o errexit
81+
}

‎test/cmd/legacy-script.sh‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ KUBE_ROOT=$(dirname "${BASH_SOURCE[0]}")/../..
2929
# source "${KUBE_ROOT}/hack/lib/test.sh"
3030
source "${KUBE_ROOT}/test/cmd/apply.sh"
3131
source "${KUBE_ROOT}/test/cmd/apps.sh"
32+
source "${KUBE_ROOT}/test/cmd/authentication.sh"
3233
source "${KUBE_ROOT}/test/cmd/authorization.sh"
3334
source "${KUBE_ROOT}/test/cmd/batch.sh"
3435
source "${KUBE_ROOT}/test/cmd/certificate.sh"
@@ -341,11 +342,13 @@ runTests() {
341342
'-s' "http://127.0.0.1:${API_PORT}"
342343
)
343344

344-
# token defined in hack/testdata/auth-tokens.csv
345-
kube_flags_with_token=(
346-
'-s' "https://127.0.0.1:${SECURE_API_PORT}" '--token=admin-token' '--insecure-skip-tls-verify=true'
345+
kube_flags_without_token=(
346+
'-s' "https://127.0.0.1:${SECURE_API_PORT}" '--insecure-skip-tls-verify=true'
347347
)
348348

349+
# token defined in hack/testdata/auth-tokens.csv
350+
kube_flags_with_token=( "${kube_flags_without_token[@]}" '--token=admin-token' )
351+
349352
if [[ -z "${ALLOW_SKEW:-}" ]]; then
350353
kube_flags+=('--match-server-version')
351354
kube_flags_with_token+=('--match-server-version')
@@ -760,6 +763,11 @@ runTests() {
760763
record_command run_nodes_tests
761764
fi
762765

766+
########################
767+
# Authentication
768+
########################
769+
770+
record_command run_exec_credentials_tests
763771

764772
########################
765773
# authorization.k8s.io #

0 commit comments

Comments
 (0)