From fe3324cb8432518ed80c4d0155ff14e7801b60d7 Mon Sep 17 00:00:00 2001 From: Brad Davidson Date: Tue, 13 Aug 2024 20:38:22 +0000 Subject: [PATCH] Fix rotateca validation failures when not touching default self-signed CAs Also silences warnings about bootstrap fields that are not intended to be handled by CA rotation Signed-off-by: Brad Davidson --- pkg/bootstrap/bootstrap.go | 6 +++--- pkg/cluster/bootstrap.go | 2 +- pkg/daemons/config/types.go | 22 +++++++++---------- pkg/server/cert.go | 42 ++++++++++++++++++++++++++++++------- 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/pkg/bootstrap/bootstrap.go b/pkg/bootstrap/bootstrap.go index b3c1242afd7b..ebc63a275808 100644 --- a/pkg/bootstrap/bootstrap.go +++ b/pkg/bootstrap/bootstrap.go @@ -33,13 +33,13 @@ func ReadFromDisk(w io.Writer, bootstrap *config.ControlRuntimeBootstrap) error if path == "" { continue } - data, err := os.ReadFile(path) + info, err := os.Stat(path) if err != nil { - logrus.Warnf("failed to read %s", path) + logrus.Warnf("failed to stat %s: %v", pathKey, err) continue } - info, err := os.Stat(path) + data, err := os.ReadFile(path) if err != nil { return err } diff --git a/pkg/cluster/bootstrap.go b/pkg/cluster/bootstrap.go index 4b8edbb3bf2d..7dd224693ba4 100644 --- a/pkg/cluster/bootstrap.go +++ b/pkg/cluster/bootstrap.go @@ -30,7 +30,7 @@ import ( // Bootstrap attempts to load a managed database driver, if one has been initialized or should be created/joined. // It then checks to see if the cluster needs to load bootstrap data, and if so, loads data into the -// ControlRuntimeBoostrap struct, either via HTTP or from the datastore. +// ControlRuntimeBootstrap struct, either via HTTP or from the datastore. func (c *Cluster) Bootstrap(ctx context.Context, clusterReset bool) error { if err := c.assignManagedDriver(ctx); err != nil { return err diff --git a/pkg/daemons/config/types.go b/pkg/daemons/config/types.go index d0d0b0252f4e..342722646482 100644 --- a/pkg/daemons/config/types.go +++ b/pkg/daemons/config/types.go @@ -288,18 +288,18 @@ func (c *Control) Loopback(urlSafe bool) string { } type ControlRuntimeBootstrap struct { - ETCDServerCA string - ETCDServerCAKey string - ETCDPeerCA string - ETCDPeerCAKey string - ServerCA string - ServerCAKey string - ClientCA string - ClientCAKey string - ServiceKey string + ETCDServerCA string `rotate:"true"` + ETCDServerCAKey string `rotate:"true"` + ETCDPeerCA string `rotate:"true"` + ETCDPeerCAKey string `rotate:"true"` + ServerCA string `rotate:"true"` + ServerCAKey string `rotate:"true"` + ClientCA string `rotate:"true"` + ClientCAKey string `rotate:"true"` + ServiceKey string `rotate:"true"` PasswdFile string - RequestHeaderCA string - RequestHeaderCAKey string + RequestHeaderCA string `rotate:"true"` + RequestHeaderCAKey string `rotate:"true"` IPSECKey string EncryptionConfig string EncryptionHash string diff --git a/pkg/server/cert.go b/pkg/server/cert.go index 0fcdf0efe37c..bde425196ea3 100644 --- a/pkg/server/cert.go +++ b/pkg/server/cert.go @@ -49,7 +49,7 @@ func caCertReplaceHandler(server *config.Control) http.HandlerFunc { // the datastore. If the functions succeeds, servers should be restarted immediately to load the new certs // from the bootstrap data. func caCertReplace(server *config.Control, buf io.ReadCloser, force bool) error { - tmpdir, err := os.MkdirTemp("", "cacerts") + tmpdir, err := os.MkdirTemp(server.DataDir, ".rotate-ca-tmp-") if err != nil { return err } @@ -94,10 +94,19 @@ func validateBootstrap(oldServer, newServer *config.Control) error { // Use reflection to iterate over all of the bootstrap fields, checking files at each of the new paths. oldMeta := reflect.ValueOf(&oldServer.Runtime.ControlRuntimeBootstrap).Elem() newMeta := reflect.ValueOf(&newServer.Runtime.ControlRuntimeBootstrap).Elem() + fields := []reflect.StructField{} + for _, field := range reflect.VisibleFields(oldMeta.Type()) { - oldVal := oldMeta.FieldByName(field.Name) - newVal := newMeta.FieldByName(field.Name) + // Only handle bootstrap fields tagged for rotation + if field.Tag.Get("rotate") != "true" { + continue + } + fields = append(fields, field) + } + // first pass: use the existing file if the new file does not exist or is empty + for _, field := range fields { + newVal := newMeta.FieldByName(field.Name) info, err := os.Stat(newVal.String()) if err != nil && !errors.Is(err, fs.ErrNotExist) { errs = append(errs, errors.Wrap(err, field.Name)) @@ -106,20 +115,29 @@ func validateBootstrap(oldServer, newServer *config.Control) error { if info == nil || info.Size() == 0 { if newVal.CanSet() { - logrus.Infof("certificate: %s not provided; using current value", field.Name) + oldVal := oldMeta.FieldByName(field.Name) + logrus.Infof("certificate: %s not provided; using current value %s", field.Name, oldVal) newVal.Set(oldVal) } else { errs = append(errs, fmt.Errorf("cannot use current data for %s; field is not settable", field.Name)) } } + } + + // second pass: validate file contents + for _, field := range fields { + oldVal := oldMeta.FieldByName(field.Name) + newVal := newMeta.FieldByName(field.Name) + // Check CA chain consistency and cert/key agreement if strings.HasSuffix(field.Name, "CA") { if err := validateCA(oldVal.String(), newVal.String()); err != nil { errs = append(errs, errors.Wrap(err, field.Name)) } newKeyVal := newMeta.FieldByName(field.Name + "Key") - if err := validateCAKey(newVal.String(), newKeyVal.String()); err != nil { + oldKeyVal := oldMeta.FieldByName(field.Name + "Key") + if err := validateCAKey(oldVal.String(), oldKeyVal.String(), newVal.String(), newKeyVal.String()); err != nil { errs = append(errs, errors.Wrap(err, field.Name+"Key")) } } @@ -139,6 +157,11 @@ func validateBootstrap(oldServer, newServer *config.Control) error { } func validateCA(oldCAPath, newCAPath string) error { + // Skip validation if old values are being reused + if oldCAPath == newCAPath { + return nil + } + oldCerts, err := certutil.CertsFromFile(oldCAPath) if err != nil { return err @@ -150,7 +173,7 @@ func validateCA(oldCAPath, newCAPath string) error { } if len(newCerts) == 1 { - return errors.New("new CA is self-signed") + return errors.New("new CA bundle contains only a single certificate but should include root or intermediate CA certificates") } roots := x509.NewCertPool() @@ -183,7 +206,12 @@ func validateCA(oldCAPath, newCAPath string) error { } // validateCAKey confirms that the private key is valid for the certificate -func validateCAKey(newCAPath, newCAKeyPath string) error { +func validateCAKey(oldCAPath, oldCAKeyPath, newCAPath, newCAKeyPath string) error { + // Skip validation if old values are being reused + if oldCAPath == newCAPath && oldCAKeyPath == newCAKeyPath { + return nil + } + _, err := tls.LoadX509KeyPair(newCAPath, newCAKeyPath) if err != nil { err = errors.Wrap(err, "new CA cert and key cannot be loaded as X590KeyPair")