kubeadm: print warnings on invalid cert period instead of erroring out

Client side period validation of certificates should not be
fatal, as local clock skews are not so uncommon. The validation
should be left to the running servers.

- Remove this validation from TryLoadCertFromDisk().
- Add a new function ValidateCertPeriod(), that can be used for this
purpose on demand.
- In phases/certs add a new function CheckCertificatePeriodValidity()
that will print warnings if a certificate does not pass period
validation, and caches certificates that were already checked.
- Use the function in a number of places where certificates
are loaded from disk.

Author: neolit123

cmd/kubeadm/app/cmd/phases/init/certs.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
+
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme"
 	kubeadmapiv1beta2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2"
@@ -228,7 +229,9 @@ func runCAPhase(ca *certsphase.KubeadmCert) func(c workflow.RunData) error {
 			return nil
 		}
 
-		if _, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), ca.BaseName); err == nil {
+		if cert, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), ca.BaseName); err == nil {
+			certsphase.CheckCertificatePeriodValidity(ca.BaseName, cert)
+
 			if _, err := pkiutil.TryLoadKeyFromDisk(data.CertificateDir(), ca.BaseName); err == nil {
 				fmt.Printf("[certs] Using existing %s certificate authority\n", ca.BaseName)
 				return nil
@@ -261,11 +264,15 @@ func runCertPhase(cert *certsphase.KubeadmCert, caCert *certsphase.KubeadmCert)
 		}
 
 		if certData, _, err := pkiutil.TryLoadCertAndKeyFromDisk(data.CertificateDir(), cert.BaseName); err == nil {
+			certsphase.CheckCertificatePeriodValidity(cert.BaseName, certData)
+
 			caCertData, err := pkiutil.TryLoadCertFromDisk(data.CertificateDir(), caCert.BaseName)
 			if err != nil {
 				return errors.Wrapf(err, "couldn't load CA certificate %s", caCert.Name)
 			}
 
+			certsphase.CheckCertificatePeriodValidity(caCert.BaseName, caCertData)
+
 			if err := certData.CheckSignatureFrom(caCertData); err != nil {
 				return errors.Wrapf(err, "[certs] certificate %s not signed by CA certificate %s", cert.BaseName, caCert.BaseName)
 			}

cmd/kubeadm/app/phases/certs/certlist.go

@@ -124,6 +124,9 @@ func (t CertificateTree) CreateTree(ic *kubeadmapi.InitConfiguration) error {
 
 		caCert, err := pkiutil.TryLoadCertFromDisk(ic.CertificatesDir, ca.BaseName)
 		if err == nil {
+			// Validate period
+			CheckCertificatePeriodValidity(ca.BaseName, caCert)
+
 			// Cert exists already, make sure it's valid
 			if !caCert.IsCA {
 				return errors.Errorf("certificate %q is not a CA", ca.Name)

cmd/kubeadm/app/phases/certs/certs.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sync"
 
 	"github.com/pkg/errors"
 	"k8s.io/client-go/util/keyutil"
@@ -32,6 +33,12 @@ import (
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 )
 
+var (
+	// certPeriodValidation is used to store if period validation was done for a certificate
+	certPeriodValidationMutex sync.Mutex
+	certPeriodValidation      = map[string]struct{}{}
+)
+
 // CreatePKIAssets will create and write to disk all PKI assets necessary to establish the control plane.
 // If the PKI assets already exists in the target folder, they are used only if evaluated equal; otherwise an error is returned.
 func CreatePKIAssets(cfg *kubeadmapi.InitConfiguration) error {
@@ -166,6 +173,8 @@ func LoadCertificateAuthority(pkiDir string, baseName string) (*x509.Certificate
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, "failure loading %s certificate authority", baseName)
 	}
+	// Validate period
+	CheckCertificatePeriodValidity(baseName, caCert)
 
 	// Make sure the loaded CA cert actually is a CA
 	if !caCert.IsCA {
@@ -189,6 +198,8 @@ func writeCertificateAuthorityFilesIfNotExist(pkiDir string, baseName string, ca
 		if err != nil {
 			return errors.Wrapf(err, "failure loading %s certificate", baseName)
 		}
+		// Validate period
+		CheckCertificatePeriodValidity(baseName, caCert)
 
 		// Check if the existing cert is a CA
 		if !caCert.IsCA {
@@ -223,6 +234,8 @@ func writeCertificateFilesIfNotExist(pkiDir string, baseName string, signingCert
 		if err != nil {
 			return errors.Wrapf(err, "failure loading %s certificate", baseName)
 		}
+		// Validate period
+		CheckCertificatePeriodValidity(baseName, signedCert)
 
 		// Check if the existing cert is signed by the given CA
 		if err := signedCert.CheckSignatureFrom(signingCert); err != nil {
@@ -364,6 +377,8 @@ func validateCACert(l certKeyLocation) error {
 	if err != nil {
 		return errors.Wrapf(err, "failure loading certificate for %s", l.uxName)
 	}
+	// Validate period
+	CheckCertificatePeriodValidity(l.uxName, caCert)
 
 	// Check if cert is a CA
 	if !caCert.IsCA {
@@ -394,6 +409,8 @@ func validateSignedCert(l certKeyLocation) error {
 	if err != nil {
 		return errors.Wrapf(err, "failure loading certificate authority for %s", l.uxName)
 	}
+	// Validate period
+	CheckCertificatePeriodValidity(l.uxName, caCert)
 
 	return validateSignedCertWithCA(l, caCert)
 }
@@ -405,6 +422,8 @@ func validateSignedCertWithCA(l certKeyLocation, caCert *x509.Certificate) error
 	if err != nil {
 		return errors.Wrapf(err, "failure loading certificate for %s", l.uxName)
 	}
+	// Validate period
+	CheckCertificatePeriodValidity(l.uxName, signedCert)
 
 	// Check if the cert is signed by the CA
 	if err := signedCert.CheckSignatureFrom(caCert); err != nil {
@@ -438,3 +457,21 @@ func validateCertificateWithConfig(cert *x509.Certificate, baseName string, cfg
 	}
 	return nil
 }
+
+// CheckCertificatePeriodValidity takes a certificate and prints a warning if its period
+// is not valid related to the current time. It does so only if the certificate was not validated already
+// by keeping track with a cache.
+func CheckCertificatePeriodValidity(baseName string, cert *x509.Certificate) {
+	certPeriodValidationMutex.Lock()
+	if _, exists := certPeriodValidation[baseName]; exists {
+		certPeriodValidationMutex.Unlock()
+		return
+	}
+	certPeriodValidation[baseName] = struct{}{}
+	certPeriodValidationMutex.Unlock()
+
+	klog.V(5).Infof("validating certificate period for %s certificate", baseName)
+	if err := pkiutil.ValidateCertPeriod(cert, 0); err != nil {
+		klog.Warningf("WARNING: could not validate bounds for certificate %s: %v", baseName, err)
+	}
+}

cmd/kubeadm/app/phases/kubeconfig/BUILD

@@ -16,6 +16,7 @@ go_library(
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
+        "//cmd/kubeadm/app/phases/certs:go_default_library",
         "//cmd/kubeadm/app/util:go_default_library",
         "//cmd/kubeadm/app/util/kubeconfig:go_default_library",
         "//cmd/kubeadm/app/util/pkiutil:go_default_library",

cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/klog/v2"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
+	certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
 	pkiutil "k8s.io/kubernetes/cmd/kubeadm/app/util/pkiutil"
@@ -124,6 +125,9 @@ func getKubeConfigSpecs(cfg *kubeadmapi.InitConfiguration) (map[string]*kubeConf
 	if err != nil {
 		return nil, errors.Wrap(err, "couldn't create a kubeconfig; the CA files couldn't be loaded")
 	}
+	// Validate period
+	certsphase.CheckCertificatePeriodValidity(kubeadmconstants.CACertAndKeyBaseName, caCert)
+
 	configs, err := getKubeConfigSpecsBase(cfg)
 	if err != nil {
 		return nil, err
@@ -265,6 +269,8 @@ func WriteKubeConfigWithClientCert(out io.Writer, cfg *kubeadmapi.InitConfigurat
 	if err != nil {
 		return errors.Wrap(err, "couldn't create a kubeconfig; the CA files couldn't be loaded")
 	}
+	// Validate period
+	certsphase.CheckCertificatePeriodValidity(kubeadmconstants.CACertAndKeyBaseName, caCert)
 
 	controlPlaneEndpoint, err := kubeadmutil.GetControlPlaneEndpoint(cfg.ControlPlaneEndpoint, &cfg.LocalAPIEndpoint)
 	if err != nil {
@@ -292,6 +298,8 @@ func WriteKubeConfigWithToken(out io.Writer, cfg *kubeadmapi.InitConfiguration,
 	if err != nil {
 		return errors.Wrap(err, "couldn't create a kubeconfig; the CA files couldn't be loaded")
 	}
+	// Validate period
+	certsphase.CheckCertificatePeriodValidity(kubeadmconstants.CACertAndKeyBaseName, caCert)
 
 	controlPlaneEndpoint, err := kubeadmutil.GetControlPlaneEndpoint(cfg.ControlPlaneEndpoint, &cfg.LocalAPIEndpoint)
 	if err != nil {
@@ -344,6 +352,8 @@ func ValidateKubeconfigsForExternalCA(outDir string, cfg *kubeadmapi.InitConfigu
 	if err != nil {
 		return errors.Wrapf(err, "the CA file couldn't be loaded")
 	}
+	// Validate period
+	certsphase.CheckCertificatePeriodValidity(kubeadmconstants.CACertAndKeyBaseName, caCert)
 
 	controlPlaneEndpoint, err := kubeadmutil.GetControlPlaneEndpoint(cfg.ControlPlaneEndpoint, &cfg.LocalAPIEndpoint)
 	if err != nil {

cmd/kubeadm/app/util/pkiutil/pki_helpers.go

@@ -240,7 +240,7 @@ func TryLoadCertAndKeyFromDisk(pkiPath, name string) (*x509.Certificate, crypto.
 	return cert, key, nil
 }
 
-// TryLoadCertFromDisk tries to load the cert from the disk and validates that it is valid
+// TryLoadCertFromDisk tries to load the cert from the disk
 func TryLoadCertFromDisk(pkiPath, name string) (*x509.Certificate, error) {
 	certificatePath := pathForCert(pkiPath, name)
 
@@ -253,15 +253,6 @@ func TryLoadCertFromDisk(pkiPath, name string) (*x509.Certificate, error) {
 	// TODO: Support multiple certs here in order to be able to rotate certs
 	cert := certs[0]
 
-	// Check so that the certificate is valid now
-	now := time.Now()
-	if now.Before(cert.NotBefore) {
-		return nil, errors.New("the certificate is not valid yet")
-	}
-	if now.After(cert.NotAfter) {
-		return nil, errors.New("the certificate has expired")
-	}
-
 	return cert, nil
 }
 
@@ -619,3 +610,17 @@ func RemoveDuplicateAltNames(altNames *certutil.AltNames) {
 	}
 	altNames.IPs = ips
 }
+
+// ValidateCertPeriod checks if the certificate is valid relative to the current time
+// (+/- offset)
+func ValidateCertPeriod(cert *x509.Certificate, offset time.Duration) error {
+	period := fmt.Sprintf("NotBefore: %v, NotAfter: %v", cert.NotBefore, cert.NotAfter)
+	now := time.Now().Add(offset)
+	if now.Before(cert.NotBefore) {
+		return errors.Errorf("the certificate is not valid yet: %s", period)
+	}
+	if now.After(cert.NotAfter) {
+		return errors.Errorf("the certificate has expired: %s", period)
+	}
+	return nil
+}