From 99851b0f8441dad6635b754f1871d3ca58d0ee3c Mon Sep 17 00:00:00 2001 From: Brad Davidson Date: Mon, 4 Apr 2022 14:53:12 -0700 Subject: [PATCH] Use core constants for cert user/group values Also update cert gen to ensure leaf certs are regenerated if other key fields change. Signed-off-by: Brad Davidson --- pkg/daemons/control/deps/deps.go | 89 ++++++++++++++------------------ pkg/server/router.go | 5 +- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/pkg/daemons/control/deps/deps.go b/pkg/daemons/control/deps/deps.go index 230c2e97e7..86fcfa4bbe 100644 --- a/pkg/daemons/control/deps/deps.go +++ b/pkg/daemons/control/deps/deps.go @@ -28,7 +28,9 @@ import ( "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apiserver/pkg/apis/apiserver" apiserverconfigv1 "k8s.io/apiserver/pkg/apis/config/v1" + "k8s.io/apiserver/pkg/authentication/user" ) const ( @@ -314,7 +316,7 @@ func genClientCerts(config *config.Control) error { } apiEndpoint := fmt.Sprintf("https://%s:%d", ip, config.APIServerPort) - certGen, err = factory("system:admin", []string{"system:masters"}, runtime.ClientAdminCert, runtime.ClientAdminKey) + certGen, err = factory("system:admin", []string{user.SystemPrivilegedGroup}, runtime.ClientAdminCert, runtime.ClientAdminKey) if err != nil { return err } @@ -324,7 +326,7 @@ func genClientCerts(config *config.Control) error { } } - certGen, err = factory("system:kube-controller-manager", nil, runtime.ClientControllerCert, runtime.ClientControllerKey) + certGen, err = factory(user.KubeControllerManager, nil, runtime.ClientControllerCert, runtime.ClientControllerKey) if err != nil { return err } @@ -334,7 +336,7 @@ func genClientCerts(config *config.Control) error { } } - certGen, err = factory("system:kube-scheduler", nil, runtime.ClientSchedulerCert, runtime.ClientSchedulerKey) + certGen, err = factory(user.KubeScheduler, nil, runtime.ClientSchedulerCert, runtime.ClientSchedulerKey) if err != nil { return err } @@ -344,7 +346,7 @@ func genClientCerts(config *config.Control) error { } } - certGen, err = factory("kube-apiserver", nil, runtime.ClientKubeAPICert, runtime.ClientKubeAPIKey) + certGen, err = factory(user.APIServerUser, []string{user.SystemPrivilegedGroup}, runtime.ClientKubeAPICert, runtime.ClientKubeAPIKey) if err != nil { return err } @@ -354,7 +356,7 @@ func genClientCerts(config *config.Control) error { } } - if _, err = factory("system:kube-proxy", nil, runtime.ClientKubeProxyCert, runtime.ClientKubeProxyKey); err != nil { + if _, err = factory(user.KubeProxy, nil, runtime.ClientKubeProxyCert, runtime.ClientKubeProxyKey); err != nil { return err } // This user (system:k3s-controller by default) must be bound to a role in rolebindings.yaml or the downstream equivalent @@ -493,23 +495,22 @@ func addSANs(altNames *certutil.AltNames, sans []string) { } } -func sansChanged(certFile string, sans *certutil.AltNames) bool { +func fieldsChanged(certFile string, commonName string, organization []string, sans *certutil.AltNames, caCertFile string) bool { if sans == nil { - return false + sans = &certutil.AltNames{} } - certBytes, err := ioutil.ReadFile(certFile) - if err != nil { + certificates, err := certutil.CertsFromFile(certFile) + if err != nil || len(certificates) == 0 { return false } - certificates, err := certutil.ParseCertsPEM(certBytes) - if err != nil { - return false + if certificates[0].Subject.CommonName != commonName { + return true } - if len(certificates) == 0 { - return false + if !sets.NewString(certificates[0].Subject.Organization...).Equal(sets.NewString(organization...)) { + return true } if !sets.NewString(certificates[0].DNSNames...).HasAll(sans.DNSNames...) { @@ -527,44 +528,45 @@ func sansChanged(certFile string, sans *certutil.AltNames) bool { } } - return false -} - -func createClientCertKey(regen bool, commonName string, organization []string, altNames *certutil.AltNames, extKeyUsage []x509.ExtKeyUsage, caCertFile, caKeyFile, certFile, keyFile string) (bool, error) { - caBytes, err := ioutil.ReadFile(caCertFile) - if err != nil { - return false, err + caCertificates, err := certutil.CertsFromFile(caCertFile) + if err != nil || len(caCertificates) == 0 { + return false } - pool := x509.NewCertPool() - pool.AppendCertsFromPEM(caBytes) + verifyOpts := x509.VerifyOptions{ + Roots: x509.NewCertPool(), + KeyUsages: []x509.ExtKeyUsage{ + x509.ExtKeyUsageAny, + }, + } - // check for certificate expiration - if !regen { - regen = expired(certFile, pool) + for _, cert := range caCertificates { + verifyOpts.Roots.AddCert(cert) } - if !regen { - regen = sansChanged(certFile, altNames) + if _, err := certificates[0].Verify(verifyOpts); err != nil { + return true } + return false +} + +func createClientCertKey(regen bool, commonName string, organization []string, altNames *certutil.AltNames, extKeyUsage []x509.ExtKeyUsage, caCertFile, caKeyFile, certFile, keyFile string) (bool, error) { + // check for reasons to renew the certificate even if not manually requested. + regen = regen || expired(certFile) || fieldsChanged(certFile, commonName, organization, altNames, caCertFile) + if !regen { if exists(certFile, keyFile) { return false, nil } } - caKeyBytes, err := ioutil.ReadFile(caKeyFile) - if err != nil { - return false, err - } - - caKey, err := certutil.ParsePrivateKeyPEM(caKeyBytes) + caKey, err := certutil.PrivateKeyFromFile(caKeyFile) if err != nil { return false, err } - caCert, err := certutil.ParseCertsPEM(caBytes) + caCert, err := certutil.CertsFromFile(caCertFile) if err != nil { return false, err } @@ -648,24 +650,11 @@ func createSigningCertKey(prefix, certFile, keyFile string) (bool, error) { return true, nil } -func expired(certFile string, pool *x509.CertPool) bool { - certBytes, err := ioutil.ReadFile(certFile) +func expired(certFile string) bool { + certificates, err := certutil.CertsFromFile(certFile) if err != nil { return false } - certificates, err := certutil.ParseCertsPEM(certBytes) - if err != nil { - return false - } - _, err = certificates[0].Verify(x509.VerifyOptions{ - Roots: pool, - KeyUsages: []x509.ExtKeyUsage{ - x509.ExtKeyUsageAny, - }, - }) - if err != nil { - return true - } return certutil.IsCertExpired(certificates[0], config.CertificateRenewDays) } diff --git a/pkg/server/router.go b/pkg/server/router.go index ceac36ab04..f3da6c93eb 100644 --- a/pkg/server/router.go +++ b/pkg/server/router.go @@ -28,6 +28,7 @@ import ( "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/json" + "k8s.io/apiserver/pkg/authentication/user" ) const ( @@ -58,7 +59,7 @@ func router(ctx context.Context, config *Config, cfg *cmds.Server) http.Handler } nodeAuthed := mux.NewRouter() - nodeAuthed.Use(authMiddleware(serverConfig, "system:nodes")) + nodeAuthed.Use(authMiddleware(serverConfig, user.NodesGroup)) nodeAuthed.Path(prefix + "/connect").Handler(serverConfig.Runtime.Tunnel) nodeAuthed.NotFoundHandler = authed @@ -247,7 +248,7 @@ func clientKubeletCert(server *config.Control, keyFile string, auth nodePassBoot cert, err := certutil.NewSignedCert(certutil.Config{ CommonName: "system:node:" + nodeName, - Organization: []string{"system:nodes"}, + Organization: []string{user.NodesGroup}, Usages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, }, key, caCert[0], caKey) if err != nil {