Merge pull request kubernetes#94504 from neolit123/1.20-warning-cert-bounds-client-side

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

Author: k8s-ci-robot

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 {
@@ -365,6 +378,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 {
@@ -395,6 +410,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)
 }
@@ -406,6 +423,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 {
@@ -439,3 +458,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"
@@ -140,6 +141,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
@@ -282,6 +286,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 {
@@ -309,6 +315,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 {
@@ -354,6 +362,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)
 
 	// validate user provided kubeconfig files for the scheduler and controller-manager
 	localAPIEndpoint, err := kubeadmutil.GetLocalAPIEndpoint(&cfg.LocalAPIEndpoint)

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
+}