Fix initial secret not being written to Kubernetes

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
 	}

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
 	}

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)
 			}
 		}
 	})

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
 	}

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
 }