observer: ccadb: check reported CRL shards for completeness

observer/probers/ccadb/ccadb.go

@@ -10,15 +10,18 @@ import (
 	"encoding/pem"
 	"errors"
 	"fmt"
-	"github.com/letsencrypt/boulder/observer/probers"
-	"github.com/letsencrypt/boulder/strictyaml"
-	"github.com/prometheus/client_golang/prometheus"
 	"io"
 	"maps"
+	"net/http"
 	"regexp"
 	"slices"
+	"strconv"
 	"time"
 
+	"github.com/letsencrypt/boulder/observer/probers"
+	"github.com/letsencrypt/boulder/strictyaml"
+	"github.com/prometheus/client_golang/prometheus"
+
 	"github.com/letsencrypt/boulder/crl/checker"
 	"github.com/letsencrypt/boulder/crl/idp"
 )
@@ -29,9 +32,7 @@ type CCADBConf struct {
 	CertificatePEMsURL    string `yaml:"certificatePEMsURL"`
 	CAOwner               string `yaml:"caOwner"`
 	CRLAgeLimit           string `yaml:"crlAgeLimit"`
-	// Because this prober fetches URLs controlled by external input (CCADB), we
-	// check this regexp to avoid arbitrary content fetching (SSRF).
-	CRLRegexp string `yaml:"crlRegexp"`
+	CRLRegexp             string `yaml:"crlRegexp"`
 }
 
 // Kind returns a name that uniquely identifies the `Kind` of `Configurer`.
@@ -79,7 +80,7 @@ func (c CCADBConf) MakeProber(collectors map[string]prometheus.Collector) (probe
 		}
 	}
 
-	crlRegexp := `^http://[a-z0-9-]+\.c\.lencr\.org/\d+\.crl$`
+	crlRegexp := `^http://[a-z0-9-]+\.c\.lencr\.org/(?P<crlNumber>\d+)\.crl$`
 	if c.CRLRegexp != "" {
 		crlRegexp = c.CRLRegexp
 	}
@@ -117,16 +118,21 @@ func getIDP(crl *x509.RevocationList) (string, error) {
 	return "", fmt.Errorf("CRL had incorrect number of IssuingDistributionPoint URIs: %s", idps)
 }
 
-// CCADBProber fetches the AllCertificatesRecordsReport from CCADB, filters for a
-// specific CA Owner (defaults to 'Internet Security Research Group'), and
-// fetches all CRLs found.
+// CCADBProber checks the CRLs we report in CCADB for correctness.
+//
+// It determines the CRLs in scope by fetching the AllCertificatesRecordsReport
+// from CCADB, and filtering for a specific CA Owner (defaults to 'Internet
+// Security Research Group').
 //
 // It checks that the CRLs:
 //   - Are not too old
 //   - Have an issuingDistributionPoint that matches the URL from which they
 //     were fetched
 //   - Have a valid signature based on their issuer SKID from CCADB
 //   - Don't have duplicate serial numbers across different CRLs
+//
+// It also checks, heuristically, whether the complete corpus of CRL shards
+// are reported in CCADB.
 type CCADBProber struct {
 	allCertificatesCSVURL string
 	certificatePEMsURL    string
@@ -164,16 +170,33 @@ func (c *CCADBProber) Probe(ctx context.Context) error {
 			continue
 		}
 
+		var (
+			seenCRLShardIndices []int
+			exampleCRLShardURL  string
+		)
 		for _, url := range urls {
 			// This can happen when an issuer is not yet issuing.
 			if url == "" {
 				continue
 			}
 
-			if !c.crlRegexp.MatchString(url) {
+			matches := c.crlRegexp.FindAllStringSubmatch(url, 2)
+			if matches == nil {
 				errs = append(errs, fmt.Errorf("CRL %s does not match regexp %s", url, c.crlRegexp))
 				continue
 			}
+			match := matches[0]
+			if len(match) != 2 {
+				errs = append(errs, fmt.Errorf("CRL %s does not match regexp %s", url, c.crlRegexp))
+				continue
+			}
+			crlNumber, err := strconv.Atoi(match[1])
+			if err != nil {
+				errs = append(errs, fmt.Errorf("cannot parse CRL shard number from %s: %s", url, err))
+				continue
+			}
+			seenCRLShardIndices = append(seenCRLShardIndices, crlNumber)
+			exampleCRLShardURL = url
 
 			crl, err := checkCRL(ctx, url, issuer, c.crlAgeLimit)
 			if err != nil {
@@ -199,6 +222,31 @@ func (c *CCADBProber) Probe(ctx context.Context) error {
 				serials[serialByteString] = crl
 			}
 		}
+
+		if len(seenCRLShardIndices) > 0 {
+			// The max()+1th shard should not be live. This generally occurs if we have
+			// live CRL shards that are not reported in CCADB.
+			maxShardIndex := slices.Max(seenCRLShardIndices)
+			err := checkCRLShardNotFound(ctx, c.crlRegexp, exampleCRLShardURL, maxShardIndex+1)
+			if err != nil {
+				errs = append(errs, err)
+			}
+
+			// The 0th shard should not be live, because shards are 1-indexed.
+			err = checkCRLShardNotFound(ctx, c.crlRegexp, exampleCRLShardURL, 0)
+			if err != nil {
+				errs = append(errs, err)
+			}
+
+			// It is possible that the number of CRL shards shrinks, but it is highly
+			// unlikely, because we would never have any reason make that so. Therefore
+			// we do not detect this case.
+
+			err = checkAllShardIndexesPresent(seenCRLShardIndices)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("issuer %q: %w", issuer.Subject.CommonName, err))
+			}
+		}
 	}
 
 	return errors.Join(errs...)
@@ -243,6 +291,50 @@ func getCSV(ctx context.Context, url string) ([]string, *csv.Reader, error) {
 	return header, reader, nil
 }
 
+func checkCRLShardNotFound(ctx context.Context, re *regexp.Regexp, exampleCRLShardURL string, shardIndex int) error {
+	match := re.FindStringSubmatchIndex(exampleCRLShardURL)
+	if match == nil || len(match) != 4 || match[2] < 0 {
+		return fmt.Errorf("CRL %s does not match regexp %s", exampleCRLShardURL, re)
+	}
+	// There is mild SSRF risk because this input is derived from CCADB controlled
+	// values. This is mitigated by using a stringent regex.
+	url := exampleCRLShardURL[:match[2]] + strconv.Itoa(shardIndex) + exampleCRLShardURL[match[3]:]
+
+	err := httpGetExpectingStatusCode(ctx, url, http.StatusNotFound)
+	if err != nil {
+		return fmt.Errorf("did not get expected status 404 for %s: %w", url, err)
+	}
+	return nil
+}
+
+// checkAllShardIndexesPresent returns an error naming any index in
+// [1..max(seen)] that is absent from seen.
+func checkAllShardIndexesPresent(seen []int) error {
+	if len(seen) == 0 {
+		return nil
+	}
+	seenSet := make(map[int]struct{}, len(seen))
+	for _, index := range seen {
+		seenSet[index] = struct{}{}
+	}
+	var missing []int
+	maxIndex := slices.Max(seen)
+	if maxIndex > 100_000 {
+		// Avoid unbounded allocation via typos in CCADB, e.g. 99999999.crl
+		return fmt.Errorf("CRL corpus is unexpectedly large: %d", maxIndex)
+	}
+	for i := 1; i <= maxIndex; i++ {
+		_, ok := seenSet[i]
+		if !ok {
+			missing = append(missing, i)
+		}
+	}
+	if len(missing) > 0 {
+		return fmt.Errorf("CRL shard indexes %v not reported in CCADB", missing)
+	}
+	return nil
+}
+
 func (c CCADBProber) getAllIntermediates(ctx context.Context) (map[string]*x509.Certificate, error) {
 	certs, err := c.getDecadeIntermediates(ctx, 2010)
 	if err != nil {

observer/probers/ccadb/retryhttp.go

@@ -8,13 +8,17 @@ import (
 	"time"
 )
 
+var backoffSchedule = []int{0, 1000, 1250, 1562, 1953, 2441, 3051, 3814, 4768, 5960, 7450, 9313, 11641}
+
+const userAgent = "letsencrypt/boulder-observer-http-client"
+
 func getBody(ctx context.Context, url string) ([]byte, error) {
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
 	if err != nil {
 		return nil, err
 	}
 
-	req.Header.Set("User-Agent", "CRL-Monitor/0.1")
+	req.Header.Set("User-Agent", userAgent)
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return nil, err
@@ -39,9 +43,8 @@ func getBody(ctx context.Context, url string) ([]byte, error) {
 
 // httpGet is a simple wrapper around http.Client.Do that will retry on a fixed backoff schedule
 func httpGet(ctx context.Context, url string) ([]byte, error) {
-	// A fixed exponential backoff schedule.
 	var err error
-	for _, backoff := range []int{0, 1000, 1250, 1562, 1953, 2441, 3051, 3814, 4768, 5960, 7450, 9313, 11641} {
+	for _, backoff := range backoffSchedule {
 		select {
 		case <-ctx.Done():
 			return nil, ctx.Err()
@@ -57,3 +60,40 @@ func httpGet(ctx context.Context, url string) ([]byte, error) {
 	}
 	return nil, err
 }
+
+// httpGetExpectingStatusCode fetches the given URL and returns nil if the given status code matches
+// the returned status code. It retries on a fixed backoff schedule.
+func httpGetExpectingStatusCode(ctx context.Context, url string, status int) error {
+	var err error
+	var resp *http.Response
+	for _, backoff := range backoffSchedule {
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		default:
+		}
+		// This isn't a `case <-time.After`, so we give priority to `<-ctx.Done()` even on the first iteration.
+		time.Sleep(time.Duration(backoff) * time.Millisecond)
+		var req *http.Request
+		req, err = http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+		if err != nil {
+			return err
+		}
+
+		req.Header.Set("User-Agent", userAgent)
+		resp, err = http.DefaultClient.Do(req)
+		if err != nil {
+			continue
+		}
+		resp.Body.Close()
+
+		if resp.StatusCode == status {
+			return nil
+		}
+	}
+	// If the last attempt failed with a transport error, resp is nil.
+	if err != nil {
+		return err
+	}
+	return fmt.Errorf("got unexpected status code %d, expected %d", resp.StatusCode, status)
+}