Check for OOB permission changes to PKI on disk every run.

If the permissions has changed, the consuming service may not be able
to access it- thus trigger a regeneration in that case so the permissions
get properly updated.

Note that this *can* lead to a war of certmgr trying to enforce permissions
it has been told to enforce, and an external SCM trying to change the permissions.

This is inherintly racy and wrong either way- and certmgr has no way to know
if the service actually was able to use the PKI- thus the only option is
to just force a regen.

cert/file.go

@@ -5,12 +5,14 @@ import (
 	"encoding/json"
 	"encoding/pem"
 	"errors"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/user"
 	"path"
 	"regexp"
 	"strconv"
+	"syscall"
 
 	log "github.com/sirupsen/logrus"
 )
@@ -126,25 +128,31 @@ func (f *File) parse() (err error) {
 	return nil
 }
 
-// Set ensures the file has the right owner/group and mode.
-func (f *File) setPermissions() error {
+// CheckPermissions checks that the permissions on disk match what we intend.  If the
+// file doesn't exist, then the raw error for stat is returned.
+func (f *File) CheckPermissions() error {
 	st, err := os.Stat(f.Path)
 	if err != nil {
 		return err
 	}
 
-	err = os.Chown(f.Path, f.uid, f.gid)
-	if err != nil {
-		return err
+	if st.Mode() != f.mode {
+		return fmt.Errorf("mode has changed from %s to %s", f.mode, st.Mode())
 	}
 
-	if st.Mode() != f.mode {
-		err = os.Chmod(f.Path, f.mode)
-		if err != nil {
-			return err
-		}
+	unixStat, ok := st.Sys().(*syscall.Stat_t)
+	if !ok {
+		// yes, this is noisy on windows.  Don't think we have windows users however...
+		log.Warningf("Certmgr doesn't know how to verify ownership for %s", f.Path)
+		return nil
 	}
 
+	if unixStat.Uid != uint32(f.uid) {
+		return fmt.Errorf("uid has changed from %d to %d", uint32(f.uid), unixStat.Uid)
+	}
+	if unixStat.Gid != uint32(f.gid) {
+		return fmt.Errorf("uid has changed from %d to %d", uint32(f.uid), unixStat.Gid)
+	}
 	return nil
 }
 

cert/spec.go

@@ -350,6 +350,11 @@ func (spec *Spec) checkDiskCertKey(ca *x509.Certificate) error {
 		log.Debugf("spec %s: cert failed to be read: %s", spec, err)
 		return err
 	}
+	err = spec.Cert.CheckPermissions()
+	if err != nil {
+		return errors.WithMessage(err, "cert requires regeneration due to permissions")
+	}
+
 	// update our internal time tracking while we're in here; even if immediately discard it,
 	// keeping it accurate to when we last saw it is desirable for metrics.
 	spec.updateCertExpiry(existingCert.NotAfter)
@@ -359,6 +364,12 @@ func (spec *Spec) checkDiskCertKey(ca *x509.Certificate) error {
 		log.Debugf("spec %s: key failed to be read: %s", spec, err)
 		return err
 	}
+
+	err = spec.Key.CheckPermissions()
+	if err != nil {
+		return errors.WithMessage(err, "key requires regeneration due to permissions")
+	}
+
 	err = verifyCertChain(ca, existingCert)
 	if err != nil {
 		log.Debugf("spec %s: CA has changed, cert is no longer valid via it: %s", spec, err)
@@ -414,7 +425,12 @@ func (spec *Spec) EnforcePKI(enableActions bool) error {
 				updateReason = "CA"
 			} else {
 				spec.updateCAExpiry(existingCA.NotAfter)
-				if !existingCA.Equal(currentCA) {
+
+				err = spec.CA.File.CheckPermissions()
+				if err != nil {
+					err = errors.WithMessage(err, "CA permissions have changed")
+					updateReason = "CA"
+				} else if !existingCA.Equal(currentCA) {
 					err = errors.New("on disk CA is no longer equal to new CA")
 					updateReason = "CA"
 				}