diff --git a/cmd/kubelet/app/server_bootstrap_test.go b/cmd/kubelet/app/server_bootstrap_test.go index c83f604340a..8cf2ee852da 100644 --- a/cmd/kubelet/app/server_bootstrap_test.go +++ b/cmd/kubelet/app/server_bootstrap_test.go @@ -298,14 +298,14 @@ func (s *csrSimulator) ServeHTTP(w http.ResponseWriter, req *http.Request) { ca := &authority.CertificateAuthority{ Certificate: s.serverCA, PrivateKey: s.serverPrivateKey, - Backdate: s.backdate, } cr, err := capihelper.ParseCSR(csr.Spec.Request) if err != nil { t.Fatal(err) } der, err := ca.Sign(cr.Raw, authority.PermissiveSigningPolicy{ - TTL: time.Hour, + TTL: time.Hour, + Backdate: s.backdate, }) if err != nil { t.Fatal(err) diff --git a/pkg/controller/certificates/authority/authority.go b/pkg/controller/certificates/authority/authority.go index 5d064bbb66d..804b633a802 100644 --- a/pkg/controller/certificates/authority/authority.go +++ b/pkg/controller/certificates/authority/authority.go @@ -22,7 +22,6 @@ import ( "crypto/x509" "fmt" "math/big" - "time" ) var serialNumberLimit = new(big.Int).Lsh(big.NewInt(1), 128) @@ -37,23 +36,11 @@ type CertificateAuthority struct { Certificate *x509.Certificate PrivateKey crypto.Signer - Backdate time.Duration - Now func() time.Time } // Sign signs a certificate request, applying a SigningPolicy and returns a DER // encoded x509 certificate. func (ca *CertificateAuthority) Sign(crDER []byte, policy SigningPolicy) ([]byte, error) { - now := time.Now() - if ca.Now != nil { - now = ca.Now() - } - - nbf := now.Add(-ca.Backdate) - if !nbf.Before(ca.Certificate.NotAfter) { - return nil, fmt.Errorf("the signer has expired: NotAfter=%v", ca.Certificate.NotAfter) - } - cr, err := x509.ParseCertificateRequest(crDER) if err != nil { return nil, fmt.Errorf("unable to parse certificate request: %v", err) @@ -78,19 +65,11 @@ func (ca *CertificateAuthority) Sign(crDER []byte, policy SigningPolicy) ([]byte PublicKey: cr.PublicKey, Extensions: cr.Extensions, ExtraExtensions: cr.ExtraExtensions, - NotBefore: nbf, } - if err := policy.apply(tmpl); err != nil { + if err := policy.apply(tmpl, ca.Certificate.NotAfter); err != nil { return nil, err } - if !tmpl.NotAfter.Before(ca.Certificate.NotAfter) { - tmpl.NotAfter = ca.Certificate.NotAfter - } - if !now.Before(ca.Certificate.NotAfter) { - return nil, fmt.Errorf("refusing to sign a certificate that expired in the past") - } - der, err := x509.CreateCertificate(rand.Reader, tmpl, ca.Certificate, cr.PublicKey, ca.PrivateKey) if err != nil { return nil, fmt.Errorf("failed to sign certificate: %v", err) diff --git a/pkg/controller/certificates/authority/authority_test.go b/pkg/controller/certificates/authority/authority_test.go index a875e4f737f..7ae13ec3823 100644 --- a/pkg/controller/certificates/authority/authority_test.go +++ b/pkg/controller/certificates/authority/authority_test.go @@ -40,6 +40,7 @@ func TestCertificateAuthority(t *testing.T) { t.Fatal(err) } now := time.Now() + nowFunc := func() time.Time { return now } tmpl := &x509.Certificate{ SerialNumber: big.NewInt(42), Subject: pkix.Name{ @@ -68,15 +69,15 @@ func TestCertificateAuthority(t *testing.T) { tests := []struct { name string cr x509.CertificateRequest - backdate time.Duration policy SigningPolicy + mutateCA func(ca *CertificateAuthority) want x509.Certificate - wantErr bool + wantErr string }{ { name: "ca info", - policy: PermissiveSigningPolicy{TTL: time.Hour}, + policy: PermissiveSigningPolicy{TTL: time.Hour, Now: nowFunc}, want: x509.Certificate{ Issuer: caCert.Subject, AuthorityKeyId: caCert.SubjectKeyId, @@ -87,7 +88,7 @@ func TestCertificateAuthority(t *testing.T) { }, { name: "key usage", - policy: PermissiveSigningPolicy{TTL: time.Hour, Usages: []capi.KeyUsage{"signing"}}, + policy: PermissiveSigningPolicy{TTL: time.Hour, Usages: []capi.KeyUsage{"signing"}, Now: nowFunc}, want: x509.Certificate{ NotBefore: now, NotAfter: now.Add(1 * time.Hour), @@ -97,7 +98,7 @@ func TestCertificateAuthority(t *testing.T) { }, { name: "ext key usage", - policy: PermissiveSigningPolicy{TTL: time.Hour, Usages: []capi.KeyUsage{"client auth"}}, + policy: PermissiveSigningPolicy{TTL: time.Hour, Usages: []capi.KeyUsage{"client auth"}, Now: nowFunc}, want: x509.Certificate{ NotBefore: now, NotAfter: now.Add(1 * time.Hour), @@ -106,18 +107,53 @@ func TestCertificateAuthority(t *testing.T) { }, }, { - name: "backdate", - policy: PermissiveSigningPolicy{TTL: time.Hour}, - backdate: 5 * time.Minute, + name: "backdate without short", + policy: PermissiveSigningPolicy{TTL: time.Hour, Backdate: 5 * time.Minute, Now: nowFunc}, want: x509.Certificate{ NotBefore: now.Add(-5 * time.Minute), NotAfter: now.Add(55 * time.Minute), BasicConstraintsValid: true, }, }, + { + name: "backdate without short and super small ttl", + policy: PermissiveSigningPolicy{TTL: time.Minute, Backdate: 5 * time.Minute, Now: nowFunc}, + want: x509.Certificate{ + NotBefore: now.Add(-5 * time.Minute), + NotAfter: now.Add(-4 * time.Minute), + BasicConstraintsValid: true, + }, + }, + { + name: "backdate with short", + policy: PermissiveSigningPolicy{TTL: time.Hour, Backdate: 5 * time.Minute, Short: 8 * time.Hour, Now: nowFunc}, + want: x509.Certificate{ + NotBefore: now.Add(-5 * time.Minute), + NotAfter: now.Add(time.Hour), + BasicConstraintsValid: true, + }, + }, + { + name: "backdate with short and super small ttl", + policy: PermissiveSigningPolicy{TTL: time.Minute, Backdate: 5 * time.Minute, Short: 8 * time.Hour, Now: nowFunc}, + want: x509.Certificate{ + NotBefore: now.Add(-5 * time.Minute), + NotAfter: now.Add(time.Minute), + BasicConstraintsValid: true, + }, + }, + { + name: "backdate with short but longer ttl", + policy: PermissiveSigningPolicy{TTL: 24 * time.Hour, Backdate: 5 * time.Minute, Short: 8 * time.Hour, Now: nowFunc}, + want: x509.Certificate{ + NotBefore: now.Add(-5 * time.Minute), + NotAfter: now.Add(24*time.Hour - 5*time.Minute), + BasicConstraintsValid: true, + }, + }, { name: "truncate expiration", - policy: PermissiveSigningPolicy{TTL: 48 * time.Hour}, + policy: PermissiveSigningPolicy{TTL: 48 * time.Hour, Now: nowFunc}, want: x509.Certificate{ NotBefore: now, NotAfter: now.Add(24 * time.Hour), @@ -126,7 +162,7 @@ func TestCertificateAuthority(t *testing.T) { }, { name: "uri sans", - policy: PermissiveSigningPolicy{TTL: time.Hour}, + policy: PermissiveSigningPolicy{TTL: time.Hour, Now: nowFunc}, cr: x509.CertificateRequest{ URIs: []*url.URL{uri}, }, @@ -137,6 +173,22 @@ func TestCertificateAuthority(t *testing.T) { BasicConstraintsValid: true, }, }, + { + name: "expired ca", + policy: PermissiveSigningPolicy{TTL: time.Hour, Now: nowFunc}, + mutateCA: func(ca *CertificateAuthority) { + ca.Certificate.NotAfter = now // pretend that the CA has expired + }, + wantErr: "the signer has expired: NotAfter=" + now.String(), + }, + { + name: "expired ca with backdate", + policy: PermissiveSigningPolicy{TTL: time.Hour, Backdate: 5 * time.Minute, Now: nowFunc}, + mutateCA: func(ca *CertificateAuthority) { + ca.Certificate.NotAfter = now // pretend that the CA has expired + }, + wantErr: "refusing to sign a certificate that expired in the past: NotAfter=" + now.String(), + }, } crKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) @@ -146,13 +198,15 @@ func TestCertificateAuthority(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + caCertShallowCopy := *caCert + ca := &CertificateAuthority{ - Certificate: caCert, + Certificate: &caCertShallowCopy, PrivateKey: caKey, - Now: func() time.Time { - return now - }, - Backdate: test.backdate, + } + + if test.mutateCA != nil { + test.mutateCA(ca) } csr, err := x509.CreateCertificateRequest(rand.Reader, &test.cr, crKey) @@ -161,15 +215,15 @@ func TestCertificateAuthority(t *testing.T) { } certDER, err := ca.Sign(csr, test.policy) - if err != nil { - t.Fatal(err) - } - if test.wantErr { - if err == nil { - t.Fatal("expected error") + if len(test.wantErr) > 0 { + if errStr := errString(err); test.wantErr != errStr { + t.Fatalf("expected error %s but got %s", test.wantErr, errStr) } return } + if err != nil { + t.Fatal(err) + } cert, err := x509.ParseCertificate(certDER) if err != nil { @@ -197,3 +251,11 @@ func TestCertificateAuthority(t *testing.T) { }) } } + +func errString(err error) string { + if err == nil { + return "" + } + + return err.Error() +} diff --git a/pkg/controller/certificates/authority/policies.go b/pkg/controller/certificates/authority/policies.go index 0d8d9b374d1..411fafeec4b 100644 --- a/pkg/controller/certificates/authority/policies.go +++ b/pkg/controller/certificates/authority/policies.go @@ -31,7 +31,7 @@ import ( type SigningPolicy interface { // not-exporting apply forces signing policy implementations to be internal // to this package. - apply(template *x509.Certificate) error + apply(template *x509.Certificate, signerNotAfter time.Time) error } // PermissiveSigningPolicy is the signing policy historically used by the local @@ -39,32 +39,75 @@ type SigningPolicy interface { // // * It forwards all SANs from the original signing request. // * It sets allowed usages as configured in the policy. -// * It sets NotAfter based on the TTL configured in the policy. // * It zeros all extensions. // * It sets BasicConstraints to true. // * It sets IsCA to false. +// * It validates that the signer has not expired. +// * It sets NotBefore and NotAfter: +// All certificates set NotBefore = Now() - Backdate. +// Long-lived certificates set NotAfter = Now() + TTL - Backdate. +// Short-lived certificates set NotAfter = Now() + TTL. +// All certificates truncate NotAfter to the expiration date of the signer. type PermissiveSigningPolicy struct { - // TTL is the certificate TTL. It's used to calculate the NotAfter value of - // the certificate. + // TTL is used in certificate NotAfter calculation as described above. TTL time.Duration + // Usages are the allowed usages of a certificate. Usages []capi.KeyUsage + + // Backdate is used in certificate NotBefore calculation as described above. + Backdate time.Duration + + // Short is the duration used to determine if the lifetime of a certificate should be considered short. + Short time.Duration + + // Now defaults to time.Now but can be stubbed for testing + Now func() time.Time } -func (p PermissiveSigningPolicy) apply(tmpl *x509.Certificate) error { +func (p PermissiveSigningPolicy) apply(tmpl *x509.Certificate, signerNotAfter time.Time) error { + var now time.Time + if p.Now != nil { + now = p.Now() + } else { + now = time.Now() + } + + ttl := p.TTL + usage, extUsages, err := keyUsagesFromStrings(p.Usages) if err != nil { return err } tmpl.KeyUsage = usage tmpl.ExtKeyUsage = extUsages - tmpl.NotAfter = tmpl.NotBefore.Add(p.TTL) tmpl.ExtraExtensions = nil tmpl.Extensions = nil tmpl.BasicConstraintsValid = true tmpl.IsCA = false + tmpl.NotBefore = now.Add(-p.Backdate) + + if ttl < p.Short { + // do not backdate the end time if we consider this to be a short lived certificate + tmpl.NotAfter = now.Add(ttl) + } else { + tmpl.NotAfter = now.Add(ttl - p.Backdate) + } + + if !tmpl.NotAfter.Before(signerNotAfter) { + tmpl.NotAfter = signerNotAfter + } + + if !tmpl.NotBefore.Before(signerNotAfter) { + return fmt.Errorf("the signer has expired: NotAfter=%v", signerNotAfter) + } + + if !now.Before(signerNotAfter) { + return fmt.Errorf("refusing to sign a certificate that expired in the past: NotAfter=%v", signerNotAfter) + } + return nil } @@ -124,7 +167,7 @@ func keyUsagesFromStrings(usages []capi.KeyUsage) (x509.KeyUsage, []x509.ExtKeyU return 0, nil, fmt.Errorf("unrecognized usage values: %q", unrecognized) } - return keyUsage, []x509.ExtKeyUsage(sorted), nil + return keyUsage, sorted, nil } type sortedExtKeyUsage []x509.ExtKeyUsage diff --git a/pkg/controller/certificates/signer/ca_provider.go b/pkg/controller/certificates/signer/ca_provider.go index 1d0d560aa3a..405b7a0506a 100644 --- a/pkg/controller/certificates/signer/ca_provider.go +++ b/pkg/controller/certificates/signer/ca_provider.go @@ -21,7 +21,6 @@ import ( "crypto" "fmt" "sync/atomic" - "time" "k8s.io/apiserver/pkg/server/dynamiccertificates" "k8s.io/client-go/util/cert" @@ -77,7 +76,6 @@ func (p *caProvider) setCA() error { Certificate: certs[0], PrivateKey: priv, - Backdate: 5 * time.Minute, } p.caValue.Store(ca) diff --git a/pkg/controller/certificates/signer/signer.go b/pkg/controller/certificates/signer/signer.go index a6c084ef45c..604765515e0 100644 --- a/pkg/controller/certificates/signer/signer.go +++ b/pkg/controller/certificates/signer/signer.go @@ -173,7 +173,7 @@ func (s *signer) handle(csr *capi.CertificateSigningRequest) error { // Ignore requests for kubernetes.io signerNames we don't recognize return nil } - cert, err := s.sign(x509cr, csr.Spec.Usages) + cert, err := s.sign(x509cr, csr.Spec.Usages, nil) if err != nil { return fmt.Errorf("error auto signing csr: %v", err) } @@ -185,14 +185,17 @@ func (s *signer) handle(csr *capi.CertificateSigningRequest) error { return nil } -func (s *signer) sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage) ([]byte, error) { +func (s *signer) sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage, now func() time.Time) ([]byte, error) { currCA, err := s.caProvider.currentCA() if err != nil { return nil, err } der, err := currCA.Sign(x509cr.Raw, authority.PermissiveSigningPolicy{ - TTL: s.certTTL, - Usages: usages, + TTL: s.certTTL, + Usages: usages, + Backdate: 5 * time.Minute, // this must always be less than the minimum TTL requested by a user + Short: 8 * time.Hour, // 5 minutes of backdating is roughly 1% of 8 hours + Now: now, }) if err != nil { return nil, err diff --git a/pkg/controller/certificates/signer/signer_test.go b/pkg/controller/certificates/signer/signer_test.go index 045349a4b77..215838d7dba 100644 --- a/pkg/controller/certificates/signer/signer_test.go +++ b/pkg/controller/certificates/signer/signer_test.go @@ -35,25 +35,17 @@ import ( "k8s.io/client-go/kubernetes/fake" testclient "k8s.io/client-go/testing" "k8s.io/client-go/util/cert" - "k8s.io/kubernetes/pkg/controller/certificates" - capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1" + "k8s.io/kubernetes/pkg/controller/certificates" ) func TestSigner(t *testing.T) { - clock := clock.FakeClock{} + fakeClock := clock.FakeClock{} s, err := newSigner("kubernetes.io/legacy-unknown", "./testdata/ca.crt", "./testdata/ca.key", nil, 1*time.Hour) if err != nil { t.Fatalf("failed to create signer: %v", err) } - currCA, err := s.caProvider.currentCA() - if err != nil { - t.Fatal(err) - } - currCA.Now = clock.Now - currCA.Backdate = 0 - s.caProvider.caValue.Store(currCA) csrb, err := ioutil.ReadFile("./testdata/kubelet.csr") if err != nil { @@ -69,7 +61,7 @@ func TestSigner(t *testing.T) { capi.UsageKeyEncipherment, capi.UsageServerAuth, capi.UsageClientAuth, - }) + }, fakeClock.Now) if err != nil { t.Fatalf("failed to sign CSR: %v", err) } @@ -94,7 +86,8 @@ func TestSigner(t *testing.T) { KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, BasicConstraintsValid: true, - NotAfter: clock.Now().Add(1 * time.Hour), + NotBefore: fakeClock.Now().Add(-5 * time.Minute), + NotAfter: fakeClock.Now().Add(1 * time.Hour), PublicKeyAlgorithm: x509.ECDSA, SignatureAlgorithm: x509.SHA256WithRSA, MaxPathLen: -1,