Skip to content
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

Fix rotateca validation failures when not touching default self-signed CAs #10710

Merged
merged 1 commit into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cluster/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 11 additions & 11 deletions pkg/daemons/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 35 additions & 7 deletions pkg/server/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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))
Expand All @@ -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"))
}
}
Expand All @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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")
Expand Down
Loading