From e7fbd6f18e7ae7d1b440b430b029c2384282f345 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 (cherry picked from commit 99851b0f8441dad6635b754f1871d3ca58d0ee3c) --- pkg/daemons/control/deps/deps.go | 96 ++++++++++++++------------------ pkg/server/router.go | 5 +- 2 files changed, 45 insertions(+), 56 deletions(-) diff --git a/pkg/daemons/control/deps/deps.go b/pkg/daemons/control/deps/deps.go index 145e741ed9..c55eb9a726 100644 --- a/pkg/daemons/control/deps/deps.go +++ b/pkg/daemons/control/deps/deps.go @@ -28,6 +28,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" apiserverconfigv1 "k8s.io/apiserver/pkg/apis/config/v1" + "k8s.io/apiserver/pkg/authentication/user" ) const ( @@ -311,7 +312,7 @@ func genClientCerts(config *config.Control) error { var certGen bool apiEndpoint := fmt.Sprintf("https://127.0.0.1:%d", 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 } @@ -321,7 +322,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 } @@ -331,7 +332,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 } @@ -341,7 +342,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 } @@ -351,7 +352,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 @@ -490,23 +491,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 { + sans = &certutil.AltNames{} + } + + certificates, err := certutil.CertsFromFile(certFile) + if err != nil || len(certificates) == 0 { return false } - certBytes, err := ioutil.ReadFile(certFile) - if err != nil { - return false + if certificates[0].Subject.CommonName != commonName { + return true } - certificates, err := certutil.ParseCertsPEM(certBytes) - if err != nil { - return false - } - - 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...) { @@ -524,26 +524,32 @@ func sansChanged(certFile string, sans *certutil.AltNames) bool { } } + caCertificates, err := certutil.CertsFromFile(caCertFile) + if err != nil || len(caCertificates) == 0 { + return false + } + + verifyOpts := x509.VerifyOptions{ + Roots: x509.NewCertPool(), + KeyUsages: []x509.ExtKeyUsage{ + x509.ExtKeyUsageAny, + }, + } + + for _, cert := range caCertificates { + verifyOpts.Roots.AddCert(cert) + } + + 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) { - caBytes, err := ioutil.ReadFile(caCertFile) - if err != nil { - return false, err - } - - pool := x509.NewCertPool() - pool.AppendCertsFromPEM(caBytes) - - // check for certificate expiration - if !regen { - regen = expired(certFile, pool) - } - - if !regen { - regen = sansChanged(certFile, altNames) - } + // 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) { @@ -551,17 +557,12 @@ func createClientCertKey(regen bool, commonName string, organization []string, a } } - caKeyBytes, err := ioutil.ReadFile(caKeyFile) + caKey, err := certutil.PrivateKeyFromFile(caKeyFile) if err != nil { return false, err } - caKey, err := certutil.ParsePrivateKeyPEM(caKeyBytes) - if err != nil { - return false, err - } - - caCert, err := certutil.ParseCertsPEM(caBytes) + caCert, err := certutil.CertsFromFile(caCertFile) if err != nil { return false, err } @@ -645,24 +646,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 78a00ba70a..c1848a60f6 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 {