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

Kubelet Server TLS Certificate Rotation #267

Open
jcbsmpsn opened this Issue Apr 24, 2017 · 23 comments

Comments

@jcbsmpsn
Member

jcbsmpsn commented Apr 24, 2017

Feature Description

  • One-line feature description (can be used as a release note):
    Rotation of the server TLS certificate on the kubelet.

  • Primary contact (assignee):
    @jcbsmpsn

  • Responsible SIGs:
    sig-auth

  • Design proposal link (community repo): kubernetes/community#602

  • Reviewer(s) - (for LGTM) recommend having 2+ reviewers (at least one from code-area OWNERS file) agreed to review. Reviewers from multiple companies preferred:
    @mikedanese @awly

  • Approver (likely from SIG/area to which feature belongs):
    @tallclair

  • Initial target stage (alpha/beta/stable) and release (x.y):

    • alpha 1.7
    • beta 1.12
@ericchiang

This comment has been minimized.

Show comment
Hide comment
@ericchiang

ericchiang Apr 25, 2017

Member

I'm assuming this encompasses initial bootstrapping of the serving cert too?

Member

ericchiang commented Apr 25, 2017

I'm assuming this encompasses initial bootstrapping of the serving cert too?

@jcbsmpsn

This comment has been minimized.

Show comment
Hide comment
@jcbsmpsn

jcbsmpsn Apr 25, 2017

Member

Yes, initial bootstrapping of the server cert will be covered by this. Would you like a separate one, or just checking that feature wasn't lost in the shuffle?

Member

jcbsmpsn commented Apr 25, 2017

Yes, initial bootstrapping of the server cert will be covered by this. Would you like a separate one, or just checking that feature wasn't lost in the shuffle?

@ericchiang

This comment has been minimized.

Show comment
Hide comment
@ericchiang

ericchiang Apr 25, 2017

Member

Just checking, thanks.

Member

ericchiang commented Apr 25, 2017

Just checking, thanks.

@idvoretskyi

This comment has been minimized.

Show comment
Hide comment
@idvoretskyi

idvoretskyi May 3, 2017

Member

@jcbsmpsn please, provide us with the design proposal link

Member

idvoretskyi commented May 3, 2017

@jcbsmpsn please, provide us with the design proposal link

@idvoretskyi idvoretskyi added this to Action required in Kubernetes 1.7 features May 3, 2017

@jcbsmpsn

This comment has been minimized.

Show comment
Hide comment
@jcbsmpsn
Member

jcbsmpsn commented May 8, 2017

Design proposal: kubernetes/community#602

@idvoretskyi

This comment has been minimized.

Show comment
Hide comment
@idvoretskyi

idvoretskyi May 9, 2017

Member

@jcbsmpsn updated the feature description with the link, thanks.

Member

idvoretskyi commented May 9, 2017

@jcbsmpsn updated the feature description with the link, thanks.

@idvoretskyi idvoretskyi removed the help wanted label May 9, 2017

@idvoretskyi idvoretskyi moved this from Action required to In Progress in Kubernetes 1.7 features May 9, 2017

@jcbsmpsn jcbsmpsn changed the title from Server TLS Certificate Rotation to Kubelet Server TLS Certificate Rotation Jun 5, 2017

@ericchiang

This comment has been minimized.

Show comment
Hide comment
@ericchiang

ericchiang Jun 22, 2017

Member

Going to merge this into #266

Member

ericchiang commented Jun 22, 2017

Going to merge this into #266

@ericchiang ericchiang closed this Jun 22, 2017

@liggitt

This comment has been minimized.

Show comment
Hide comment
@liggitt

liggitt Jun 22, 2017

Member

Actually, I wouldn't. There's more work to do here with determining which SANs a node is allowed to serve.

Member

liggitt commented Jun 22, 2017

Actually, I wouldn't. There's more work to do here with determining which SANs a node is allowed to serve.

@liggitt liggitt reopened this Jun 22, 2017

@apsinha

This comment has been minimized.

Show comment
Hide comment
@apsinha

apsinha Jun 27, 2017

@jcbsmpsn
What is the link for the documentation update for certificate rotation? Is there no docs that need update?

apsinha commented Jun 27, 2017

@jcbsmpsn
What is the link for the documentation update for certificate rotation? Is there no docs that need update?

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017

Update TLS bootstrapping with 1.7 features
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certifiate
rotation.

ref:

- kubernetes/kubernetes#45030
- kubernetes/features#266
- kubernetes/features#267

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017

Update TLS bootstrapping with 1.7 features
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certifiate
rotation.

Since the CSR approver changed over the 1.7 release cycle we need
to call out the migration steps for those using the alpha feature.
This document as a whole could probably use some updates, but the
main focus of this PR is just to get these features minimally
documented before the release.

ref:

- kubernetes/kubernetes#45030
- kubernetes/features#266
- kubernetes/features#267
@jcbsmpsn

This comment has been minimized.

Show comment
Hide comment
@jcbsmpsn

jcbsmpsn Jun 27, 2017

Member

@apsinha Some documentation for this is included in kubernetes/website#4208

Member

jcbsmpsn commented Jun 27, 2017

@apsinha Some documentation for this is included in kubernetes/website#4208

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017

Update TLS bootstrapping with 1.7 features
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certificate
rotation.

Since the CSR approver changed over the 1.7 release cycle we need
to call out the migration steps for those using the alpha feature.
This document as a whole could probably use some updates, but the
main focus of this PR is just to get these features minimally
documented before the release.

ref:

- kubernetes/kubernetes#45030
- kubernetes/features#266
- kubernetes/features#267

ericchiang added a commit to ericchiang/website that referenced this issue Jun 27, 2017

Update TLS bootstrapping with 1.7 features
This includes documenting the new CSR approver built into the
controller manager and the kubelet alpha features for certificate
rotation.

Since the CSR approver changed over the 1.7 release cycle we need
to call out the migration steps for those using the alpha feature.
This document as a whole could probably use some updates, but the
main focus of this PR is just to get these features minimally
documented before the release.

ref:

- kubernetes/kubernetes#45030
- kubernetes/features#266
- kubernetes/features#267
@luxas

This comment has been minimized.

Show comment
Hide comment
@luxas

luxas Aug 14, 2017

Member

@jcbsmpsn Can you please update this feature's status for v1.8?
Is beta targeted or will this be still in alpha?

Member

luxas commented Aug 14, 2017

@jcbsmpsn Can you please update this feature's status for v1.8?
Is beta targeted or will this be still in alpha?

@idvoretskyi

This comment has been minimized.

Show comment
Hide comment
@idvoretskyi

idvoretskyi Sep 14, 2017

Member

@jcbsmpsn @kubernetes/sig-auth-feature-requests can you confirm that this feature targets 1.8?

If yes, please, update the features tracking spreadsheet with the feature data, otherwise, let's remove this item from 1.8 milestone.

Thanks

Member

idvoretskyi commented Sep 14, 2017

@jcbsmpsn @kubernetes/sig-auth-feature-requests can you confirm that this feature targets 1.8?

If yes, please, update the features tracking spreadsheet with the feature data, otherwise, let's remove this item from 1.8 milestone.

Thanks

@ericchiang

This comment has been minimized.

Show comment
Hide comment
@ericchiang

ericchiang Sep 14, 2017

Member

Bumping this to the 1.9 milestone since there wasn't much work done on it during 1.8.

Member

ericchiang commented Sep 14, 2017

Bumping this to the 1.9 milestone since there wasn't much work done on it during 1.8.

@ericchiang ericchiang modified the milestones: 1.8, 1.9 Sep 14, 2017

@ericchiang ericchiang removed the help wanted label Sep 14, 2017

@jcbsmpsn

This comment has been minimized.

Show comment
Hide comment
@jcbsmpsn

jcbsmpsn Sep 14, 2017

Member

Currently the kubelet provides the CN and the SANs that go into the certificate. Without some mechanism for the signing server to validate that information, it is possible for the kubelet to request a certificate for a domain it shouldn't and get the certificate (which would be a server certificate) blessed by the cluster CA.

This feature should remain alpha until there is an answer this problem.

@mikedanese

Member

jcbsmpsn commented Sep 14, 2017

Currently the kubelet provides the CN and the SANs that go into the certificate. Without some mechanism for the signing server to validate that information, it is possible for the kubelet to request a certificate for a domain it shouldn't and get the certificate (which would be a server certificate) blessed by the cluster CA.

This feature should remain alpha until there is an answer this problem.

@mikedanese

@luxas

This comment has been minimized.

Show comment
Hide comment
@luxas

luxas Oct 27, 2017

Member

@jcbsmpsn Is this on track shipping to beta in v1.9? Have we found a solution to the problem?

Member

luxas commented Oct 27, 2017

@jcbsmpsn Is this on track shipping to beta in v1.9? Have we found a solution to the problem?

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot Apr 3, 2018

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot commented Apr 3, 2018

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@justaugustus

This comment has been minimized.

Show comment
Hide comment
@justaugustus

justaugustus Apr 17, 2018

Member

@jcbsmpsn @kubernetes/sig-auth-feature-requests
Any plans for this in 1.11?

If so, can you please ensure the feature is up-to-date with the appropriate:

  • Description
  • Milestone
  • Assignee(s)
  • Labels:
    • stage/{alpha,beta,stable}
    • sig/*
    • kind/feature

cc @idvoretskyi

Member

justaugustus commented Apr 17, 2018

@jcbsmpsn @kubernetes/sig-auth-feature-requests
Any plans for this in 1.11?

If so, can you please ensure the feature is up-to-date with the appropriate:

  • Description
  • Milestone
  • Assignee(s)
  • Labels:
    • stage/{alpha,beta,stable}
    • sig/*
    • kind/feature

cc @idvoretskyi

@fejta-bot

This comment has been minimized.

Show comment
Hide comment
@fejta-bot

fejta-bot May 17, 2018

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

fejta-bot commented May 17, 2018

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
/remove-lifecycle stale

@smarterclayton

This comment has been minimized.

Show comment
Hide comment
@smarterclayton

smarterclayton May 29, 2018

Contributor
Contributor

smarterclayton commented May 29, 2018

k8s-merge-robot added a commit to kubernetes/kubernetes that referenced this issue Jul 12, 2018

Merge pull request #65594 from liggitt/node-csr-addresses-2
Automatic merge from submit-queue (batch tested with PRs 65052, 65594). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Derive kubelet serving certificate CSR template from node status addresses

xref kubernetes/features#267
fixes #55633

Builds on #65587

* Makes the cloud provider authoritative when recording node status addresses
* Makes the node status addresses authoritative for the kube-apiserver determining how to speak to a kubelet (stops paying attention to the hostname label when determining how to reach a kubelet, which was only done to support kubelets < 1.5)
* Updates kubelet certificate rotation to be driven from node status
  * Avoids needing to compute node addresses a second time, and differently, in order to request serving certificates.
  * Allows the kubelet to react to changes in its status addresses by updating its serving certificate
  * Allows the kubelet to be driven by external cloud providers recording node addresses on the node status

test procedure:
```sh
# setup
export FEATURE_GATES=RotateKubeletServerCertificate=true
export KUBELET_FLAGS="--rotate-server-certificates=true --cloud-provider=external"

# cleanup from previous runs
sudo rm -fr /var/lib/kubelet/pki/

# startup
hack/local-up-cluster.sh

# wait for a node to register, verify it didn't set addresses
kubectl get nodes 
kubectl get node/127.0.0.1 -o jsonpath={.status.addresses}

# verify the kubelet server isn't available, and that it didn't populate a serving certificate
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
ls -la /var/lib/kubelet/pki

# set an address on the node
curl -X PATCH http://localhost:8080/api/v1/nodes/127.0.0.1/status \
  -H "Content-Type: application/merge-patch+json" \
  --data '{"status":{"addresses":[{"type":"Hostname","address":"localhost"}]}}'

# verify a csr was submitted with the right SAN, and approve it
kubectl describe csr
kubectl certificate approve csr-...

# verify the kubelet connection uses a cert that is properly signed and valid for the specified hostname, but NOT the IP
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
curl --cacert _output/certs/server-ca.crt -v https://127.0.0.1:10250/pods
ls -la /var/lib/kubelet/pki

# set an hostname and IP address on the node
curl -X PATCH http://localhost:8080/api/v1/nodes/127.0.0.1/status \
  -H "Content-Type: application/merge-patch+json" \
  --data '{"status":{"addresses":[{"type":"Hostname","address":"localhost"},{"type":"InternalIP","address":"127.0.0.1"}]}}'

# verify a csr was submitted with the right SAN, and approve it
kubectl describe csr
kubectl certificate approve csr-...

# verify the kubelet connection uses a cert that is properly signed and valid for the specified hostname AND IP
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
curl --cacert _output/certs/server-ca.crt -v https://127.0.0.1:10250/pods
ls -la /var/lib/kubelet/pki
```

```release-note
* kubelets that specify `--cloud-provider` now only report addresses in Node status as determined by the cloud provider
* kubelet serving certificate rotation now reacts to changes in reported node addresses, and will request certificates for addresses set by an external cloud provider
```

k8s-publishing-bot added a commit to kubernetes/client-go that referenced this issue Jul 12, 2018

Merge pull request #65594 from liggitt/node-csr-addresses-2
Automatic merge from submit-queue (batch tested with PRs 65052, 65594). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Derive kubelet serving certificate CSR template from node status addresses

xref kubernetes/features#267
fixes #55633

Builds on kubernetes/kubernetes#65587

* Makes the cloud provider authoritative when recording node status addresses
* Makes the node status addresses authoritative for the kube-apiserver determining how to speak to a kubelet (stops paying attention to the hostname label when determining how to reach a kubelet, which was only done to support kubelets < 1.5)
* Updates kubelet certificate rotation to be driven from node status
  * Avoids needing to compute node addresses a second time, and differently, in order to request serving certificates.
  * Allows the kubelet to react to changes in its status addresses by updating its serving certificate
  * Allows the kubelet to be driven by external cloud providers recording node addresses on the node status

test procedure:
```sh
# setup
export FEATURE_GATES=RotateKubeletServerCertificate=true
export KUBELET_FLAGS="--rotate-server-certificates=true --cloud-provider=external"

# cleanup from previous runs
sudo rm -fr /var/lib/kubelet/pki/

# startup
hack/local-up-cluster.sh

# wait for a node to register, verify it didn't set addresses
kubectl get nodes
kubectl get node/127.0.0.1 -o jsonpath={.status.addresses}

# verify the kubelet server isn't available, and that it didn't populate a serving certificate
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
ls -la /var/lib/kubelet/pki

# set an address on the node
curl -X PATCH http://localhost:8080/api/v1/nodes/127.0.0.1/status \
  -H "Content-Type: application/merge-patch+json" \
  --data '{"status":{"addresses":[{"type":"Hostname","address":"localhost"}]}}'

# verify a csr was submitted with the right SAN, and approve it
kubectl describe csr
kubectl certificate approve csr-...

# verify the kubelet connection uses a cert that is properly signed and valid for the specified hostname, but NOT the IP
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
curl --cacert _output/certs/server-ca.crt -v https://127.0.0.1:10250/pods
ls -la /var/lib/kubelet/pki

# set an hostname and IP address on the node
curl -X PATCH http://localhost:8080/api/v1/nodes/127.0.0.1/status \
  -H "Content-Type: application/merge-patch+json" \
  --data '{"status":{"addresses":[{"type":"Hostname","address":"localhost"},{"type":"InternalIP","address":"127.0.0.1"}]}}'

# verify a csr was submitted with the right SAN, and approve it
kubectl describe csr
kubectl certificate approve csr-...

# verify the kubelet connection uses a cert that is properly signed and valid for the specified hostname AND IP
curl --cacert _output/certs/server-ca.crt -v https://localhost:10250/pods
curl --cacert _output/certs/server-ca.crt -v https://127.0.0.1:10250/pods
ls -la /var/lib/kubelet/pki
```

```release-note
* kubelets that specify `--cloud-provider` now only report addresses in Node status as determined by the cloud provider
* kubelet serving certificate rotation now reacts to changes in reported node addresses, and will request certificates for addresses set by an external cloud provider
```

Kubernetes-commit: 337dfe0a9cde3894eb6a26f9184df659d54007c6

@liggitt liggitt added stage/beta and removed stage/alpha labels Jul 25, 2018

@liggitt liggitt added this to the v1.12 milestone Jul 25, 2018

@liggitt liggitt self-assigned this Jul 25, 2018

@liggitt

This comment has been minimized.

Show comment
Hide comment
@liggitt

liggitt Jul 25, 2018

Member

@mikedanese I updated this to beta for 1.12, just for the rotation/kubelet bits, since the approval piece was moved out of scope

Member

liggitt commented Jul 25, 2018

@mikedanese I updated this to beta for 1.12, just for the rotation/kubelet bits, since the approval piece was moved out of scope

@justaugustus

This comment has been minimized.

Show comment
Hide comment
@justaugustus

justaugustus Jul 26, 2018

Member

Thanks for the update! This has been added to the 1.12 Tracking sheet.

Member

justaugustus commented Jul 26, 2018

Thanks for the update! This has been added to the 1.12 Tracking sheet.

k8s-merge-robot added a commit to kubernetes/kubernetes that referenced this issue Jul 28, 2018

Merge pull request #66726 from liggitt/kubelet-server
Automatic merge from submit-queue (batch tested with PRs 62444, 66358, 66724, 66726). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Move kubelet serving cert rotation to beta

xref kubernetes/features#267

This is exercised in the alpha gke e2es, and can be enabled in the non-alpha gke e2es once it no longer requires an alpha feature gate.

```release-note
Kubelet serving certificate bootstrapping and rotation has been promoted to beta status.
```
@zparnold

This comment has been minimized.

Show comment
Hide comment
@zparnold

zparnold Aug 20, 2018

Member

Hey there! @jcbsmpsn I'm the wrangler for the Docs this release. Is there any chance I could have you open up a docs PR against the release-1.12 branch as a placeholder? That gives us more confidence in the feature shipping in this release and gives me something to work with when we start doing reviews/edits. Thanks! If this feature does not require docs, could you please update the features tracking spreadsheet to reflect it?

Member

zparnold commented Aug 20, 2018

Hey there! @jcbsmpsn I'm the wrangler for the Docs this release. Is there any chance I could have you open up a docs PR against the release-1.12 branch as a placeholder? That gives us more confidence in the feature shipping in this release and gives me something to work with when we start doing reviews/edits. Thanks! If this feature does not require docs, could you please update the features tracking spreadsheet to reflect it?

@mikedanese

This comment has been minimized.

Show comment
Hide comment
@mikedanese

mikedanese Sep 7, 2018

Member

Beta docs PR here:

kubernetes/website#10232

Member

mikedanese commented Sep 7, 2018

Beta docs PR here:

kubernetes/website#10232

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment