Treat uppercase blocked fingerprints the same as lowercase (#1964)

- Error for invalid on startup 
- Still block the valid blocklist entries when a reload has a bad one

examples/config.yml

@@ -8,6 +8,7 @@ pki:
   cert: /etc/nebula/host.crt
   key: /etc/nebula/host.key
   # blocklist is a list of certificate fingerprints that we will refuse to talk to
+  # Entries are the hex fingerprints shown by `nebula-cert print`, an invalid entry fails startup and reload.
   #blocklist:
   #  - c99d4e650533b92061b09918e838a5a0a6aaee21eed1d12fd937682865936c72
   # disconnect_invalid is a toggle to force a client to be disconnected if the certificate is expired or invalid.

pki.go

@@ -2,7 +2,9 @@ package nebula
 
 import (
 	"crypto/fips140"
+	"crypto/sha256"
 	"encoding/binary"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -233,12 +235,14 @@ func (p *PKI) reloadCerts(c *config.C, initial bool) *util.ContextualError {
 
 func (p *PKI) reloadCAPool(c *config.C) *util.ContextualError {
 	caPool, err := loadCAPoolFromConfig(p.l, c)
+	if caPool != nil {
+		p.caPool.Store(caPool)
+		p.l.Debug("Trusted CA fingerprints", "fingerprints", caPool.GetFingerprints())
+	}
 	if err != nil {
 		return util.NewContextualError("Failed to load ca from config", nil, err)
 	}
 
-	p.caPool.Store(caPool)
-	p.l.Debug("Trusted CA fingerprints", "fingerprints", caPool.GetFingerprints())
 	return nil
 }
 
@@ -613,11 +617,22 @@ func loadCAPoolFromConfig(l *slog.Logger, c *config.C) (*cert.CAPool, error) {
 
 	bl := c.GetStringSlice("pki.blocklist", []string{})
 	if len(bl) > 0 {
-		for _, fp := range bl {
+		var bad []string
+		for i, raw := range bl {
+			// Fingerprints are compared as lowercase hex strings, a malformed entry would silently block nothing
+			fp := strings.ToLower(strings.TrimSpace(raw))
+			if b, err := hex.DecodeString(fp); err != nil || len(b) != sha256.Size {
+				bad = append(bad, fmt.Sprintf("entry %d %q", i+1, raw))
+				continue
+			}
 			caPool.BlocklistFingerprint(fp)
 		}
 
-		l.Info("Blocklisted certificates", "fingerprintCount", len(bl))
+		l.Info("Blocklisted certificates", "fingerprintCount", len(bl)-len(bad))
+		if len(bad) > 0 {
+			// The valid entries are still returned so a failed reload blocks them, startup refuses the error
+			return caPool, fmt.Errorf("pki.blocklist entries are not hex certificate fingerprints: %s", strings.Join(bad, ", "))
+		}
 	}
 
 	return caPool, nil

pki_test.go

@@ -217,3 +217,46 @@ func TestPKIReloadChachapolyInFIPSOnlyFails(t *testing.T) {
 	assert.True(t, res.Failed)
 	assert.False(t, res.RestartRequired)
 }
+
+func TestLoadCAPoolBlocklist(t *testing.T) {
+	ca, _, caKey, caPEM := cert_test.NewTestCaCert(cert.Version2, cert.Curve_CURVE25519, time.Time{}, time.Time{}, nil, nil, nil)
+	crt, _, _, _ := cert_test.NewTestCert(cert.Version2, cert.Curve_CURVE25519, ca, caKey, "host", time.Time{}, time.Time{}, []netip.Prefix{netip.MustParsePrefix("10.1.0.2/24")}, nil, nil)
+	fp, err := crt.Fingerprint()
+	require.NoError(t, err)
+
+	load := func(entry string) (*cert.CAPool, error) {
+		c := config.NewC(test.NewLogger())
+		require.NoError(t, c.LoadString(fmt.Sprintf("pki:\n  ca: %q\n  blocklist:\n    - %q\n", caPEM, entry)))
+		return loadCAPoolFromConfig(test.NewLogger(), c)
+	}
+
+	for _, entry := range []string{fp, strings.ToUpper(fp), " " + fp + "\t"} {
+		pool, err := load(entry)
+		require.NoError(t, err, entry)
+		assert.True(t, pool.IsBlocklisted(fp), entry)
+	}
+
+	for _, entry := range []string{fp[1:], fp + "00", "sha256:" + fp, "zz" + fp[2:], ""} {
+		_, err := load(entry)
+		require.Error(t, err, entry)
+	}
+
+	// A bad entry on reload fails it, the valid entries are still blocked
+	_, _, keyPEM, certPEM := cert_test.NewTestCert(cert.Version2, cert.Curve_CURVE25519, ca, caKey, "me", time.Time{}, time.Time{}, []netip.Prefix{netip.MustParsePrefix("10.1.0.1/24")}, nil, nil)
+	pkiConfig := func(entry string) string {
+		return fmt.Sprintf("pki:\n  ca: %q\n  cert: %q\n  key: %q\n  blocklist:\n    - %q\n", caPEM, certPEM, keyPEM, entry)
+	}
+	c := config.NewC(test.NewLogger())
+	require.NoError(t, c.LoadString(pkiConfig(fp)))
+	p, err := NewPKIFromConfig(test.NewLogger(), c)
+	require.NoError(t, err)
+
+	other, _, _, _ := cert_test.NewTestCert(cert.Version2, cert.Curve_CURVE25519, ca, caKey, "other", time.Time{}, time.Time{}, []netip.Prefix{netip.MustParsePrefix("10.1.0.3/24")}, nil, nil)
+	otherFp, err := other.Fingerprint()
+	require.NoError(t, err)
+
+	res := c.ReloadConfigString(pkiConfig(fp) + fmt.Sprintf("    - %q\n    - %q\n", otherFp, fp[1:]))
+	assert.True(t, res.Failed)
+	assert.True(t, p.GetCAPool().IsBlocklisted(fp))
+	assert.True(t, p.GetCAPool().IsBlocklisted(otherFp))
+}