Merge pull request #313 from justinsb/always_renew_certificates

Always renew certificates

pkg/pki/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
@@ -18,3 +18,9 @@ go_library(
         "//vendor/k8s.io/klog:go_default_library",
     ],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["parse_test.go"],
+    embed = [":go_default_library"],
+)

pkg/pki/certs.go

@@ -4,16 +4,71 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"fmt"
+	"os"
 	"reflect"
 	"sort"
+	"strconv"
+	"strings"
 	"time"
 
 	certutil "k8s.io/client-go/util/cert"
 	"k8s.io/klog"
 )
 
-// Renew certificates with less than 60 days left.
-const RENEW_THRESHOLD = 60 * 24 * time.Hour
+// CertDuration controls how long we issue certificates for.  We set
+// it to a longer time period, primarily because we don't have a nice
+// means of rotation.  This was historically one year, but we now set
+// it to two years, with kubernetes LTS proposing one year's support.
+var CertDuration = 2 * 365 * 24 * time.Hour
+
+// CertRenewalMinTimeLeft is the minimum amount of validity required on
+// a certificate to reuse it.  Becasue we set this (much) higher than
+// CertDuration, we will now always reissue certificates.
+var CertMinTimeLeft = 20 * 365 * 24 * time.Hour
+
+// ParseHumanDuration parses a go-style duration string, but
+// recognizes additional suffixes: d means "day" and is interpreted as
+// 24 hours; y means "year" and is interpreted as 365 days.
+func ParseHumanDuration(s string) (time.Duration, error) {
+	if strings.HasSuffix(s, "y") {
+		s = strings.TrimSuffix(s, "y")
+		n, err := strconv.Atoi(s)
+		if err != nil {
+			return time.Duration(0), err
+		}
+		return time.Duration(n) * 365 * 24 * time.Hour, nil
+	}
+
+	if strings.HasSuffix(s, "d") {
+		s = strings.TrimSuffix(s, "d")
+		n, err := strconv.Atoi(s)
+		if err != nil {
+			return time.Duration(0), err
+		}
+		return time.Duration(n) * 24 * time.Hour, nil
+	}
+
+	return time.ParseDuration(s)
+
+}
+
+func init() {
+	if s := os.Getenv("ETCD_MANAGER_CERT_DURATION"); s != "" {
+		v, err := ParseHumanDuration(s)
+		if err != nil {
+			klog.Fatalf("failed to parse ETCD_MANAGER_CERT_DURATION=%q", s)
+		}
+		CertDuration = v
+	}
+
+	if s := os.Getenv("ETCD_MANAGER_CERT_MIN_TIME_LEFT"); s != "" {
+		v, err := ParseHumanDuration(s)
+		if err != nil {
+			klog.Fatalf("failed to parse ETCD_MANAGER_CERT_MIN_TIME_LEFT=%q", s)
+		}
+		CertMinTimeLeft = v
+	}
+}
 
 type Keypair struct {
 	Certificate    *x509.Certificate
@@ -47,8 +102,8 @@ func EnsureKeypair(store MutableKeypair, config certutil.Config, signer *Keypair
 
 			match := true
 
-			if match && time.Until(cert.NotAfter) <= RENEW_THRESHOLD {
-				klog.Infof("certificate expired or almost expired, not valid after %s; will regenerate", cert.NotAfter.Format(time.RFC3339))
+			if match && time.Until(cert.NotAfter) <= CertMinTimeLeft {
+				klog.Infof("existing certificate not valid after %s; will regenerate", cert.NotAfter.Format(time.RFC3339))
 				match = false
 			}
 
@@ -113,7 +168,8 @@ func EnsureKeypair(store MutableKeypair, config certutil.Config, signer *Keypair
 			var cert *x509.Certificate
 			var err error
 			if signer != nil {
-				cert, err = NewSignedCert(&config, keypair.PrivateKey, signer.Certificate, signer.PrivateKey)
+				duration := CertDuration
+				cert, err = NewSignedCert(&config, keypair.PrivateKey, signer.Certificate, signer.PrivateKey, duration)
 			} else {
 				cert, err = certutil.NewSelfSignedCACert(config, keypair.PrivateKey)
 			}

pkg/pki/certutils.go

@@ -39,8 +39,6 @@ const (
 	RSAPrivateKeyBlockType = "RSA PRIVATE KEY"
 
 	rsaKeySize = 2048
-
-	duration365d = time.Hour * 24 * 365
 )
 
 // EncodeCertPEM returns PEM-endcoded certificate data
@@ -67,7 +65,7 @@ func NewPrivateKey() (*rsa.PrivateKey, error) {
 }
 
 // NewSignedCert creates a signed certificate using the given CA certificate and key
-func NewSignedCert(cfg *certutil.Config, key crypto.Signer, caCert *x509.Certificate, caKey crypto.Signer) (*x509.Certificate, error) {
+func NewSignedCert(cfg *certutil.Config, key crypto.Signer, caCert *x509.Certificate, caKey crypto.Signer, duration time.Duration) (*x509.Certificate, error) {
 	serial, err := cryptorand.Int(cryptorand.Reader, new(big.Int).SetInt64(math.MaxInt64))
 	if err != nil {
 		return nil, err
@@ -88,7 +86,7 @@ func NewSignedCert(cfg *certutil.Config, key crypto.Signer, caCert *x509.Certifi
 		IPAddresses:  cfg.AltNames.IPs,
 		SerialNumber: serial,
 		NotBefore:    caCert.NotBefore,
-		NotAfter:     time.Now().Add(duration365d).UTC(),
+		NotAfter:     time.Now().Add(duration).UTC(),
 		KeyUsage:     x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
 		ExtKeyUsage:  cfg.Usages,
 	}

pkg/pki/parse_test.go

@@ -0,0 +1,34 @@
+package pki
+
+import (
+	"testing"
+	"time"
+)
+
+func TestParseHumanDuration(t *testing.T) {
+	grid := []struct {
+		Value    string
+		Expected time.Duration
+	}{
+		{"10m", 10 * time.Minute},
+		{"24h", 24 * time.Hour},
+		{"1d", 24 * time.Hour},
+		{"10d", 10 * 24 * time.Hour},
+		{"365d", 365 * 24 * time.Hour},
+		{"1y", 365 * 24 * time.Hour},
+		{"10y", 10 * 365 * 24 * time.Hour},
+	}
+
+	for _, g := range grid {
+		g := g
+		t.Run(g.Value, func(t *testing.T) {
+			actual, err := ParseHumanDuration(g.Value)
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if actual != g.Expected {
+				t.Fatalf("unexpected value; expected=%v, actual=%v", g.Expected, actual)
+			}
+		})
+	}
+}

test/integration/upgradedowngrade_test.go

@@ -139,7 +139,7 @@ func TestUpgradeDowngrade(t *testing.T) {
 						t.Fatalf("error reading test key after downgrade: %v", err)
 					}
 					if v != "worldv3" {
-						t.Fatalf("unexpected test key value after upgrade: %q", v)
+						t.Fatalf("unexpected test key value after downgrade: %q", v)
 					}
 
 					n1.AssertVersion(t, fromVersion)