address feedback

cert/ca.go

@@ -24,37 +24,40 @@ func NewCAPool() *NebulaCAPool {
 
 // 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 unsupported certificates, they will generate warnings
+// in the []error return arg.
 // 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) {
+func NewCAPoolFromBytes(caPEMs []byte) (*NebulaCAPool, []error, error) {
 	pool := NewCAPool()
 	var err error
+	var warnings []error
 	var expired bool
-	var caTooNew bool
 	for {
 		caPEMs, err = pool.AddCACertificate(caPEMs)
 		if errors.Is(err, ErrExpired) {
 			expired = true
 			err = nil
 		} else if errors.Is(err, ErrInvalidPEMCertificateUnsupported) {
-			caTooNew = true
+			warnings = append(warnings, err)
 			err = nil
 		}
 		if err != nil {
-			return nil, err
+			return nil, warnings, err
 		}
 		if len(caPEMs) == 0 || strings.TrimSpace(string(caPEMs)) == "" {
 			break
 		}
 	}
-
+	if len(pool.CAs) == 0 {
+		//this is outside of cert.NewCAPoolFromBytes so we can warn the user about present-but-unsupported certs first
+		return nil, warnings, errors.New("no valid CA certificates present")
+	}
 	if expired {
-		return pool, ErrExpired
-	} else if caTooNew {
-		return pool, ErrInvalidPEMCertificateUnsupported
+		return pool, warnings, ErrExpired
 	}
 
-	return pool, nil
+	return pool, warnings, nil
 }
 
 // AddCACertificate verifies a Nebula CA certificate and adds it to the pool

cert/cert.go

@@ -165,7 +165,7 @@ func UnmarshalNebulaCertificateFromPEM(b []byte) (*NebulaCertificate, []byte, er
 		return nil, r, fmt.Errorf("input did not contain a valid PEM encoded block")
 	}
 	if p.Type == CertificateV2Banner {
-		return nil, r, ErrInvalidPEMCertificateUnsupported
+		return nil, r, fmt.Errorf("%w: %s", ErrInvalidPEMCertificateUnsupported, p.Type)
 	}
 	if p.Type != CertBanner {
 		return nil, r, fmt.Errorf("bytes did not contain a proper nebula certificate banner")

cert/cert_test.go

@@ -5,6 +5,7 @@ import (
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -599,36 +600,42 @@ CmYKEG5lYnVsYSBQMjU2IHRlc3Qo4s+7mgYw4tXrsAc6QQRkaW2jFmllYvN4+/k2
 		},
 	}
 
-	p, err := NewCAPoolFromBytes([]byte(noNewLines))
+	p, warn, err := NewCAPoolFromBytes([]byte(noNewLines))
 	assert.Nil(t, err)
+	assert.Nil(t, warn)
 	assert.Equal(t, p.CAs[string("c9bfaf7ce8e84b2eeda2e27b469f4b9617bde192efd214b68891ecda6ed49522")].Details.Name, rootCA.Details.Name)
 	assert.Equal(t, p.CAs[string("5c9c3f23e7ee7fe97637cbd3a0a5b854154d1d9aaaf7b566a51f4a88f76b64cd")].Details.Name, rootCA01.Details.Name)
 
-	pp, err := NewCAPoolFromBytes([]byte(withNewLines))
+	pp, warn, err := NewCAPoolFromBytes([]byte(withNewLines))
 	assert.Nil(t, err)
+	assert.Nil(t, warn)
 	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))
+	ppp, warn, err := NewCAPoolFromBytes([]byte(expired))
+	assert.Nil(t, warn)
 	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...))
+	pppp, warn, err := NewCAPoolFromBytes(append([]byte(expired), noNewLines...))
+	assert.Nil(t, warn)
 	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)
 
-	ppppp, err := NewCAPoolFromBytes([]byte(p256))
+	ppppp, warn, err := NewCAPoolFromBytes([]byte(p256))
 	assert.Nil(t, err)
+	assert.Nil(t, warn)
 	assert.Equal(t, ppppp.CAs[string("a7938893ec8c4ef769b06d7f425e5e46f7a7f5ffa49c3bcf4a86b608caba9159")].Details.Name, rootCAP256.Details.Name)
 	assert.Equal(t, len(ppppp.CAs), 1)
 
-	pppppp, err := NewCAPoolFromBytes(append([]byte(p256), []byte(v2)...))
-	assert.Equal(t, err, ErrInvalidPEMCertificateUnsupported)
+	pppppp, warn, err := NewCAPoolFromBytes(append([]byte(p256), []byte(v2)...))
+	assert.Nil(t, err)
+	assert.True(t, errors.Is(warn[0], ErrInvalidPEMCertificateUnsupported))
 	assert.Equal(t, pppppp.CAs[string("a7938893ec8c4ef769b06d7f425e5e46f7a7f5ffa49c3bcf4a86b608caba9159")].Details.Name, rootCAP256.Details.Name)
 	assert.Equal(t, len(pppppp.CAs), 1)
 }

pki.go

@@ -223,7 +223,10 @@ func loadCAPoolFromConfig(l *logrus.Logger, c *config.C) (*cert.NebulaCAPool, er
 		}
 	}
 
-	caPool, err := cert.NewCAPoolFromBytes(rawCA)
+	caPool, warnings, err := cert.NewCAPoolFromBytes(rawCA)
+	for _, w := range warnings {
+		l.WithError(w).Warn("parsing a CA certificate failed")
+	}
 	if errors.Is(err, cert.ErrExpired) {
 		var expired int
 		for _, crt := range caPool.CAs {
@@ -236,9 +239,6 @@ func loadCAPoolFromConfig(l *logrus.Logger, c *config.C) (*cert.NebulaCAPool, er
 		if expired >= len(caPool.CAs) {
 			return nil, errors.New("no valid CA certificates present")
 		}
-
-	} else if errors.Is(err, cert.ErrInvalidPEMCertificateUnsupported) {
-		l.WithError(err).Warn("At least one configured CA is unsupported by this version of nebula. It has been ignored.")
 	} else if err != nil {
 		return nil, fmt.Errorf("error while adding CA certificate to CA trust store: %s", err)
 	}