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

kubeadm: print warnings on invalid cert period instead of erroring out
This commit is contained in:
Kubernetes Prow Robot 2020-09-29 02:49:25 -07:00 committed by GitHub
commit 604569482f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 74 additions and 11 deletions

View File

@ -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)
}

View File

@ -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)

View File

@ -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)
}
}

View File

@ -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",

View File

@ -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)

View File

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