Merge pull request #12534 from sharifelgamal/renew-certs

renew minikube certs if expired

cmd/minikube/cmd/start_flags.go

@@ -122,6 +122,7 @@ const (
 	defaultSSHPort          = 22
 	listenAddress           = "listen-address"
 	extraDisks              = "extra-disks"
+	certExpiration          = "cert-expiration"
 )
 
 var (
@@ -169,6 +170,7 @@ func initMinikubeFlags() {
 	startCmd.Flags().StringVarP(&outputFormat, "output", "o", "text", "Format to print stdout in. Options include: [text,json]")
 	startCmd.Flags().StringP(trace, "", "", "Send trace events. Options include: [gcp]")
 	startCmd.Flags().Int(extraDisks, 0, "Number of extra disks created and attached to the minikube VM (currently only implemented for hyperkit and kvm2 drivers)")
+	startCmd.Flags().Duration(certExpiration, constants.DefaultCertExpiration, "Duration until minikube certificate expiration, defaults to three years (26280h).")
 }
 
 // initKubernetesFlags inits the commandline flags for Kubernetes related options
@@ -455,6 +457,7 @@ func generateNewConfigFromFlags(cmd *cobra.Command, k8sVersion string, drvName s
 		SSHKey:                  viper.GetString(sshSSHKey),
 		SSHPort:                 viper.GetInt(sshSSHPort),
 		ExtraDisks:              viper.GetInt(extraDisks),
+		CertExpiration:          viper.GetDuration(certExpiration),
 		KubernetesConfig: config.KubernetesConfig{
 			KubernetesVersion:      k8sVersion,
 			ClusterName:            ClusterFlagValue(),
@@ -580,6 +583,10 @@ func upgradeExistingConfig(cmd *cobra.Command, cc *config.ClusterConfig) {
 		cc.KubernetesConfig.NodePort = viper.GetInt(apiServerPort)
 	}
 
+	if cc.CertExpiration == 0 {
+		cc.CertExpiration = constants.DefaultCertExpiration
+	}
+
 }
 
 // updateExistingConfigFromFlags will update the existing config from the flags - used on a second start
@@ -652,6 +659,7 @@ func updateExistingConfigFromFlags(cmd *cobra.Command, existing *config.ClusterC
 	updateStringFromFlag(cmd, &cc.KubernetesConfig.ServiceCIDR, serviceCIDR)
 	updateBoolFromFlag(cmd, &cc.KubernetesConfig.ShouldLoadCachedImages, cacheImages)
 	updateIntFromFlag(cmd, &cc.KubernetesConfig.NodePort, apiServerPort)
+	updateDurationFromFlag(cmd, &cc.CertExpiration, certExpiration)
 
 	if cmd.Flags().Changed(kubernetesVersion) {
 		cc.KubernetesConfig.KubernetesVersion = getKubernetesVersion(existing)

pkg/minikube/bootstrapper/bootstrapper.go

@@ -44,7 +44,7 @@ type Bootstrapper interface {
 	GenerateToken(config.ClusterConfig) (string, error)
 	// LogCommands returns a map of log type to a command which will display that log.
 	LogCommands(config.ClusterConfig, LogOptions) map[string]string
-	SetupCerts(config.KubernetesConfig, config.Node) error
+	SetupCerts(config.ClusterConfig, config.Node) error
 	GetAPIServerStatus(string, int) (string, error)
 }
 

pkg/minikube/bootstrapper/certs.go

@@ -18,6 +18,7 @@ package bootstrapper
 
 import (
 	"crypto/sha1"
+	"crypto/x509"
 	"encoding/pem"
 	"fmt"
 	"io/ioutil"
@@ -28,6 +29,7 @@ import (
 	"path/filepath"
 	"sort"
 	"strings"
+	"time"
 
 	"github.com/otiai10/copy"
 	"github.com/pkg/errors"
@@ -42,21 +44,22 @@ import (
 	"k8s.io/minikube/pkg/minikube/constants"
 	"k8s.io/minikube/pkg/minikube/kubeconfig"
 	"k8s.io/minikube/pkg/minikube/localpath"
+	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/minikube/vmpath"
 	"k8s.io/minikube/pkg/util"
 )
 
 // SetupCerts gets the generated credentials required to talk to the APIServer.
-func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig, n config.Node) error {
-	localPath := localpath.Profile(k8s.ClusterName)
+func SetupCerts(cmd command.Runner, k8s config.ClusterConfig, n config.Node) error {
+	localPath := localpath.Profile(k8s.KubernetesConfig.ClusterName)
 	klog.Infof("Setting up %s for IP: %s\n", localPath, n.IP)
 
-	ccs, err := generateSharedCACerts()
+	ccs, regen, err := generateSharedCACerts()
 	if err != nil {
 		return errors.Wrap(err, "shared CA certs")
 	}
 
-	xfer, err := generateProfileCerts(k8s, n, ccs)
+	xfer, err := generateProfileCerts(k8s, n, ccs, regen)
 	if err != nil {
 		return errors.Wrap(err, "profile certs")
 	}
@@ -148,7 +151,8 @@ type CACerts struct {
 }
 
 // generateSharedCACerts generates CA certs shared among profiles, but only if missing
-func generateSharedCACerts() (CACerts, error) {
+func generateSharedCACerts() (CACerts, bool, error) {
+	regenProfileCerts := false
 	globalPath := localpath.MiniPath()
 	cc := CACerts{
 		caCert:    localpath.CACert(),
@@ -175,28 +179,30 @@ func generateSharedCACerts() (CACerts, error) {
 	}
 
 	for _, ca := range caCertSpecs {
-		if canRead(ca.certPath) && canRead(ca.keyPath) {
+		if isValid(ca.certPath, ca.keyPath) {
 			klog.Infof("skipping %s CA generation: %s", ca.subject, ca.keyPath)
 			continue
 		}
 
+		regenProfileCerts = true
 		klog.Infof("generating %s CA: %s", ca.subject, ca.keyPath)
 		if err := util.GenerateCACert(ca.certPath, ca.keyPath, ca.subject); err != nil {
-			return cc, errors.Wrap(err, "generate ca cert")
+			return cc, false, errors.Wrap(err, "generate ca cert")
 		}
 	}
 
-	return cc, nil
+	return cc, regenProfileCerts, nil
 }
 
 // generateProfileCerts generates profile certs for a profile
-func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACerts) ([]string, error) {
+func generateProfileCerts(cfg config.ClusterConfig, n config.Node, ccs CACerts, regen bool) ([]string, error) {
 
 	// Only generate these certs for the api server
 	if !n.ControlPlane {
 		return []string{}, nil
 	}
 
+	k8s := cfg.KubernetesConfig
 	profilePath := localpath.Profile(k8s.ClusterName)
 
 	serviceIP, err := util.GetServiceClusterIP(k8s.ServiceCIDR)
@@ -289,16 +295,23 @@ func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACert
 			kp = kp + "." + spec.hash
 		}
 
-		if canRead(cp) && canRead(kp) {
+		if !regen && isValid(cp, kp) {
 			klog.Infof("skipping %s signed cert generation: %s", spec.subject, kp)
 			continue
 		}
 
 		klog.Infof("generating %s signed cert: %s", spec.subject, kp)
+		if canRead(cp) {
+			os.Remove(cp)
+		}
+		if canRead(kp) {
+			os.Remove(kp)
+		}
 		err := util.GenerateSignedCert(
 			cp, kp, spec.subject,
 			spec.ips, spec.alternateNames,
 			spec.caCertPath, spec.caKeyPath,
+			cfg.CertExpiration,
 		)
 		if err != nil {
 			return xfer, errors.Wrapf(err, "generate signed cert for %q", spec.subject)
@@ -478,3 +491,45 @@ func canRead(path string) bool {
 	defer f.Close()
 	return true
 }
+
+// isValid checks a cert/key path and makes sure it's still valid
+// if a cert is expired or otherwise invalid, it will be deleted
+func isValid(certPath, keyPath string) bool {
+	if !canRead(keyPath) {
+		return false
+	}
+
+	certFile, err := os.ReadFile(certPath)
+	if err != nil {
+		klog.Infof("failed to read cert file %s: %v", certPath, err)
+		os.Remove(certPath)
+		os.Remove(keyPath)
+		return false
+	}
+
+	certData, _ := pem.Decode(certFile)
+	if certData == nil {
+		klog.Infof("failed to decode cert file %s", certPath)
+		os.Remove(certPath)
+		os.Remove(keyPath)
+		return false
+	}
+
+	cert, err := x509.ParseCertificate(certData.Bytes)
+	if err != nil {
+		klog.Infof("failed to parse cert file %s: %v\n", certPath, err)
+		os.Remove(certPath)
+		os.Remove(keyPath)
+		return false
+	}
+
+	if cert.NotAfter.Before(time.Now()) {
+		out.WarningT("Certificate {{.certPath}} has expired. Generating a new one...", out.V{"certPath": filepath.Base(certPath)})
+		klog.Infof("cert expired %s: expiration: %s, now: %s", certPath, cert.NotAfter, time.Now())
+		os.Remove(certPath)
+		os.Remove(keyPath)
+		return false
+	}
+
+	return true
+}

pkg/minikube/bootstrapper/certs_test.go

@@ -32,10 +32,13 @@ func TestSetupCerts(t *testing.T) {
 	tempDir := tests.MakeTempDir()
 	defer tests.RemoveTempDir(tempDir)
 
-	k8s := config.KubernetesConfig{
-		APIServerName: constants.APIServerName,
-		DNSDomain:     constants.ClusterDNSDomain,
-		ServiceCIDR:   constants.DefaultServiceCIDR,
+	k8s := config.ClusterConfig{
+		CertExpiration: constants.DefaultCertExpiration,
+		KubernetesConfig: config.KubernetesConfig{
+			APIServerName: constants.APIServerName,
+			DNSDomain:     constants.ClusterDNSDomain,
+			ServiceCIDR:   constants.DefaultServiceCIDR,
+		},
 	}
 
 	if err := os.Mkdir(filepath.Join(tempDir, "certs"), 0777); err != nil {

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -847,7 +847,7 @@ func (k *Bootstrapper) DeleteCluster(k8s config.KubernetesConfig) error {
 }
 
 // SetupCerts sets up certificates within the cluster.
-func (k *Bootstrapper) SetupCerts(k8s config.KubernetesConfig, n config.Node) error {
+func (k *Bootstrapper) SetupCerts(k8s config.ClusterConfig, n config.Node) error {
 	return bootstrapper.SetupCerts(k.c, k8s, n)
 }
 

pkg/minikube/config/types.go

@@ -84,6 +84,7 @@ type ClusterConfig struct {
 	Network                 string   // only used by docker driver
 	MultiNodeRequested      bool
 	ExtraDisks              int // currently only implemented for hyperkit and kvm2
+	CertExpiration          time.Duration
 }
 
 // KubernetesConfig contains the parameters used to configure the VM Kubernetes.

pkg/minikube/constants/constants.go

@@ -116,6 +116,9 @@ const (
 	TimeFormat = time.RFC1123
 	// MaxResources is the value that can be passed into the memory and cpus flags to specify to use maximum resources
 	MaxResources = "max"
+
+	// DefaultCertExpiration is the amount of time in the future a certificate will expire in by default, which is 3 years
+	DefaultCertExpiration = time.Hour * 24 * 365 * 3
 )
 
 var (

pkg/minikube/node/start.go

@@ -156,7 +156,7 @@ func Start(starter Starter, apiServer bool) (*kubeconfig.Settings, error) {
 			return nil, errors.Wrap(err, "Failed to get bootstrapper")
 		}
 
-		if err = bs.SetupCerts(starter.Cfg.KubernetesConfig, *starter.Node); err != nil {
+		if err = bs.SetupCerts(*starter.Cfg, *starter.Node); err != nil {
 			return nil, errors.Wrap(err, "setting up certs")
 		}
 
@@ -445,7 +445,7 @@ func setupKubeAdm(mAPI libmachine.API, cfg config.ClusterConfig, n config.Node,
 		exit.Error(reason.KubernetesInstallFailed, "Failed to update cluster", err)
 	}
 
-	if err := bs.SetupCerts(cfg.KubernetesConfig, n); err != nil {
+	if err := bs.SetupCerts(cfg, n); err != nil {
 		exit.Error(reason.GuestCert, "Failed to setup certs", err)
 	}
 

pkg/util/crypto.go

@@ -65,7 +65,7 @@ func GenerateCACert(certPath, keyPath string, name string) error {
 // Any parent directories of the certPath or keyPath will be created as needed with file mode 0755.
 
 // GenerateSignedCert generates a signed certificate and key
-func GenerateSignedCert(certPath, keyPath, cn string, ips []net.IP, alternateDNS []string, signerCertPath, signerKeyPath string) error {
+func GenerateSignedCert(certPath, keyPath, cn string, ips []net.IP, alternateDNS []string, signerCertPath, signerKeyPath string, expiration time.Duration) error {
 	klog.Infof("Generating cert %s with IP's: %s", certPath, ips)
 	signerCertBytes, err := ioutil.ReadFile(signerCertPath)
 	if err != nil {
@@ -99,7 +99,7 @@ func GenerateSignedCert(certPath, keyPath, cn string, ips []net.IP, alternateDNS
 			Organization: []string{"system:masters"},
 		},
 		NotBefore: time.Now().Add(time.Hour * -24),
-		NotAfter:  time.Now().Add(time.Hour * 24 * 365),
+		NotAfter:  time.Now().Add(expiration),
 
 		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
 		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth},

pkg/util/crypto_test.go

@@ -140,7 +140,7 @@ func TestGenerateSignedCert(t *testing.T) {
 		t.Run(test.description, func(t *testing.T) {
 			err := GenerateSignedCert(
 				certPath, keyPath, "minikube", ips, alternateDNS, test.signerCertPath,
-				test.signerKeyPath,
+				test.signerKeyPath, constants.DefaultCertExpiration,
 			)
 			if err != nil && !test.err {
 				t.Errorf("GenerateSignedCert() error = %v", err)

test/integration/cert_options_test.go

@@ -24,6 +24,7 @@ import (
 	"os/exec"
 	"strings"
 	"testing"
+	"time"
 )
 
 // TestCertOptions makes sure minikube certs respect the --apiserver-ips and --apiserver-names parameters
@@ -37,7 +38,6 @@ func TestCertOptions(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), Minutes(30))
 	defer CleanupWithLogs(t, profile, cancel)
 
-	// Use the most verbose logging for the simplest test. If it fails, something is very wrong.
 	args := append([]string{"start", "-p", profile, "--memory=2048", "--apiserver-ips=127.0.0.1", "--apiserver-ips=192.168.15.15", "--apiserver-names=localhost", "--apiserver-names=www.google.com", "--apiserver-port=8555"}, StartArgs()...)
 
 	// We can safely override --apiserver-name with
@@ -80,3 +80,32 @@ func TestCertOptions(t *testing.T) {
 	}
 
 }
+
+// TestCertExpiration makes sure minikube can start after its profile certs have expired.
+// It does this by configuring minikube certs to expire after 3 minutes, then waiting 3 minutes, then starting again.
+// It also makes sure minikube prints a cert expiration warning to the user.
+func TestCertExpiration(t *testing.T) {
+	MaybeParallel(t)
+
+	profile := UniqueProfileName("cert-expiration")
+	ctx, cancel := context.WithTimeout(context.Background(), Minutes(30))
+	defer CleanupWithLogs(t, profile, cancel)
+
+	args := append([]string{"start", "-p", profile, "--memory=2048", "--cert-expiration=3m"}, StartArgs()...)
+
+	rr, err := Run(t, exec.CommandContext(ctx, Target(), args...))
+	if err != nil {
+		t.Errorf("failed to start minikube with args: %q : %v", rr.Command(), err)
+	}
+
+	// Now wait 3 minutes for the certs to expire and make sure minikube starts properly
+	time.Sleep(time.Minute * 3)
+	args = append([]string{"start", "-p", profile, "--memory=2048", "--cert-expiration=8760h"}, StartArgs()...)
+	rr, err = Run(t, exec.CommandContext(ctx, Target(), args...))
+	if err != nil {
+		t.Errorf("failed to start minikube after cert expiration: %q : %v", rr.Command(), err)
+	}
+	if !strings.Contains(rr.Output(), "expired") {
+		t.Errorf("minikube start output did not warn about expired certs: %v", rr.Output())
+	}
+}