csr signer has no need to sign certificates for a duration longer than the signer itself

pkg/controller/certificates/signer/cfssl_signer.go

@@ -59,6 +59,9 @@ type cfsslSigner struct {
 	sigAlgo             x509.SignatureAlgorithm
 	client              clientset.Interface
 	certificateDuration time.Duration
+
+	// nowFn returns the current time.  We have here for unit testing
+	nowFn func() time.Time
 }
 
 func newCFSSLSigner(caFile, caKeyFile string, client clientset.Interface, certificateDuration time.Duration) (*cfsslSigner, error) {
@@ -92,6 +95,7 @@ func newCFSSLSigner(caFile, caKeyFile string, client clientset.Interface, certif
 		sigAlgo:             signer.DefaultSigAlgo(priv),
 		client:              client,
 		certificateDuration: certificateDuration,
+		nowFn:               time.Now,
 	}, nil
 }
 
@@ -115,11 +119,21 @@ func (s *cfsslSigner) sign(csr *capi.CertificateSigningRequest) (*capi.Certifica
 	for _, usage := range csr.Spec.Usages {
 		usages = append(usages, string(usage))
 	}
+
+	certExpiryDuration := s.certificateDuration
+	durationUntilExpiry := s.ca.NotAfter.Sub(s.nowFn())
+	if durationUntilExpiry <= 0 {
+		return nil, fmt.Errorf("the signer has expired: %v", s.ca.NotAfter)
+	}
+	if durationUntilExpiry < certExpiryDuration {
+		certExpiryDuration = durationUntilExpiry
+	}
+
 	policy := &config.Signing{
 		Default: &config.SigningProfile{
 			Usage:        usages,
-			Expiry:       s.certificateDuration,
+			Expiry:       certExpiryDuration,
-			ExpiryString: s.certificateDuration.String(),
+			ExpiryString: certExpiryDuration.String(),
 		},
 	}
 	cfs, err := local.NewSigner(s.priv, s.ca, s.sigAlgo, policy)

pkg/controller/certificates/signer/cfssl_signer_test.go

@@ -20,6 +20,7 @@ import (
 	"crypto/x509"
 	"io/ioutil"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -28,10 +29,16 @@ import (
 )
 
 func TestSigner(t *testing.T) {
+	testNow := time.Now()
+	testNowFn := func() time.Time {
+		return testNow
+	}
+
 	s, err := newCFSSLSigner("./testdata/ca.crt", "./testdata/ca.key", nil, 1*time.Hour)
 	if err != nil {
 		t.Fatalf("failed to create signer: %v", err)
 	}
+	s.nowFn = testNowFn
 
 	csrb, err := ioutil.ReadFile("./testdata/kubelet.csr")
 	if err != nil {
@@ -81,4 +88,108 @@ func TestSigner(t *testing.T) {
 	if !reflect.DeepEqual(crt.ExtKeyUsage, []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}) {
 		t.Errorf("bad extended key usage")
 	}
+
+	expectedTime := testNow.Add(1 * time.Hour)
+	// there is some jitter that we need to tolerate
+	diff := expectedTime.Sub(crt.NotAfter)
+	if diff > 10*time.Minute || diff < -10*time.Minute {
+		t.Fatal(crt.NotAfter)
+	}
+}
+
+func TestSignerExpired(t *testing.T) {
+	hundredYearsFromNowFn := func() time.Time {
+		return time.Now().Add(24 * time.Hour * 365 * 100)
+	}
+	s, err := newCFSSLSigner("./testdata/ca.crt", "./testdata/ca.key", nil, 1*time.Hour)
+	if err != nil {
+		t.Fatalf("failed to create signer: %v", err)
+	}
+	s.nowFn = hundredYearsFromNowFn
+
+	csrb, err := ioutil.ReadFile("./testdata/kubelet.csr")
+	if err != nil {
+		t.Fatalf("failed to read CSR: %v", err)
+	}
+
+	csr := &capi.CertificateSigningRequest{
+		Spec: capi.CertificateSigningRequestSpec{
+			Request: []byte(csrb),
+			Usages: []capi.KeyUsage{
+				capi.UsageSigning,
+				capi.UsageKeyEncipherment,
+				capi.UsageServerAuth,
+				capi.UsageClientAuth,
+			},
+		},
+	}
+
+	_, err = s.sign(csr)
+	if err == nil {
+		t.Fatal("missing error")
+	}
+	if !strings.HasPrefix(err.Error(), "the signer has expired") {
+		t.Fatal(err)
+	}
+}
+
+func TestDurationLongerThanExpiry(t *testing.T) {
+	testNow := time.Now()
+	testNowFn := func() time.Time {
+		return testNow
+	}
+
+	hundredYears := 24 * time.Hour * 365 * 100
+	s, err := newCFSSLSigner("./testdata/ca.crt", "./testdata/ca.key", nil, hundredYears)
+	if err != nil {
+		t.Fatalf("failed to create signer: %v", err)
+	}
+	s.nowFn = testNowFn
+
+	csrb, err := ioutil.ReadFile("./testdata/kubelet.csr")
+	if err != nil {
+		t.Fatalf("failed to read CSR: %v", err)
+	}
+
+	csr := &capi.CertificateSigningRequest{
+		Spec: capi.CertificateSigningRequestSpec{
+			Request: []byte(csrb),
+			Usages: []capi.KeyUsage{
+				capi.UsageSigning,
+				capi.UsageKeyEncipherment,
+				capi.UsageServerAuth,
+				capi.UsageClientAuth,
+			},
+		},
+	}
+
+	_, err = s.sign(csr)
+	if err != nil {
+		t.Fatalf("failed to sign CSR: %v", err)
+	}
+
+	// now we just need to verify that the expiry is based on the signing cert
+	certData := csr.Status.Certificate
+	if len(certData) == 0 {
+		t.Fatalf("expected a certificate after signing")
+	}
+
+	certs, err := cert.ParseCertsPEM(certData)
+	if err != nil {
+		t.Fatalf("failed to parse certificate: %v", err)
+	}
+	if len(certs) != 1 {
+		t.Fatalf("expected one certificate")
+	}
+
+	crt := certs[0]
+	expected, err := time.Parse("2006-01-02 15:04:05.999999999 -0700 MST", "2044-05-09 00:20:11 +0000 UTC")
+	if err != nil {
+		t.Fatal(err)
+	}
+	// there is some jitter that we need to tolerate
+	diff := expected.Sub(crt.NotAfter)
+	if diff > 10*time.Minute || diff < -10*time.Minute {
+		t.Fatal(crt.NotAfter)
+	}
 }