From 5bf666dd9048038dfe3f33188159283beebaae11 Mon Sep 17 00:00:00 2001 From: John Maguire Date: Fri, 16 Oct 2026 16:28:37 -0400 Subject: [PATCH] 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 | 1 + pki.go | 23 +++++++++++++++++++---- pki_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/examples/config.yml b/examples/config.yml index 03ce96f1..b2460778 100644 --- a/examples/config.yml +++ b/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. diff --git a/pki.go b/pki.go index 7956ee4a..cb03f11e 100644 --- a/pki.go +++ b/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 diff --git a/pki_test.go b/pki_test.go index 7f05b103..dc94eb01 100644 --- a/pki_test.go +++ b/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)) +}