Connect: Verify the leaf cert to determine if its ready.

connect/service.go

@@ -72,7 +72,7 @@ func NewServiceWithLogger(serviceName string, client *api.Client,
 		service:              serviceName,
 		client:               client,
 		logger:               logger,
-		tlsCfg:               newDynamicTLSConfig(defaultTLSConfig()),
+		tlsCfg:               newDynamicTLSConfig(defaultTLSConfig(), logger),
 		httpResolverFromAddr: ConsulResolverFromAddrFunc(client),
 	}
 
@@ -121,7 +121,7 @@ func NewDevServiceWithTLSConfig(serviceName string, logger *log.Logger,
 	s := &Service{
 		service: serviceName,
 		logger:  logger,
-		tlsCfg:  newDynamicTLSConfig(tlsCfg),
+		tlsCfg:  newDynamicTLSConfig(tlsCfg, logger),
 	}
 	return s, nil
 }

connect/tls.go

@@ -15,6 +15,30 @@ import (
 	"github.com/hashicorp/consul/api"
 )
 
+// parseLeafX509Cert will parse an X509 certificate
+// from the TLS certificate and store the parsed
+// value in the TLS certificate as the Leaf field.
+func parseLeafX509Cert(leaf *tls.Certificate) error {
+	if leaf == nil {
+		// nothing to parse for nil cert
+		return nil
+	}
+
+	if leaf.Leaf != nil {
+		// leaf cert was already parsed
+		return nil
+	}
+
+	cert, err := x509.ParseCertificate(leaf.Certificate[0])
+
+	if err != nil {
+		return err
+	}
+
+	leaf.Leaf = cert
+	return nil
+}
+
 // verifierFunc is a function that can accept rawCertificate bytes from a peer
 // and verify them against a given tls.Config. It's called from the
 // tls.Config.VerifyPeerCertificate hook.
@@ -267,12 +291,17 @@ type tlsCfgUpdate struct {
 // newDynamicTLSConfig returns a dynamicTLSConfig constructed from base.
 // base.Certificates[0] is used as the initial leaf and base.RootCAs is used as
 // the initial roots.
-func newDynamicTLSConfig(base *tls.Config) *dynamicTLSConfig {
+func newDynamicTLSConfig(base *tls.Config, logger *log.Logger) *dynamicTLSConfig {
 	cfg := &dynamicTLSConfig{
 		base: base,
 	}
 	if len(base.Certificates) > 0 {
 		cfg.leaf = &base.Certificates[0]
+		// If this does error then future calls to Ready will fail
+		// It is better to handle not-Ready rather than failing
+		if err := parseLeafX509Cert(cfg.leaf); err != nil && logger != nil {
+			logger.Printf("[ERR] Error parsing configured leaf certificate: %v", err)
+		}
 	}
 	if base.RootCAs != nil {
 		cfg.roots = base.RootCAs
@@ -334,19 +363,39 @@ func (cfg *dynamicTLSConfig) SetRoots(roots *x509.CertPool) error {
 func (cfg *dynamicTLSConfig) SetLeaf(leaf *tls.Certificate) error {
 	cfg.Lock()
 	defer cfg.Unlock()
+	if err := parseLeafX509Cert(leaf); err != nil {
+		return err
+	}
 	cfg.leaf = leaf
+
 	cfg.notify()
 	return nil
 }
 
 // notify is called under lock during an update to check if we are now ready.
 func (cfg *dynamicTLSConfig) notify() {
-	if cfg.readyCh != nil && cfg.leaf != nil && cfg.roots != nil {
+	if cfg.readyCh != nil && cfg.leaf != nil && cfg.roots != nil && cfg.leaf.Leaf != nil {
 		close(cfg.readyCh)
 		cfg.readyCh = nil
 	}
 }
 
+func (cfg *dynamicTLSConfig) VerifyLeafWithRoots() error {
+	cfg.RLock()
+	defer cfg.RUnlock()
+
+	if cfg.roots == nil {
+		return fmt.Errorf("No roots are set")
+	} else if cfg.leaf == nil {
+		return fmt.Errorf("No leaf certificate is set")
+	} else if cfg.leaf.Leaf == nil {
+		return fmt.Errorf("Leaf certificate has not been parsed")
+	}
+
+	_, err := cfg.leaf.Leaf.Verify(x509.VerifyOptions{Roots: cfg.roots})
+	return err
+}
+
 // Roots returns the current CA root CertPool.
 func (cfg *dynamicTLSConfig) Roots() *x509.CertPool {
 	cfg.RLock()
@@ -364,19 +413,19 @@ func (cfg *dynamicTLSConfig) Leaf() *tls.Certificate {
 // Ready returns whether or not both roots and a leaf certificate are
 // configured. If both are non-nil, they are assumed to be valid and usable.
 func (cfg *dynamicTLSConfig) Ready() bool {
-	cfg.RLock()
-	defer cfg.RUnlock()
-	return cfg.leaf != nil && cfg.roots != nil
+	// not locking because VerifyLeafWithRoots will do that
+	return cfg.VerifyLeafWithRoots() == nil
 }
 
 // ReadyWait returns a chan that is closed when the the Service becomes ready
 // for use for the first time. Note that if the Service is ready when it is
 // called it returns a nil chan. Ready means that it has root and leaf
-// certificates configured which we assume are valid. The service may
-// subsequently stop being "ready" if it's certificates expire or are revoked
-// and an error prevents new ones being loaded but this method will not stop
-// returning a nil chan in that case. It is only useful for initial startup. For
-// ongoing health Ready() should be used.
+// certificates configured but not that the combination is valid nor that
+// the current time is within the validity window of the certificate. The
+// service may subsequently stop being "ready" if it's certificates expire
+// or are revoked and an error prevents new ones from being loaded but this
+// method will not stop returning a nil chan in that case. It is only useful
+// for initial startup. For ongoing health Ready() should be used.
 func (cfg *dynamicTLSConfig) ReadyWait() <-chan struct{} {
 	return cfg.readyCh
 }

connect/tls_test.go

@@ -299,7 +299,7 @@ func TestDynamicTLSConfig(t *testing.T) {
 	baseCfg := TestTLSConfig(t, "web", ca1)
 	newCfg := TestTLSConfig(t, "web", ca2)
 
-	c := newDynamicTLSConfig(baseCfg)
+	c := newDynamicTLSConfig(baseCfg, nil)
 
 	// Should set them from the base config
 	require.Equal(c.Leaf(), &baseCfg.Certificates[0])
@@ -365,7 +365,7 @@ func TestDynamicTLSConfig_Ready(t *testing.T) {
 	ca1 := connect.TestCA(t, nil)
 	baseCfg := TestTLSConfig(t, "web", ca1)
 
-	c := newDynamicTLSConfig(defaultTLSConfig())
+	c := newDynamicTLSConfig(defaultTLSConfig(), nil)
 	readyCh := c.ReadyWait()
 	assertBlocked(t, readyCh)
 	require.False(c.Ready(), "no roots or leaf, should not be ready")
@@ -379,6 +379,17 @@ func TestDynamicTLSConfig_Ready(t *testing.T) {
 	require.NoError(err)
 	assertNotBlocked(t, readyCh)
 	require.True(c.Ready(), "should be ready")
+
+	ca2 := connect.TestCA(t, nil)
+	ca2cfg := TestTLSConfig(t, "web", ca2)
+
+	require.NoError(c.SetRoots(ca2cfg.RootCAs))
+	assertNotBlocked(t, readyCh)
+	require.False(c.Ready(), "invalid leaf, should not be ready")
+
+	require.NoError(c.SetRoots(baseCfg.RootCAs))
+	assertNotBlocked(t, readyCh)
+	require.True(c.Ready(), "should be ready")
 }
 
 func assertBlocked(t *testing.T, ch <-chan struct{}) {