diff --git a/factory/ca.go b/factory/ca.go index 5dfa863..8a357b6 100644 --- a/factory/ca.go +++ b/factory/ca.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "os" + "time" "github.com/rancher/dynamiclistener/cert" ) @@ -16,7 +17,7 @@ func GenCA() (*x509.Certificate, crypto.Signer, error) { return nil, nil, err } - caCert, err := NewSelfSignedCACert(caKey, "dynamiclistener-ca", "dynamiclistener-org") + caCert, err := NewSelfSignedCACert(caKey, fmt.Sprintf("dynamiclistener-ca@%d", time.Now().Unix()), "dynamiclistener-org") if err != nil { return nil, nil, err } diff --git a/factory/gen.go b/factory/gen.go index b59f527..e620c5f 100644 --- a/factory/gen.go +++ b/factory/gen.go @@ -154,6 +154,10 @@ func (t *TLS) generateCert(secret *v1.Secret, cn ...string) (*v1.Secret, bool, e secret = &v1.Secret{} } + if err := t.Verify(secret); err != nil { + logrus.Warnf("unable to verify existing certificate: %v - signing operation may change certificate issuer", err) + } + secret = populateCN(secret, cn...) privateKey, err := getPrivateKey(secret) @@ -171,7 +175,7 @@ func (t *TLS) generateCert(secret *v1.Secret, cn ...string) (*v1.Secret, bool, e return nil, false, err } - certBytes, keyBytes, err := Marshal(newCert, privateKey) + keyBytes, certBytes, err := MarshalChain(privateKey, newCert, t.CACert) if err != nil { return nil, false, err } @@ -187,6 +191,29 @@ func (t *TLS) generateCert(secret *v1.Secret, cn ...string) (*v1.Secret, bool, e return secret, true, nil } +func (t *TLS) Verify(secret *v1.Secret) error { + certsPem := secret.Data[v1.TLSCertKey] + if len(certsPem) == 0 { + return nil + } + + certificates, err := cert.ParseCertsPEM(certsPem) + if err != nil || len(certificates) == 0 { + return err + } + + verifyOpts := x509.VerifyOptions{ + Roots: x509.NewCertPool(), + KeyUsages: []x509.ExtKeyUsage{ + x509.ExtKeyUsageAny, + }, + } + verifyOpts.Roots.AddCert(t.CACert) + + _, err = certificates[0].Verify(verifyOpts) + return err +} + func (t *TLS) newCert(domains []string, ips []net.IP, privateKey crypto.Signer) (*x509.Certificate, error) { return NewSignedCert(privateKey, t.CACert, t.CAKey, t.CN, t.Organization, domains, ips) } @@ -250,14 +277,33 @@ func getPrivateKey(secret *v1.Secret) (crypto.Signer, error) { return NewPrivateKey() } +// MarshalChain returns given key and certificates as byte slices. +func MarshalChain(privateKey crypto.Signer, certs ...*x509.Certificate) (keyBytes, certChainBytes []byte, err error) { + keyBytes, err = cert.MarshalPrivateKeyToPEM(privateKey) + if err != nil { + return nil, nil, err + } + + for _, cert := range certs { + if cert != nil { + certBlock := pem.Block{ + Type: CertificateBlockType, + Bytes: cert.Raw, + } + certChainBytes = append(certChainBytes, pem.EncodeToMemory(&certBlock)...) + } + } + return keyBytes, certChainBytes, nil +} + // Marshal returns the given cert and key as byte slices. -func Marshal(x509Cert *x509.Certificate, privateKey crypto.Signer) ([]byte, []byte, error) { +func Marshal(x509Cert *x509.Certificate, privateKey crypto.Signer) (certBytes, keyBytes []byte, err error) { certBlock := pem.Block{ Type: CertificateBlockType, Bytes: x509Cert.Raw, } - keyBytes, err := cert.MarshalPrivateKeyToPEM(privateKey) + keyBytes, err = cert.MarshalPrivateKeyToPEM(privateKey) if err != nil { return nil, nil, err } diff --git a/listener.go b/listener.go index c7f189b..3288003 100644 --- a/listener.go +++ b/listener.go @@ -171,7 +171,7 @@ func (l *listener) WrapExpiration(days int) net.Listener { for { wait := 6 * time.Hour if err := l.checkExpiration(days); err != nil { - logrus.Errorf("failed to check and renew dynamic cert: %v", err) + logrus.Errorf("dynamiclistener %s: failed to check and renew dynamic cert: %v", l.Addr(), err) // Don't go into short retry loop if we're using a static (user-provided) cert. // We will still check and print an error every six hours until the user updates the secret with // a cert that is not about to expire. Hopefully this will prompt them to take action. @@ -263,12 +263,18 @@ func (l *listener) Accept() (net.Conn, error) { l.init.Do(func() { if len(l.sans) > 0 { if err := l.updateCert(l.sans...); err != nil { - logrus.Errorf("failed to update cert with configured SANs: %v", err) + logrus.Errorf("dynamiclistener %s: failed to update cert with configured SANs: %v", l.Addr(), err) return } - if _, err := l.loadCert(nil); err != nil { - logrus.Errorf("failed to preload certificate: %v", err) - } + } + if cert, err := l.loadCert(nil); err != nil { + logrus.Errorf("dynamiclistener %s: failed to preload certificate: %v", l.Addr(), err) + } else if cert == nil { + // This should only occur on the first startup when no SANs are configured in the listener config, in which + // case no certificate can be created, as dynamiclistener will not create certificates until at least one IP + // or DNS SAN is set. It will also occur when using the Kubernetes storage without a local File cache. + // For reliable serving of requests, callers should configure a local cache and/or a default set of SANs. + logrus.Warnf("dynamiclistener %s: no cached certificate available for preload - deferring certificate load until storage initialization or first client request", l.Addr()) } }) @@ -284,14 +290,12 @@ func (l *listener) Accept() (net.Conn, error) { host, _, err := net.SplitHostPort(addr.String()) if err != nil { - logrus.Errorf("failed to parse network %s: %v", addr.Network(), err) + logrus.Errorf("dynamiclistener %s: failed to parse connection local address %s: %v", l.Addr(), addr, err) return conn, nil } - if !strings.Contains(host, ":") { - if err := l.updateCert(host); err != nil { - logrus.Errorf("failed to update cert with listener address: %v", err) - } + if err := l.updateCert(host); err != nil { + logrus.Errorf("dynamiclistener %s: failed to update cert with connection local address: %v", l.Addr(), err) } if l.conns != nil { @@ -338,7 +342,7 @@ func (l *listener) getCertificate(hello *tls.ClientHelloInfo) (*tls.Certificate, newConn := hello.Conn if hello.ServerName != "" { if err := l.updateCert(hello.ServerName); err != nil { - logrus.Errorf("failed to update cert with TLS ServerName: %v", err) + logrus.Errorf("dynamiclistener %s: failed to update cert with TLS ServerName: %v", l.Addr(), err) return nil, err } } @@ -455,7 +459,7 @@ func (l *listener) cacheHandler() http.Handler { } if err := l.updateCert(h); err != nil { - logrus.Errorf("failed to update cert with HTTP request Host header: %v", err) + logrus.Errorf("dynamiclistener %s: failed to update cert with HTTP request Host header: %v", l.Addr(), err) } } }) diff --git a/storage/kubernetes/ca.go b/storage/kubernetes/ca.go index d23ac4f..9584da4 100644 --- a/storage/kubernetes/ca.go +++ b/storage/kubernetes/ca.go @@ -56,7 +56,7 @@ func createAndStoreClientCert(secrets v1controller.SecretClient, namespace strin return nil, err } - certPem, keyPem, err := factory.Marshal(cert, key) + keyPem, certPem, err := factory.MarshalChain(key, cert, caCert) if err != nil { return nil, err } diff --git a/storage/kubernetes/controller.go b/storage/kubernetes/controller.go index 9f10854..80a756d 100644 --- a/storage/kubernetes/controller.go +++ b/storage/kubernetes/controller.go @@ -69,6 +69,8 @@ type storage struct { } func (s *storage) SetFactory(tls dynamiclistener.TLSFactory) { + s.Lock() + defer s.Unlock() s.tls = tls } @@ -109,7 +111,9 @@ func (s *storage) init(secrets v1controller.SecretController) { // local storage was empty, try to populate it secret, err := s.secrets.Get(s.namespace, s.name, metav1.GetOptions{}) if err != nil { - logrus.Warnf("Failed to init Kubernetes secret: %v", err) + if !errors.IsNotFound(err) { + logrus.Warnf("Failed to init Kubernetes secret: %v", err) + } return } @@ -144,7 +148,7 @@ func (s *storage) targetSecret() (*v1.Secret, error) { } func (s *storage) saveInK8s(secret *v1.Secret) (*v1.Secret, error) { - if !s.controllerHasSynced() { + if !s.initComplete() { return secret, nil } @@ -206,9 +210,14 @@ func (s *storage) Update(secret *v1.Secret) error { return nil } +func isConflictOrAlreadyExists(err error) bool { + return errors.IsConflict(err) || errors.IsAlreadyExists(err) +} + func (s *storage) update(secret *v1.Secret) (err error) { - err = retry.RetryOnConflict(retry.DefaultRetry, func() error { - secret, err = s.saveInK8s(secret) + var newSecret *v1.Secret + err = retry.OnError(retry.DefaultRetry, isConflictOrAlreadyExists, func() error { + newSecret, err = s.saveInK8s(secret) return err }) @@ -219,14 +228,11 @@ func (s *storage) update(secret *v1.Secret) (err error) { // Only hold the lock while updating underlying storage s.Lock() defer s.Unlock() - return s.storage.Update(secret) + return s.storage.Update(newSecret) } -func (s *storage) controllerHasSynced() bool { +func (s *storage) initComplete() bool { s.RLock() defer s.RUnlock() - if s.secrets == nil { - return false - } - return s.secrets.Informer().HasSynced() + return s.secrets != nil }