prober: cache CRLs across TLS probes (#21286)

The TLS probe fetched and parsed the leaf certificate's CRL on every
run. That is fine when the CRL is small, but some CAs publish CRLs of
several megabytes: the one for the AWS ACM R2M04 intermediate is about
2.5MB, which at the default 15s interval is a continuous 170kB/s per
probed node.

Cache parsed CRLs by distribution point URL and reuse each for up to
an hour, or until its NextUpdate if that comes first. An hour is the
HTTP max-age Let's Encrypt serves on its root CRL. A CRL is cached
only after its signature verifies, every use still re-verifies it
against the probing leaf's issuer (the cache is keyed by URL alone),
and a CRL without a NextUpdate is never cached since it declares no
validity window.

Concurrent probes fetch through singleflight.DoChanContext to avoid
re-fetching a single CRL, with each waiter keeping its own deadline. A
caller that missed the cache re-checks it inside the singleflight
closure, since singleflight dedupes only calls that overlap.

Leaf certificates whose issuer is missing from the presented chain now
fail before any fetch. Previously the probe downloaded the CRL and then
panicked in CheckSignatureFrom, which the prober recovered and recorded
as a probe failure.

Also update the TLS probe's doc comments, which said OCSP where the
code checks a CRL.

Fixes #21310

Signed-off-by: Thomas Desrosiers <git@hive.pw>

prober/tls.go

@@ -13,7 +13,11 @@
 	"net/http"
 	"net/netip"
 	"slices"
+	"sync"
 	"time"
+
+	"tailscale.com/util/mak"
+	"tailscale.com/util/singleflight"
 )
 
 const expiresSoon = 7 * 24 * time.Hour // 7 days from now
@@ -26,7 +30,7 @@
 //
 // The ProbeFunc connects to a hostPort (host:port string), does a TLS
 // handshake, verifies that the hostname matches the presented certificate,
-// checks certificate validity time and OCSP revocation status.
+// checks certificate validity time and CRL revocation status.
 //
 // The TLS config is optional and may be nil.
 func TLS(hostPort string, config *tls.Config) ProbeClass {
@@ -59,13 +63,13 @@ func probeTLS(ctx context.Context, config *tls.Config, dialHostPort string) erro
 	defer conn.Close()
 
 	tlsConnState := conn.(*tls.Conn).ConnectionState()
-	return validateConnState(ctx, &tlsConnState)
+	return validateConnState(ctx, defaultCRLCache, &tlsConnState)
 }
 
 // validateConnState verifies certificate validity time in all certificates
-// returned by the TLS server and checks OCSP revocation status for the
+// returned by the TLS server and checks CRL revocation status for the
 // leaf cert.
-func validateConnState(ctx context.Context, cs *tls.ConnectionState) (returnerr error) {
+func validateConnState(ctx context.Context, crls *crlCache, cs *tls.ConnectionState) (returnerr error) {
 	var errs []error
 	defer func() {
 		returnerr = errors.Join(errs...)
@@ -117,37 +121,45 @@ func validateConnState(ctx context.Context, cs *tls.ConnectionState) (returnerr
 		return
 	}
 
-	err := checkCertCRL(ctx, leafCert.CRLDistributionPoints[0], leafCert, issuerCert)
+	err := crls.checkCertCRL(ctx, leafCert.CRLDistributionPoints[0], leafCert, issuerCert)
 	if err != nil {
 		errs = append(errs, fmt.Errorf("CRL verification failed for %v: %w", leafCert.Subject, err))
 	}
 	return
 }
 
-func checkCertCRL(ctx context.Context, crlURL string, leafCert, issuerCert *x509.Certificate) error {
-	hreq, err := http.NewRequestWithContext(ctx, "GET", crlURL, nil)
-	if err != nil {
-		return fmt.Errorf("could not create CRL GET request: %w", err)
+var defaultCRLCache = &crlCache{now: time.Now}
+
+// crlRefreshInterval caps how long a cached CRL is reused, even when its
+// NextUpdate (the x509.RevocationList field giving the deadline for the
+// issuer's next CRL) is further out. An hour was chosen based on the
+// Cache-Control max-age Let's Encrypt serves on its root CRL.
+const crlRefreshInterval = time.Hour
+
+// crlCache caches parsed CRLs by distribution point URL, so that repeated
+// probes do not refetch one on every run.
+type crlCache struct {
+	now   func() time.Time
+	fetch singleflight.Group[string, *x509.RevocationList]
+
+	mu   sync.Mutex
+	crls map[string]crlCacheEntry
+}
+
+type crlCacheEntry struct {
+	crl       *x509.RevocationList
+	refreshAt time.Time
+}
+
+func (c *crlCache) checkCertCRL(ctx context.Context, crlURL string, leafCert, issuerCert *x509.Certificate) error {
+	if issuerCert == nil {
+		return fmt.Errorf("issuer certificate for %v not in presented chain", leafCert.Subject)
 	}
-	hresp, err := http.DefaultClient.Do(hreq)
-	if err != nil {
-		return fmt.Errorf("CRL request failed: %w", err)
-	}
-	defer hresp.Body.Close()
-	if hresp.StatusCode != http.StatusOK {
-		return fmt.Errorf("crl: non-200 status code from CRL server: %s", hresp.Status)
-	}
-	lr := io.LimitReader(hresp.Body, 10<<20) // 10MB
-	crlB, err := io.ReadAll(lr)
+	crl, err := c.get(ctx, crlURL, issuerCert)
 	if err != nil {
 		return err
 	}
 
-	crl, err := x509.ParseRevocationList(crlB)
-	if err != nil {
-		return fmt.Errorf("could not parse CRL: %w", err)
-	}
-
 	if err := crl.CheckSignatureFrom(issuerCert); err != nil {
 		return fmt.Errorf("could not verify CRL signature: %w", err)
 	}
@@ -160,3 +172,88 @@ func checkCertCRL(ctx context.Context, crlURL string, leafCert, issuerCert *x509
 
 	return nil
 }
+
+// get returns the CRL at crlURL, refetching it once the cached copy is due
+// for a refresh. A CRL is cached only once it verifies against the fetching
+// caller's issuerCert; one with no NextUpdate declares no lifetime and is
+// not cached.
+func (c *crlCache) get(ctx context.Context, crlURL string, issuerCert *x509.Certificate) (*x509.RevocationList, error) {
+	if crl, ok := c.cached(crlURL); ok {
+		return crl, nil
+	}
+
+	res := <-c.fetch.DoChanContext(ctx, crlURL, func(ctx context.Context) (*x509.RevocationList, error) {
+		// A caller that missed the cache above can reach here after another
+		// caller's fetch stored one; singleflight dedupes only overlapping calls.
+		if crl, ok := c.cached(crlURL); ok {
+			return crl, nil
+		}
+		crl, err := fetchCRL(ctx, crlURL)
+		if err != nil {
+			return nil, err
+		}
+		if err := crl.CheckSignatureFrom(issuerCert); err != nil {
+			return nil, fmt.Errorf("could not verify CRL signature: %w", err)
+		}
+		if !crl.NextUpdate.IsZero() {
+			c.store(crlURL, crl)
+		}
+		return crl, nil
+	})
+	return res.Val, res.Err
+}
+
+// cached returns the CRL cached for crlURL, if one is present and not yet due
+// for a refresh.
+func (c *crlCache) cached(crlURL string) (*x509.RevocationList, bool) {
+	c.mu.Lock()
+	e, ok := c.crls[crlURL]
+	c.mu.Unlock()
+	if !ok || !c.now().Before(e.refreshAt) {
+		return nil, false
+	}
+	return e.crl, true
+}
+
+func (c *crlCache) store(crlURL string, crl *x509.RevocationList) {
+	now := c.now()
+	refreshAt := now.Add(crlRefreshInterval)
+	if crl.NextUpdate.Before(refreshAt) {
+		refreshAt = crl.NextUpdate
+	}
+
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	for url, e := range c.crls {
+		if !e.refreshAt.After(now) {
+			delete(c.crls, url)
+		}
+	}
+	mak.Set(&c.crls, crlURL, crlCacheEntry{crl: crl, refreshAt: refreshAt})
+}
+
+func fetchCRL(ctx context.Context, crlURL string) (*x509.RevocationList, error) {
+	hreq, err := http.NewRequestWithContext(ctx, "GET", crlURL, nil)
+	if err != nil {
+		return nil, fmt.Errorf("could not create CRL GET request: %w", err)
+	}
+	hresp, err := http.DefaultClient.Do(hreq)
+	if err != nil {
+		return nil, fmt.Errorf("CRL request failed: %w", err)
+	}
+	defer hresp.Body.Close()
+	if hresp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("crl: non-200 status code from CRL server: %s", hresp.Status)
+	}
+	lr := io.LimitReader(hresp.Body, 10<<20) // 10MB
+	crlB, err := io.ReadAll(lr)
+	if err != nil {
+		return nil, err
+	}
+
+	crl, err := x509.ParseRevocationList(crlB)
+	if err != nil {
+		return nil, fmt.Errorf("could not parse CRL: %w", err)
+	}
+	return crl, nil
+}

prober/tls_test.go

@@ -18,8 +18,12 @@
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
+
+	"tailscale.com/tstest"
 )
 
 var leafCert = x509.Certificate{
@@ -119,7 +123,7 @@ func() *x509.Certificate {
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			cs := &tls.ConnectionState{PeerCertificates: []*x509.Certificate{tt.cert()}}
-			err := validateConnState(context.Background(), cs)
+			err := validateConnState(context.Background(), &crlCache{now: time.Now}, cs)
 			if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
 				t.Errorf("unexpected error %q; want %q", err, tt.wantErr)
 			}
@@ -129,9 +133,11 @@ func() *x509.Certificate {
 
 type CRLServer struct {
 	crlBytes []byte
+	requests atomic.Int32 // total requests served
 }
 
 func (s *CRLServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	s.requests.Add(1)
 	if s.crlBytes == nil {
 		w.WriteHeader(http.StatusInternalServerError)
 		return
@@ -186,7 +192,15 @@ func parseECKey(t *testing.T, pemPriv string) *ecdsa.PrivateKey {
 	return key
 }
 
-func TestCRL(t *testing.T) {
+// crlTestPKI is a CA that can sign CRLs plus a leaf certificate it issued.
+type crlTestPKI struct {
+	caCert *x509.Certificate
+	caKey  *ecdsa.PrivateKey
+	leaf   *x509.Certificate
+}
+
+func newCRLTestPKI(t *testing.T) crlTestPKI {
+	t.Helper()
 	// Generate CA key and self-signed CA cert
 	caKey := parseECKey(t, someECDSAKey1)
 
@@ -218,6 +232,39 @@ func TestCRL(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	return crlTestPKI{caCert: caCert, caKey: caKey, leaf: leafCertParsed}
+}
+
+// crl signs a CRL from the test CA revoking the given serials as of
+// thisUpdate. A zero nextUpdate omits the field and requires a zero
+// thisUpdate, since x509.CreateRevocationList rejects a NextUpdate before
+// ThisUpdate.
+func (p crlTestPKI) crl(t *testing.T, thisUpdate, nextUpdate time.Time, revoked ...*big.Int) []byte {
+	t.Helper()
+	rl := x509.RevocationList{
+		SignatureAlgorithm: p.caCert.SignatureAlgorithm,
+		Issuer:             p.caCert.Subject,
+		ThisUpdate:         thisUpdate,
+		NextUpdate:         nextUpdate,
+		Number:             big.NewInt(1),
+	}
+	for _, serial := range revoked {
+		rl.RevokedCertificateEntries = append(rl.RevokedCertificateEntries, x509.RevocationListEntry{
+			SerialNumber:   serial,
+			RevocationTime: thisUpdate,
+			ReasonCode:     1, // Key compromise
+		})
+	}
+	b, err := x509.CreateRevocationList(rand.Reader, &rl, p.caCert, p.caKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return b
+}
+
+func TestCRL(t *testing.T) {
+	pki := newCRLTestPKI(t)
+	caCert, caKey, leafCertParsed := pki.caCert, pki.caKey, pki.leaf
 
 	// Catch no CRL set by Let's Encrypt date.
 	noCRLCert := leafCert
@@ -243,7 +290,7 @@ func TestCRL(t *testing.T) {
 	// Create a CRL that revokes the leaf cert using x509.CreateRevocationList
 	now := time.Now()
 	revoked := []x509.RevocationListEntry{{
-		SerialNumber:   leaf.SerialNumber,
+		SerialNumber:   leafCertParsed.SerialNumber,
 		RevocationTime: now,
 		ReasonCode:     1, // Key compromise
 	}}
@@ -325,7 +372,9 @@ func TestCRL(t *testing.T) {
 				crlServer.crlBytes = nil
 				tt.cert.CRLDistributionPoints = []string{}
 			}
-			err := validateConnState(context.Background(), cs)
+			// Each subtest needs its own cache: they share one CRL server
+			// URL, which is the cache key, but swap the bytes it serves.
+			err := validateConnState(context.Background(), &crlCache{now: time.Now}, cs)
 
 			if err == nil && tt.wantErr == "" {
 				return
@@ -337,3 +386,152 @@ func TestCRL(t *testing.T) {
 		})
 	}
 }
+
+func TestCRLCache(t *testing.T) {
+	pki := newCRLTestPKI(t)
+	start := time.Date(2026, 9, 1, 12, 0, 0, 0, time.UTC)
+
+	newServer := func(t *testing.T, s *CRLServer) *httptest.Server {
+		srv := httptest.NewServer(s)
+		t.Cleanup(srv.Close)
+		return srv
+	}
+	check := func(c *crlCache, srv *httptest.Server) error {
+		return c.checkCertCRL(t.Context(), srv.URL, pki.leaf, pki.caCert)
+	}
+	mustCheck := func(t *testing.T, c *crlCache, srv *httptest.Server) {
+		t.Helper()
+		if err := check(c, srv); err != nil {
+			t.Fatalf("checkCertCRL: %v", err)
+		}
+	}
+	wantRequests := func(t *testing.T, s *CRLServer, want int32) {
+		t.Helper()
+		if got := s.requests.Load(); got != want {
+			t.Errorf("CRL server saw %d requests; want %d", got, want)
+		}
+	}
+
+	t.Run("ReusedAcrossProbes", func(t *testing.T) {
+		clock := tstest.NewClock(tstest.ClockOpts{Start: start})
+		c := &crlCache{now: clock.Now}
+		s := &CRLServer{crlBytes: pki.crl(t, start, start.Add(7*24*time.Hour))}
+		srv := newServer(t, s)
+
+		for range 5 {
+			mustCheck(t, c, srv)
+			clock.Advance(15 * time.Second)
+		}
+		wantRequests(t, s, 1)
+	})
+
+	t.Run("RefetchedAfterRefreshInterval", func(t *testing.T) {
+		clock := tstest.NewClock(tstest.ClockOpts{Start: start})
+		c := &crlCache{now: clock.Now}
+		s := &CRLServer{crlBytes: pki.crl(t, start, start.Add(7*24*time.Hour))}
+		srv := newServer(t, s)
+
+		mustCheck(t, c, srv)
+		clock.Advance(crlRefreshInterval - time.Minute)
+		mustCheck(t, c, srv)
+		wantRequests(t, s, 1)
+
+		clock.Advance(2 * time.Minute)
+		mustCheck(t, c, srv)
+		wantRequests(t, s, 2)
+	})
+
+	t.Run("RefetchedAfterNextUpdate", func(t *testing.T) {
+		clock := tstest.NewClock(tstest.ClockOpts{Start: start})
+		c := &crlCache{now: clock.Now}
+		s := &CRLServer{crlBytes: pki.crl(t, start, start.Add(crlRefreshInterval/2))}
+		srv := newServer(t, s)
+
+		mustCheck(t, c, srv)
+		clock.Advance(crlRefreshInterval/2 - time.Minute)
+		mustCheck(t, c, srv)
+		wantRequests(t, s, 1)
+
+		clock.Advance(2 * time.Minute)
+		mustCheck(t, c, srv)
+		wantRequests(t, s, 2)
+	})
+
+	t.Run("ConcurrentCallersShareOneFetch", func(t *testing.T) {
+		clock := tstest.NewClock(tstest.ClockOpts{Start: start})
+		c := &crlCache{now: clock.Now}
+		s := &CRLServer{crlBytes: pki.crl(t, start, start.Add(7*24*time.Hour))}
+		srv := newServer(t, s)
+
+		var wg sync.WaitGroup
+		errs := make([]error, 8)
+		for i := range errs {
+			wg.Go(func() {
+				errs[i] = check(c, srv)
+			})
+		}
+		wg.Wait()
+		for i, err := range errs {
+			if err != nil {
+				t.Errorf("caller %d: %v", i, err)
+			}
+		}
+		wantRequests(t, s, 1)
+	})
+
+	t.Run("RevokedFromCache", func(t *testing.T) {
+		clock := tstest.NewClock(tstest.ClockOpts{Start: start})
+		c := &crlCache{now: clock.Now}
+		s := &CRLServer{crlBytes: pki.crl(t, start, start.Add(7*24*time.Hour), pki.leaf.SerialNumber)}
+		srv := newServer(t, s)
+
+		for range 3 {
+			err := check(c, srv)
+			if err == nil || !strings.Contains(err.Error(), "has been revoked on") {
+				t.Fatalf("unexpected error %q; want revoked", err)
+			}
+			clock.Advance(15 * time.Second)
+		}
+		wantRequests(t, s, 1)
+	})
+
+	t.Run("WrongIssuerNotCached", func(t *testing.T) {
+		clock := tstest.NewClock(tstest.ClockOpts{Start: start})
+		c := &crlCache{now: clock.Now}
+		s := &CRLServer{crlBytes: pki.crl(t, start, start.Add(7*24*time.Hour))}
+		srv := newServer(t, s)
+
+		// The leaf is not a CA, so it cannot have signed the CRL.
+		err := c.checkCertCRL(t.Context(), srv.URL, pki.leaf, pki.leaf)
+		if err == nil || !strings.Contains(err.Error(), "could not verify CRL signature") {
+			t.Fatalf("unexpected error %q; want signature failure", err)
+		}
+		wantRequests(t, s, 1)
+
+		mustCheck(t, c, srv)
+		wantRequests(t, s, 2)
+	})
+
+	t.Run("NoNextUpdateNotCached", func(t *testing.T) {
+		c := &crlCache{now: tstest.NewClock(tstest.ClockOpts{Start: start}).Now}
+		s := &CRLServer{crlBytes: pki.crl(t, time.Time{}, time.Time{})}
+		srv := newServer(t, s)
+
+		for range 3 {
+			mustCheck(t, c, srv)
+		}
+		wantRequests(t, s, 3)
+	})
+
+	t.Run("NilIssuerReturnsError", func(t *testing.T) {
+		c := &crlCache{now: tstest.NewClock(tstest.ClockOpts{Start: start}).Now}
+		s := &CRLServer{crlBytes: pki.crl(t, start, start.Add(7*24*time.Hour))}
+		srv := newServer(t, s)
+
+		err := c.checkCertCRL(t.Context(), srv.URL, pki.leaf, nil)
+		if err == nil || !strings.Contains(err.Error(), "not in presented chain") {
+			t.Fatalf("unexpected error %q; want missing issuer", err)
+		}
+		wantRequests(t, s, 0)
+	})
+}