diff --git a/pkg/kubelet/certificate/certificate_manager.go b/pkg/kubelet/certificate/certificate_manager.go index d3426c0571b..6df0c8b44db 100644 --- a/pkg/kubelet/certificate/certificate_manager.go +++ b/pkg/kubelet/certificate/certificate_manager.go @@ -48,19 +48,56 @@ const ( type Manager interface { // Start the API server status sync loop. Start() - // GetCertificate gets the current certificate from the certificate - // manager. This function matches the signature required by - // tls.Config.GetCertificate so it can be passed as TLS configuration. A - // TLS server will automatically call back here to get the correct - // certificate when establishing each new connection. - GetCertificate(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) + // Current returns the currently selected certificate from the + // certificate manager. + Current() *tls.Certificate +} + +// Config is the set of configuration parameters available for a new Manager. +type Config struct { + // CertificateSigningRequestClient will be used for signing new certificate + // requests generated when a key rotation occurs. + CertificateSigningRequestClient certificatesclient.CertificateSigningRequestInterface + // Template is the CertificateRequest that will be used as a template for + // generating certificate signing requests for all new keys generated as + // part of rotation. It follows the same rules as the template parameter of + // crypto.x509.CreateCertificateRequest in the Go standard libraries. + Template *x509.CertificateRequest + // Usages is the types of usages that certificates generated by the manager + // can be used for. + Usages []certificates.KeyUsage + // CertificateStore is a persistent store where the current cert/key is + // kept and future cert/key pairs will be persisted after they are + // generated. + CertificateStore Store + // BootstrapCertificatePEM is the certificate data that will be returned + // from the Manager if the CertificateStore doesn't have any cert/key pairs + // currently available. If the CertificateStore does have a cert/key pair, + // this will be ignored. If the bootstrap cert/key pair are used, they will + // be rotated at the first opportunity, possibly well in advance of + // expiring. This is intended to allow the first boot of a component to be + // initialized using a generic, multi-use cert/key pair which will be + // quickly replaced with a unique cert/key pair. + BootstrapCertificatePEM []byte + // BootstrapKeyPEM is the key data that will be returned from the Manager + // if the CertificateStore doesn't have any cert/key pairs currently + // available. If the CertificateStore does have a cert/key pair, this will + // be ignored. If the bootstrap cert/key pair are used, they will be + // rotated at the first opportunity, possibly well in advance of expiring. + // This is intended to allow the first boot of a component to be + // initialized using a generic, multi-use cert/key pair which will be + // quickly replaced with a unique cert/key pair. + BootstrapKeyPEM []byte } // Store is responsible for getting and updating the current certificate. // Depending on the concrete implementation, the backing store for this // behavior may vary. type Store interface { - // Current returns the currently selected certificate. + // Current returns the currently selected certificate. If the Store doesn't + // have a cert/key pair currently, it should return a NoCertKeyError so + // that the Manager can recover by using bootstrap certificates to request + // a new cert/key pair. Current() (*tls.Certificate, error) // Update accepts the PEM data for the cert/key pair and makes the new // cert/key pair the 'current' pair, that will be returned by future calls @@ -68,6 +105,11 @@ type Store interface { Update(cert, key []byte) (*tls.Certificate, error) } +// NoCertKeyError indicates there is no cert/key currently available. +type NoCertKeyError string + +func (e *NoCertKeyError) Error() string { return string(*e) } + type manager struct { certSigningRequestClient certificatesclient.CertificateSigningRequestInterface template *x509.CertificateRequest @@ -75,49 +117,39 @@ type manager struct { certStore Store certAccessLock sync.RWMutex cert *tls.Certificate + forceRotation bool } // NewManager returns a new certificate manager. A certificate manager is // responsible for being the authoritative source of certificates in the // Kubelet and handling updates due to rotation. -func NewManager( - certSigningRequestClient certificatesclient.CertificateSigningRequestInterface, - template *x509.CertificateRequest, - usages []certificates.KeyUsage, - certificateStore Store) (Manager, error) { - - cert, err := certificateStore.Current() +func NewManager(config *Config) (Manager, error) { + cert, forceRotation, err := getCurrentCertificateOrBootstrap( + config.CertificateStore, + config.BootstrapCertificatePEM, + config.BootstrapKeyPEM) if err != nil { return nil, err } m := manager{ - certSigningRequestClient: certSigningRequestClient, - template: template, - usages: usages, - certStore: certificateStore, + certSigningRequestClient: config.CertificateSigningRequestClient, + template: config.Template, + usages: config.Usages, + certStore: config.CertificateStore, cert: cert, + forceRotation: forceRotation, } return &m, nil } -// GetCertificate returns the certificate that should be used with TLS -// connections. The value returned by this function will change over time as -// the certificate is rotated. If a reference to this method is passed directly -// into the TLS options for a connection, certificate rotation will be handled -// correctly by the underlying go libraries. -// -// tlsOptions := &server.TLSOptions{ -// ... -// GetCertificate: certificateManager.GetCertificate -// ... -// } -// -func (m *manager) GetCertificate(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) { +// Current returns the currently selected certificate from the +// certificate manager. +func (m *manager) Current() *tls.Certificate { m.certAccessLock.RLock() defer m.certAccessLock.RUnlock() - return m.cert, nil + return m.cert } // Start will start the background work of rotating the certificates. @@ -142,6 +174,40 @@ func (m *manager) Start() { }, 0) } +func getCurrentCertificateOrBootstrap( + store Store, + bootstrapCertificatePEM []byte, + bootstrapKeyPEM []byte) (cert *tls.Certificate, shouldRotate bool, errResult error) { + + currentCert, err := store.Current() + if err == nil { + return currentCert, false, nil + } + + if _, ok := err.(*NoCertKeyError); !ok { + return nil, false, err + } + + if bootstrapCertificatePEM == nil || bootstrapKeyPEM == nil { + return nil, false, fmt.Errorf("no cert/key available and no bootstrap cert/key to fall back to") + } + + bootstrapCert, err := tls.X509KeyPair(bootstrapCertificatePEM, bootstrapKeyPEM) + if err != nil { + return nil, false, err + } + if len(bootstrapCert.Certificate) < 1 { + return nil, false, fmt.Errorf("no cert/key data found") + } + + certs, err := x509.ParseCertificates(bootstrapCert.Certificate[0]) + if err != nil { + return nil, false, fmt.Errorf("unable to parse certificate data: %v", err) + } + bootstrapCert.Leaf = certs[0] + return &bootstrapCert, true, nil +} + // shouldRotate looks at how close the current certificate is to expiring and // decides if it is time to rotate or not. func (m *manager) shouldRotate() bool { @@ -158,7 +224,8 @@ func (m *manager) shouldRotate() bool { jitteryDuration := wait.Jitter(time.Duration(totalDuration), 0.2) - time.Duration(totalDuration*0.3) rotationThreshold := m.cert.Leaf.NotBefore.Add(jitteryDuration) - return time.Now().After(rotationThreshold) + passedThreshold := time.Now().After(rotationThreshold) + return m.forceRotation || passedThreshold } func (m *manager) rotateCerts() error { @@ -183,10 +250,15 @@ func (m *manager) rotateCerts() error { return fmt.Errorf("unable to store the new cert/key pair: %v", err) } + m.updateCached(cert) + m.forceRotation = false + return nil +} + +func (m *manager) updateCached(cert *tls.Certificate) { m.certAccessLock.Lock() defer m.certAccessLock.Unlock() m.cert = cert - return nil } func (m *manager) generateCSR() (csrPEM []byte, keyPEM []byte, err error) { diff --git a/pkg/kubelet/certificate/certificate_manager_test.go b/pkg/kubelet/certificate/certificate_manager_test.go index 6311009ffda..423981e0c05 100644 --- a/pkg/kubelet/certificate/certificate_manager_test.go +++ b/pkg/kubelet/certificate/certificate_manager_test.go @@ -17,9 +17,11 @@ limitations under the License. package certificate import ( + "bytes" "crypto/tls" "crypto/x509" "fmt" + "strings" "testing" "time" @@ -78,6 +80,24 @@ O1eRCsCGPAnUCviFgNeH15ug+6N54DTTR6ZV/TTV64FDOcsox9nrhYcmH9sYuITi -----END CERTIFICATE-----` ) +func TestNewManagerNoRotation(t *testing.T) { + cert, err := tls.X509KeyPair([]byte(certificateData), []byte(privateKeyData)) + if err != nil { + t.Fatalf("Unable to initialize a certificate: %v", err) + } + + store := &fakeStore{ + cert: &cert, + } + if _, err := NewManager(&Config{ + Template: &x509.CertificateRequest{}, + Usages: []certificates.KeyUsage{}, + CertificateStore: store, + }); err != nil { + t.Fatalf("Failed to initialize the certificate manager: %v", err) + } +} + func TestShouldRotate(t *testing.T) { now := time.Now() tests := []struct { @@ -122,8 +142,8 @@ func TestRotateCertCreateCSRError(t *testing.T) { m := manager{ cert: &tls.Certificate{ Leaf: &x509.Certificate{ - NotAfter: now.Add(-1 * time.Hour), NotBefore: now.Add(-2 * time.Hour), + NotAfter: now.Add(-1 * time.Hour), }, }, template: &x509.CertificateRequest{}, @@ -143,8 +163,8 @@ func TestRotateCertWaitingForResultError(t *testing.T) { m := manager{ cert: &tls.Certificate{ Leaf: &x509.Certificate{ - NotAfter: now.Add(-1 * time.Hour), NotBefore: now.Add(-2 * time.Hour), + NotAfter: now.Add(-1 * time.Hour), }, }, template: &x509.CertificateRequest{}, @@ -159,6 +179,146 @@ func TestRotateCertWaitingForResultError(t *testing.T) { } } +func TestNewManagerBootstrap(t *testing.T) { + store := &fakeStore{} + + cm, err := NewManager(&Config{ + Template: &x509.CertificateRequest{}, + Usages: []certificates.KeyUsage{}, + CertificateStore: store, + BootstrapCertificatePEM: []byte(certificateData), + BootstrapKeyPEM: []byte(privateKeyData), + }) + + if err != nil { + t.Fatalf("Failed to initialize the certificate manager: %v", err) + } + + cert := cm.Current() + + if cert == nil { + t.Errorf("Certificate was nil, expected something.") + } + + if m, ok := cm.(*manager); !ok { + t.Errorf("Expected a '*manager' from 'NewManager'") + } else if !m.shouldRotate() { + t.Errorf("Expected rotation should happen during bootstrap, but it won't.") + } +} + +func TestNewManagerNoBootstrap(t *testing.T) { + now := time.Now() + cert, err := tls.X509KeyPair([]byte(certificateData), []byte(privateKeyData)) + if err != nil { + t.Fatalf("Unable to initialize a certificate: %v", err) + } + cert.Leaf = &x509.Certificate{ + NotBefore: now.Add(-24 * time.Hour), + NotAfter: now.Add(24 * time.Hour), + } + store := &fakeStore{ + cert: &cert, + } + + cm, err := NewManager(&Config{ + Template: &x509.CertificateRequest{}, + Usages: []certificates.KeyUsage{}, + CertificateStore: store, + BootstrapCertificatePEM: []byte(certificateData), + BootstrapKeyPEM: []byte(privateKeyData), + }) + + if err != nil { + t.Fatalf("Failed to initialize the certificate manager: %v", err) + } + + currentCert := cm.Current() + + if currentCert == nil { + t.Errorf("Certificate was nil, expected something.") + } + + if m, ok := cm.(*manager); !ok { + t.Errorf("Expected a '*manager' from 'NewManager'") + } else if m.shouldRotate() { + t.Errorf("Expected rotation should happen during bootstrap, but it won't.") + } +} + +func TestGetCurrentCertificateOrBootstrap(t *testing.T) { + cert, err := tls.X509KeyPair([]byte(certificateData), []byte(privateKeyData)) + if err != nil { + t.Fatalf("Unable to initialize a certificate: %v", err) + } + + testCases := []struct { + description string + storeCert *tls.Certificate + bootstrapCertData []byte + bootstrapKeyData []byte + expectedCert *tls.Certificate + expectedShouldRotate bool + expectedErrMsg string + }{ + { + "return cert from store", + &cert, + nil, + nil, + &cert, + false, + "", + }, + { + "no cert in store and no bootstrap cert", + nil, + nil, + nil, + nil, + false, + "no cert/key available and no bootstrap cert/key to fall back to", + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + store := &fakeStore{ + cert: tc.storeCert, + } + + certResult, shouldRotate, err := getCurrentCertificateOrBootstrap( + store, + tc.bootstrapCertData, + tc.bootstrapKeyData) + if certResult == nil || tc.expectedCert == nil { + if certResult != tc.expectedCert { + t.Errorf("Got certificate %v, wanted %v", certResult, tc.expectedCert) + } + } else { + if len(certResult.Certificate) != len(tc.expectedCert.Certificate) { + t.Errorf("Got %d certificates, wanted %d", len(certResult.Certificate), len(tc.expectedCert.Certificate)) + } + if !bytes.Equal(certResult.Certificate[0], tc.expectedCert.Certificate[0]) { + t.Errorf("Got certificate %v, wanted %v", certResult, tc.expectedCert) + } + } + if shouldRotate != tc.expectedShouldRotate { + t.Errorf("Got shouldRotate %t, wanted %t", shouldRotate, tc.expectedShouldRotate) + } + if err == nil { + if tc.expectedErrMsg != "" { + t.Errorf("Got err %v, wanted %q", err, tc.expectedErrMsg) + } + } else { + if tc.expectedErrMsg == "" || !strings.Contains(err.Error(), tc.expectedErrMsg) { + t.Errorf("Got err %v, wanted %q", err, tc.expectedErrMsg) + } + } + }) + } +} + type fakeClientFailureType int const ( @@ -233,6 +393,10 @@ type fakeStore struct { } func (s *fakeStore) Current() (*tls.Certificate, error) { + if s.cert == nil { + noKeyErr := NoCertKeyError("") + return nil, &noKeyErr + } return s.cert, nil } @@ -244,6 +408,11 @@ func (s *fakeStore) Update(certPEM, keyPEM []byte) (*tls.Certificate, error) { if err != nil { return nil, err } + now := time.Now() s.cert = &cert + s.cert.Leaf = &x509.Certificate{ + NotBefore: now.Add(-24 * time.Hour), + NotAfter: now.Add(24 * time.Hour), + } return s.cert, nil } diff --git a/pkg/kubelet/certificate/certificate_store.go b/pkg/kubelet/certificate/certificate_store.go index 02f5faaf738..890961d83f2 100644 --- a/pkg/kubelet/certificate/certificate_store.go +++ b/pkg/kubelet/certificate/certificate_store.go @@ -148,12 +148,14 @@ func (s *fileStore) Current() (*tls.Certificate, error) { return loadX509KeyPair(c, k) } - return nil, fmt.Errorf("no cert/key files read at %q, (%q, %q) or (%q, %q)", - pairFile, - s.certFile, - s.keyFile, - s.certDirectory, - s.keyDirectory) + noKeyErr := NoCertKeyError( + fmt.Sprintf("no cert/key files read at %q, (%q, %q) or (%q, %q)", + pairFile, + s.certFile, + s.keyFile, + s.certDirectory, + s.keyDirectory)) + return nil, &noKeyErr } func loadFile(pairFile string) (*tls.Certificate, error) { diff --git a/pkg/kubelet/certificate/certificate_store_test.go b/pkg/kubelet/certificate/certificate_store_test.go index 5769f463a92..968ff3e315d 100644 --- a/pkg/kubelet/certificate/certificate_store_test.go +++ b/pkg/kubelet/certificate/certificate_store_test.go @@ -181,7 +181,7 @@ func TestUpdateSymlinkReplaceExistingSymlink(t *testing.T) { func TestLoadCertKeyBlocksNoFile(t *testing.T) { dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -199,7 +199,7 @@ func TestLoadCertKeyBlocksNoFile(t *testing.T) { func TestLoadCertKeyBlocksEmptyFile(t *testing.T) { dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -220,7 +220,7 @@ func TestLoadCertKeyBlocksEmptyFile(t *testing.T) { func TestLoadCertKeyBlocksPartialFile(t *testing.T) { dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -241,7 +241,7 @@ func TestLoadCertKeyBlocksPartialFile(t *testing.T) { func TestLoadCertKeyBlocks(t *testing.T) { dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -269,7 +269,7 @@ func TestLoadCertKeyBlocks(t *testing.T) { func TestLoadFile(t *testing.T) { dir, err := ioutil.TempDir("", "k8s-test-load-cert-key-blocks") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -298,7 +298,7 @@ func TestUpdateNoRotation(t *testing.T) { prefix := "kubelet-server" dir, err := ioutil.TempDir("", "k8s-test-certstore-current") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -332,7 +332,7 @@ func TestUpdateRotation(t *testing.T) { prefix := "kubelet-server" dir, err := ioutil.TempDir("", "k8s-test-certstore-current") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -366,7 +366,7 @@ func TestUpdateWithBadCertKeyData(t *testing.T) { prefix := "kubelet-server" dir, err := ioutil.TempDir("", "k8s-test-certstore-current") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -400,7 +400,7 @@ func TestCurrentPairFile(t *testing.T) { prefix := "kubelet-server" dir, err := ioutil.TempDir("", "k8s-test-certstore-current") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -437,7 +437,7 @@ func TestCurrentCertKeyFiles(t *testing.T) { prefix := "kubelet-server" dir, err := ioutil.TempDir("", "k8s-test-certstore-current") if err != nil { - t.Fatalf("Unable to created the test directory %q: %v", dir, err) + t.Fatalf("Unable to create the test directory %q: %v", dir, err) } defer func() { if err := os.RemoveAll(dir); err != nil { @@ -469,3 +469,31 @@ func TestCurrentCertKeyFiles(t *testing.T) { t.Fatalf("Got an empty leaf, expected private data.") } } + +func TestCurrentNoFiles(t *testing.T) { + dir, err := ioutil.TempDir("", "k8s-test-certstore-current") + if err != nil { + t.Fatalf("Unable to create the test directory %q: %v", dir, err) + } + defer func() { + if err := os.RemoveAll(dir); err != nil { + t.Errorf("Unable to clean up test directory %q: %v", dir, err) + } + }() + + store, err := NewFileStore("kubelet-server", dir, dir, "", "") + if err != nil { + t.Fatalf("Failed to initialize certificate store: %v", err) + } + + cert, err := store.Current() + if err == nil { + t.Fatalf("Got no error, expected an error because the cert/key files don't exist.") + } + if _, ok := err.(*NoCertKeyError); !ok { + t.Fatalf("Got error %v, expected NoCertKeyError.", err) + } + if cert != nil { + t.Fatalf("Got certificate, expected no certificate because the cert/key files don't exist.") + } +}