Add cert rotation command #4495

galal-hussein · 2021-11-15T18:46:02Z

Signed-off-by: galal-hussein [email protected]

Proposed Changes

Adding cert command to k3s plus adding subcommand rotate
Upgrading dynamic listener to add support for cert rotate

Types of Changes

New Feature

Verification

start k3s server
stop k3s
run cert rotate k3s cert rotate

This command will back up tls certs for server and agent excepts for CA certs and service key, and then remove these files allowing k3s on the next restart to regenerate the files.

restart k3s, and make sure that certificates are rotated successfully

Linked Issues

[epic] - Improved and Manual Cert Rotation #2636

User-Facing Change

A new command is added to k3s cert which supposed to contain subcommands for certificate management, this PR include the rotate subcommand which will rotate the TLS certs for server or agent.

pkg/cli/cert/cert.go

briandowns · 2021-11-15T22:26:36Z

pkg/cli/cert/cert.go

+
+	for _, cert := range certList {
+		if err := os.Remove(cert); err == nil {
+			logrus.Infof("Certificate %s is deleted", cert)


I wonder if we could replace "deleted" with "processed" or something lighter.

Oats87

A couple of nits; are we doing enough safe-guarding to prevent a user from accidentally rotating certs on a K3s cluster while K3s is still running? What happens if this command is run on a live cluster?

pkg/cli/cmds/certs.go

cmd/cert/main.go

Oats87 · 2021-11-15T22:39:13Z

pkg/cluster/https.go

+			const regenerateDynamicListenerFile = "dynamic-cert-regenerate"
+			dynamicListenerRegenFilePath := filepath.Join(c.config.DataDir, "tls", regenerateDynamicListenerFile)
+			if _, err := os.Stat(dynamicListenerRegenFilePath); err == nil {
+				os.Remove(dynamicListenerRegenFilePath)


What happens if the certificate rotation fails and we are prematurely removing this file?

These files are backed up before this operation begins. We output the path to the backup directory.

Signed-off-by: galal-hussein <[email protected]>

Signed-off-by: Brian Downs <[email protected]>

Signed-off-by: galal-hussein <[email protected]>

briandowns

LGTM

Signed-off-by: galal-hussein <[email protected]>

pkg/cli/cmds/certs.go

pkg/cli/cert/cert.go

Signed-off-by: galal-hussein <[email protected]>

galal-hussein · 2021-11-24T20:40:03Z

@brandond Added fixes according to your comments, and to add some context around the UX, here what the user will see if certificate rotate command is issued:

bash-5.1# k3s --debug certificate rotate
DEBU[0000] Asset dir /var/lib/rancher/k3s/data/96b8e62e8b0dffcc6ed3579dcf03f2a936bfb41aed5c6784ae817483a78eec56 
DEBU[0000] Running /var/lib/rancher/k3s/data/96b8e62e8b0dffcc6ed3579dcf03f2a936bfb41aed5c6784ae817483a78eec56/bin/k3s-certificate [../k3s/dist/artifacts/k3s --debug certificate rotate] 
INFO[0000] Server detected, rotatig server certificates 
INFO[0000] Rotating certificates for admin service      
INFO[0000] Rotating certificates for etcd service       
INFO[0000] Rotating certificates for api-server service 
INFO[0000] Rotating certificates for controller-manager service 
INFO[0000] Rotating certificates for cloud-controller service 
INFO[0000] Rotating certificates for scheduler service  
INFO[0000] Rotating certificates for k3s-server service 
INFO[0000] Rotating dynamic listener certificate        
INFO[0000] Rotating certificates for k3s-controller service 
INFO[0000] Rotating certificates for auth-proxy service 
INFO[0000] Rotating certificates for kubelet service    
INFO[0000] Rotating certificates for kube-proxy service 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-admin.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-admin.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/etcd/client.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/etcd/client.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/etcd/server-client.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/etcd/server-client.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/etcd/peer-server-client.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/etcd/peer-server-client.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-kube-apiserver.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-kube-apiserver.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/serving-kube-apiserver.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/serving-kube-apiserver.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-controller.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-controller.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-k3s-cloud-controller.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-k3s-cloud-controller.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-scheduler.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-scheduler.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-k3s-controller.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-k3s-controller.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/agent/client-k3s-controller.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/agent/client-k3s-controller.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-auth-proxy.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-auth-proxy.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-kubelet.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/serving-kubelet.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/agent/client-kubelet.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/agent/client-kubelet.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/agent/serving-kubelet.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/agent/serving-kubelet.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-kube-proxy.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/server/tls/client-kube-proxy.key is deleted 
DEBU[0000] file /var/lib/rancher/k3s/agent/client-kube-proxy.crt is deleted 
DEBU[0000] file /var/lib/rancher/k3s/agent/client-kube-proxy.key is deleted 
INFO[0000] Successfully backed up certificates for all services to path /var/lib/rancher/k3s/server/tls-1637786253, please restart k3s server or agent to rotate certificates

Without the debug mode the user will just see the services that will be rotated

pkg/cli/cert/cert.go

Signed-off-by: galal-hussein <[email protected]>

brandond · 2021-11-30T21:13:07Z

LGTM; are the test failures legit or a flake?

galal-hussein · 2021-11-30T21:21:32Z

LGTM; are the test failures legit or a flake?

I think flakes, I will restart

* Add cert rotation command Signed-off-by: galal-hussein <[email protected]> * add function to check for dynamic listener file Signed-off-by: Brian Downs <[email protected]> * Add dynamiclistener cert rotation support Signed-off-by: galal-hussein <[email protected]> * fixes to the cert rotation Signed-off-by: galal-hussein <[email protected]> * fix ci tests Signed-off-by: galal-hussein <[email protected]> * fixes to certificate rotation command Signed-off-by: galal-hussein <[email protected]> * more fixes Signed-off-by: galal-hussein <[email protected]> Co-authored-by: Brian Downs <[email protected]>

* Add cert rotation command (#4495) * Add cert rotation command Signed-off-by: galal-hussein <[email protected]> * add function to check for dynamic listener file Signed-off-by: Brian Downs <[email protected]> * Add dynamiclistener cert rotation support Signed-off-by: galal-hussein <[email protected]> * fixes to the cert rotation Signed-off-by: galal-hussein <[email protected]> * fix ci tests Signed-off-by: galal-hussein <[email protected]> * fixes to certificate rotation command Signed-off-by: galal-hussein <[email protected]> * more fixes Signed-off-by: galal-hussein <[email protected]> Co-authored-by: Brian Downs <[email protected]> * Upgrade dynamic listener Signed-off-by: galal-hussein <[email protected]> Co-authored-by: Brian Downs <[email protected]>

* Add cert rotation command (#4495) * Add cert rotation command Signed-off-by: galal-hussein <[email protected]> * add function to check for dynamic listener file Signed-off-by: Brian Downs <[email protected]> * Add dynamiclistener cert rotation support Signed-off-by: galal-hussein <[email protected]> * fixes to the cert rotation Signed-off-by: galal-hussein <[email protected]> * fix ci tests Signed-off-by: galal-hussein <[email protected]> * fixes to certificate rotation command Signed-off-by: galal-hussein <[email protected]> * more fixes Signed-off-by: galal-hussein <[email protected]> Co-authored-by: Brian Downs <[email protected]> * Upgrade dynamic listener Signed-off-by: galal-hussein <[email protected]> * go mod tidy Signed-off-by: galal-hussein <[email protected]> Co-authored-by: Brian Downs <[email protected]>

* Add cert rotation command Signed-off-by: galal-hussein <[email protected]> * add function to check for dynamic listener file Signed-off-by: Brian Downs <[email protected]> * Add dynamiclistener cert rotation support Signed-off-by: galal-hussein <[email protected]> * fixes to the cert rotation Signed-off-by: galal-hussein <[email protected]> * fix ci tests Signed-off-by: galal-hussein <[email protected]> * fixes to certificate rotation command Signed-off-by: galal-hussein <[email protected]> * more fixes Signed-off-by: galal-hussein <[email protected]> Co-authored-by: Brian Downs <[email protected]> Co-authored-by: Brian Downs <[email protected]>

VestigeJ · 2021-12-07T00:45:58Z

Validated cert rotation using the steps outlined roughly above.

Verification
start k3s server
stop k3s
k3s cert rotate
This command will back up tls certs for the server and will locate adjacent to the existing tls directory "/var/lib/rancher/k3s/server/tls-" except for the following eight files that get duplicated.

client-ca.crt are identical
client-ca.key are identical
dynamic-cert.json are identical
request-header-ca.crt are identical
request-header-ca.key are identical
server-ca.crt are identical
server-ca.key are identical
service.key are identical

restart k3s, and make sure that certificates are rotated successfully

k3s version v1.20.13+k3s-6d3c31ad (6d3c31ad)
go version go1.15.15

k3s version v1.21.7+k3s-7b629008 (7b629008)
go version go1.16.10

k3s version v1.22.4+k3s-2dc4e2c3 (2dc4e2c3)
go version go1.16.10

galal-hussein marked this pull request as ready for review November 15, 2021 22:19

galal-hussein requested a review from a team as a code owner November 15, 2021 22:19

briandowns reviewed Nov 15, 2021

View reviewed changes

pkg/cli/cert/cert.go Outdated Show resolved Hide resolved

briandowns reviewed Nov 15, 2021

View reviewed changes

pkg/cli/cert/cert.go Outdated Show resolved Hide resolved

briandowns reviewed Nov 15, 2021

View reviewed changes

Oats87 requested changes Nov 15, 2021

View reviewed changes

galal-hussein and others added 4 commits November 17, 2021 20:52

Add cert rotation command

5c6646c

Signed-off-by: galal-hussein <[email protected]>

add function to check for dynamic listener file

9b8e2a2

Signed-off-by: Brian Downs <[email protected]>

Add dynamiclistener cert rotation support

ce705f0

Signed-off-by: galal-hussein <[email protected]>

fixes to the cert rotation

ab6aede

Signed-off-by: galal-hussein <[email protected]>

galal-hussein force-pushed the cert_rotation_command branch from d8e1481 to ab6aede Compare November 17, 2021 19:01

briandowns approved these changes Nov 17, 2021

View reviewed changes

galal-hussein requested a review from Oats87 November 18, 2021 22:27

fix ci tests

ec12317

Signed-off-by: galal-hussein <[email protected]>

galal-hussein force-pushed the cert_rotation_command branch from 4cf57b4 to ec12317 Compare November 23, 2021 21:15

Oats87 approved these changes Nov 23, 2021

View reviewed changes

brandond requested changes Nov 23, 2021

View reviewed changes

pkg/cli/cmds/certs.go Outdated Show resolved Hide resolved

pkg/cli/cert/cert.go Show resolved Hide resolved

pkg/cli/cert/cert.go Outdated Show resolved Hide resolved

pkg/cli/cert/cert.go Outdated Show resolved Hide resolved

pkg/cli/cert/cert.go Outdated Show resolved Hide resolved

brandond reviewed Nov 23, 2021

View reviewed changes

pkg/cli/cert/cert.go Show resolved Hide resolved

fixes to certificate rotation command

a0c4aa1

Signed-off-by: galal-hussein <[email protected]>

galal-hussein requested review from brandond, briandowns and Oats87 November 24, 2021 20:40

briandowns approved these changes Nov 24, 2021

View reviewed changes

brandond requested changes Nov 24, 2021

View reviewed changes

pkg/cli/cert/cert.go Outdated Show resolved Hide resolved

pkg/cli/cert/cert.go Outdated Show resolved Hide resolved

pkg/cli/cert/cert.go Outdated Show resolved Hide resolved

more fixes

71b6bdb

Signed-off-by: galal-hussein <[email protected]>

galal-hussein requested a review from brandond November 29, 2021 17:24

brandond approved these changes Nov 30, 2021

View reviewed changes

galal-hussein merged commit 77fd3e9 into k3s-io:master Dec 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cert rotation command #4495

Add cert rotation command #4495

galal-hussein commented Nov 15, 2021 •

edited

Loading

briandowns Nov 15, 2021

Oats87 left a comment

Oats87 Nov 15, 2021

briandowns Nov 17, 2021

briandowns left a comment

galal-hussein commented Nov 24, 2021

brandond commented Nov 30, 2021

galal-hussein commented Nov 30, 2021

VestigeJ commented Dec 7, 2021 •

edited

Loading

Add cert rotation command #4495

Add cert rotation command #4495

Conversation

galal-hussein commented Nov 15, 2021 • edited Loading

Proposed Changes

Types of Changes

Verification

Linked Issues

User-Facing Change

briandowns Nov 15, 2021

Choose a reason for hiding this comment

Oats87 left a comment

Choose a reason for hiding this comment

Oats87 Nov 15, 2021

Choose a reason for hiding this comment

briandowns Nov 17, 2021

Choose a reason for hiding this comment

briandowns left a comment

Choose a reason for hiding this comment

galal-hussein commented Nov 24, 2021

brandond commented Nov 30, 2021

galal-hussein commented Nov 30, 2021

VestigeJ commented Dec 7, 2021 • edited Loading

galal-hussein commented Nov 15, 2021 •

edited

Loading

VestigeJ commented Dec 7, 2021 •

edited

Loading