diff --git a/cert/ca.go b/cert/ca.go index 331bd65df..a32139b88 100644 --- a/cert/ca.go +++ b/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 diff --git a/cert/cert.go b/cert/cert.go index 8d90ffe60..3cb50ddb6 100644 --- a/cert/cert.go +++ b/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") diff --git a/cert/cert_test.go b/cert/cert_test.go index b316e7447..dcb0b7554 100644 --- a/cert/cert_test.go +++ b/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) } diff --git a/pki.go b/pki.go index e3fd5e0c2..d5d806c6f 100644 --- a/pki.go +++ b/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) }