Allow setup of a CA pool from bytes that contain expired certs (#599)

Co-authored-by: Nate Brown <nbrown.us@gmail.com>

cert.go

@@ -124,19 +124,13 @@ func loadCAFromConfig(l *logrus.Logger, c *config.C) (*cert.NebulaCAPool, error)
 	var err error
 
 	caPathOrPEM := c.GetString("pki.ca", "")
-	if caPathOrPEM == "" {
-		// Support backwards compat with the old x509
-		//TODO: remove after this is rolled out everywhere - NB 2018/02/23
-		caPathOrPEM = c.GetString("x509.ca", "")
-	}
-
 	if caPathOrPEM == "" {
 		return nil, errors.New("no pki.ca path or PEM data provided")
 	}
 
 	if strings.Contains(caPathOrPEM, "-----BEGIN") {
 		rawCA = []byte(caPathOrPEM)
-		caPathOrPEM = "<inline>"
+
 	} else {
 		rawCA, err = ioutil.ReadFile(caPathOrPEM)
 		if err != nil {
@@ -145,7 +139,20 @@ func loadCAFromConfig(l *logrus.Logger, c *config.C) (*cert.NebulaCAPool, error)
 	}
 
 	CAs, err := cert.NewCAPoolFromBytes(rawCA)
-	if err != nil {
+	if errors.Is(err, cert.ErrExpired) {
+		var expired int
+		for _, cert := range CAs.CAs {
+			if cert.Expired(time.Now()) {
+				expired++
+				l.WithField("cert", cert).Warn("expired certificate present in CA pool")
+			}
+		}
+
+		if expired >= len(CAs.CAs) {
+			return nil, errors.New("no valid CA certificates present")
+		}
+
+	} else if err != nil {
 		return nil, fmt.Errorf("error while adding CA certificate to CA trust store: %s", err)
 	}
 
@@ -154,7 +161,8 @@ func loadCAFromConfig(l *logrus.Logger, c *config.C) (*cert.NebulaCAPool, error)
 		CAs.BlocklistFingerprint(fp)
 	}
 
-	// Support deprecated config for at leaast one minor release to allow for migrations
+	// Support deprecated config for at least one minor release to allow for migrations
+	//TODO: remove in 2022 or later
 	for _, fp := range c.GetStringSlice("pki.blacklist", []string{}) {
 		l.WithField("fingerprint", fp).Infof("Blocklisting cert")
 		l.Warn("pki.blacklist is deprecated and will not be supported in a future release. Please migrate your config to use pki.blocklist")

cert/ca.go

@@ -1,6 +1,7 @@
 package cert
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -21,19 +22,32 @@ func NewCAPool() *NebulaCAPool {
 	return &ca
 }
 
+// NewCAPoolFromBytes will create a new CA pool from the provided
+// input bytes, which must be a PEM-encoded set of nebula certificates.
+// If the pool contains any expired certificates, an ErrExpired will be
+// returned along with the pool. The caller must handle any such errors.
 func NewCAPoolFromBytes(caPEMs []byte) (*NebulaCAPool, error) {
 	pool := NewCAPool()
 	var err error
+	var expired bool
 	for {
 		caPEMs, err = pool.AddCACertificate(caPEMs)
+		if errors.Is(err, ErrExpired) {
+			expired = true
+			err = nil
+		}
 		if err != nil {
 			return nil, err
 		}
-		if caPEMs == nil || len(caPEMs) == 0 || strings.TrimSpace(string(caPEMs)) == "" {
+		if len(caPEMs) == 0 || strings.TrimSpace(string(caPEMs)) == "" {
 			break
 		}
 	}
 
+	if expired {
+		return pool, ErrExpired
+	}
+
 	return pool, nil
 }
 
@@ -47,15 +61,11 @@ func (ncp *NebulaCAPool) AddCACertificate(pemBytes []byte) ([]byte, error) {
 	}
 
 	if !c.Details.IsCA {
-		return pemBytes, fmt.Errorf("provided certificate was not a CA; %s", c.Details.Name)
+		return pemBytes, fmt.Errorf("%s: %w", c.Details.Name, ErrNotCA)
 	}
 
 	if !c.CheckSignature(c.Details.PublicKey) {
-		return pemBytes, fmt.Errorf("provided certificate was not self signed; %s", c.Details.Name)
-	}
-
-	if c.Expired(time.Now()) {
-		return pemBytes, fmt.Errorf("provided CA certificate is expired; %s", c.Details.Name)
+		return pemBytes, fmt.Errorf("%s: %w", c.Details.Name, ErrNotSelfSigned)
 	}
 
 	sum, err := c.Sha256Sum()
@@ -64,6 +74,10 @@ func (ncp *NebulaCAPool) AddCACertificate(pemBytes []byte) ([]byte, error) {
 	}
 
 	ncp.CAs[sum] = c
+	if c.Expired(time.Now()) {
+		return pemBytes, fmt.Errorf("%s: %w", c.Details.Name, ErrExpired)
+	}
+
 	return pemBytes, nil
 }
 

cert/cert_test.go

@@ -429,6 +429,15 @@ BVG+oJpAoqokUBbI4U0N8CSfpUABEkB/Pm5A2xyH/nc8mg/wvGUWG3pZ7nHzaDMf
 8/phAUt+FLzqTECzQKisYswKvE3pl9mbEYKbOdIHrxdIp95mo4sF
 -----END NEBULA CERTIFICATE-----
 
+`
+
+	expired := `
+# expired certificate
+-----BEGIN NEBULA CERTIFICATE-----
+CjkKB2V4cGlyZWQouPmWjQYwufmWjQY6ILCRaoCkJlqHgv5jfDN4lzLHBvDzaQm4
+vZxfu144hmgjQAESQG4qlnZi8DncvD/LDZnLgJHOaX1DWCHHEh59epVsC+BNgTie
+WH1M9n4O7cFtGlM6sJJOS+rCVVEJ3ABS7+MPdQs=
+-----END NEBULA CERTIFICATE-----
 `
 
 	rootCA := NebulaCertificate{
@@ -452,6 +461,19 @@ BVG+oJpAoqokUBbI4U0N8CSfpUABEkB/Pm5A2xyH/nc8mg/wvGUWG3pZ7nHzaDMf
 	assert.Nil(t, err)
 	assert.Equal(t, pp.CAs[string("c9bfaf7ce8e84b2eeda2e27b469f4b9617bde192efd214b68891ecda6ed49522")].Details.Name, rootCA.Details.Name)
 	assert.Equal(t, pp.CAs[string("5c9c3f23e7ee7fe97637cbd3a0a5b854154d1d9aaaf7b566a51f4a88f76b64cd")].Details.Name, rootCA01.Details.Name)
+
+	// expired cert, no valid certs
+	ppp, err := NewCAPoolFromBytes([]byte(expired))
+	assert.Equal(t, ErrExpired, err)
+	assert.Equal(t, ppp.CAs[string("152070be6bb19bc9e3bde4c2f0e7d8f4ff5448b4c9856b8eccb314fade0229b0")].Details.Name, "expired")
+
+	// expired cert, with valid certs
+	pppp, err := NewCAPoolFromBytes(append([]byte(expired), noNewLines...))
+	assert.Equal(t, ErrExpired, err)
+	assert.Equal(t, pppp.CAs[string("c9bfaf7ce8e84b2eeda2e27b469f4b9617bde192efd214b68891ecda6ed49522")].Details.Name, rootCA.Details.Name)
+	assert.Equal(t, pppp.CAs[string("5c9c3f23e7ee7fe97637cbd3a0a5b854154d1d9aaaf7b566a51f4a88f76b64cd")].Details.Name, rootCA01.Details.Name)
+	assert.Equal(t, pppp.CAs[string("152070be6bb19bc9e3bde4c2f0e7d8f4ff5448b4c9856b8eccb314fade0229b0")].Details.Name, "expired")
+	assert.Equal(t, len(pppp.CAs), 3)
 }
 
 func appendByteSlices(b ...[]byte) []byte {

cert/errors.go

@@ -0,0 +1,9 @@
+package cert
+
+import "errors"
+
+var (
+	ErrExpired       = errors.New("certificate is expired")
+	ErrNotCA         = errors.New("certificate is not a CA")
+	ErrNotSelfSigned = errors.New("certificate is not self-signed")
+)