Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exec plugin overrides static certs in kubeconfig (different behavior than over statically configured credentials) #99603

Closed
ankeesler opened this issue Mar 1, 2021 · 11 comments · Fixed by #107410
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/auth Categorizes an issue or PR as relevant to SIG Auth. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@ankeesler
Copy link
Contributor

What happened:

  • My exec plugin overrode the static certs I had in my kubeconfig.

What you expected to happen:

  • The static certs I had in my kubeconfig should take precedent over whatever is configured by my exec plugin.

How to reproduce it (as minimally and precisely as possible):

With admin-like privileges, do the following.

# Generate CSR.
openssl req -nodes -subj /CN=some-username -newkey rsa:2048 -keyout /tmp/aaa.key -out /tmp/aaa.csr

# Create CSR.
cat <<EOF | kubectl replace -f -
apiVersion: certificates.k8s.io/v1
kind: CertificateSigningRequest
metadata:
  name: aaa
spec:
  request: $(cat /tmp/aaa.csr | base64 | tr -d '\n')
  signerName: kubernetes.io/kube-apiserver-client
  usages: [client auth]
EOF

# Issue certificate.
kubectl certificate approve aaa
kubectl get csr aaa -o jsonpath={.status.certificate} | base64 -d > /tmp/aaa.crt

# Make request with both certificate and exec plugin that returns token.
cat <<EOF >/tmp/aaa.sh
#!/bin/bash
echo '{"kind":"ExecCredential","apiVersion":"client.authentication.k8s.io/v1beta1","status":{"clientKeyData":"bad","clientCertificateData":"bad"}}'
EOF
chmod +x /tmp/aaa.sh
kubectl config set-credentials aaa --client-certificate=/tmp/aaa.crt --client-key=/tmp/aaa.key --exec-api-version client.authentication.k8s.io/v1beta1 --exec-command=/tmp/aaa.sh
kubectl --user aaa get pods

Anything else we need to know?:

  • This seems like unexpected behavior because other statically configured credentials (i.e., basic auth, bearer token) take precedent over the stuff returned by the exec plugin.
  • This was pointed out by @enj when he was reviewing some exec plugin integration tests.

Environment:

  • Kubernetes version (use kubectl version): 1.20.0
  • Cloud provider or hardware configuration: kind, but shouldn't matter
  • OS (e.g: cat /etc/os-release): Darwin, but shouldn't matter
  • Kernel (e.g. uname -a): Darwin, but shouldn't matter
  • Install tools:
  • Network plugin and version (if this is a network-related bug):
  • Others:
@ankeesler ankeesler added the kind/bug Categorizes issue or PR as related to a bug. label Mar 1, 2021
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Mar 1, 2021
@ankeesler
Copy link
Contributor Author

/sig auth

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 1, 2021
@enj
Copy link
Member

enj commented Mar 1, 2021

This should also cover the direct case right?

kubectl get ns --client-...=

$ kubectl options |& grep TLS
      --client-certificate='': Path to a client certificate file for TLS
      --client-key='': Path to a client key file for TLS

Precedence should be:

  1. CLI flags
  2. Static config
  3. Exec plugin

@enj
Copy link
Member

enj commented Mar 1, 2021

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 1, 2021
@enj
Copy link
Member

enj commented Mar 1, 2021

/priority important-soon

@k8s-ci-robot k8s-ci-robot added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Mar 1, 2021
@enj enj added this to Backlog in SIG Auth Old Apr 9, 2021
@margocrawf
Copy link
Contributor

/assign

@liggitt
Copy link
Member

liggitt commented May 24, 2021

is this required for exec auth GA in 1.22?

@liggitt liggitt moved this from Needs Triage to Backlog in SIG Auth Old May 24, 2021
@ankeesler
Copy link
Contributor Author

is this required for exec auth GA in 1.22?

I asked @enj that same question week before last (I've been on PTO). @enj posited that this is a "nice to have" (i.e., it shouldn't block GA) since there are other CLI cred override weirdisms.

@enj
Copy link
Member

enj commented Jul 12, 2021

is this required for exec auth GA in 1.22?

I asked @enj that same question week before last (I've been on PTO). @enj posited that this is a "nice to have" (i.e., it shouldn't block GA) since there are other CLI cred override weirdisms.

Based on what I observed in kubernetes-sigs/kind#2229 (comment), it looks like --token does not override client certs in static config, which is wrong. In general we need to fix credential precedence handling in kubectl per #99603 (comment).

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 10, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Nov 9, 2021
@enj
Copy link
Member

enj commented Nov 10, 2021

/remove-lifecycle rotten
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Nov 10, 2021
SIG Auth Old automation moved this from Backlog to Closed / Done Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/auth Categorizes an issue or PR as relevant to SIG Auth. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Archived in project
SIG Auth Old
Closed / Done
Development

Successfully merging a pull request may close this issue.

6 participants