prober: update cert check for prober (#15919)

OCSP has been removed from the LE certs.
Use CRL verification instead.

If a cert provides a CRL, check its revocation
status, if no CRL is provided and otherwise
is valid, pass the check.

Fixes #15912

Signed-off-by: Mike O'Driscoll <[email protected]>
Co-authored-by: Simon Law <[email protected]>

prober/tls.go

@@ -4,7 +4,6 @@
 package prober
 
 import (
-	"bytes"
 	"context"
 	"crypto/tls"
 	"crypto/x509"
@@ -15,12 +14,14 @@ import (
 	"net/netip"
 	"time"
 
-	"github.com/pkg/errors"
-	"golang.org/x/crypto/ocsp"
 	"tailscale.com/util/multierr"
 )
 
 const expiresSoon = 7 * 24 * time.Hour // 7 days from now
+// Let’s Encrypt promises to issue certificates with CRL servers after 2025-05-07:
+// https://letsencrypt.org/2024/12/05/ending-ocsp/
+// https://github.com/tailscale/tailscale/issues/15912
+const letsEncryptStartedStaplingCRL int64 = 1746576000 // 2025-05-07 00:00:00 UTC
 
 // TLS returns a Probe that healthchecks a TLS endpoint.
 //
@@ -106,50 +107,55 @@ func validateConnState(ctx context.Context, cs *tls.ConnectionState) (returnerr
 		}
 	}
 
-	if len(leafCert.OCSPServer) == 0 {
-		errs = append(errs, fmt.Errorf("no OCSP server presented in leaf cert for %v", leafCert.Subject))
+	if len(leafCert.CRLDistributionPoints) == 0 {
+		if leafCert.NotBefore.Before(time.Unix(letsEncryptStartedStaplingCRL, 0)) {
+			// Certificate might not have a CRL.
+			return
+		}
+		errs = append(errs, fmt.Errorf("no CRL server presented in leaf cert for %v", leafCert.Subject))
 		return
 	}
 
-	ocspResp, err := getOCSPResponse(ctx, leafCert.OCSPServer[0], leafCert, issuerCert)
+	err := checkCertCRL(ctx, leafCert.CRLDistributionPoints[0], leafCert, issuerCert)
 	if err != nil {
-		errs = append(errs, errors.Wrapf(err, "OCSP verification failed for %v", leafCert.Subject))
-		return
-	}
-
-	if ocspResp.Status == ocsp.Unknown {
-		errs = append(errs, fmt.Errorf("unknown OCSP verification status for %v", leafCert.Subject))
-	}
-
-	if ocspResp.Status == ocsp.Revoked {
-		errs = append(errs, fmt.Errorf("cert for %v has been revoked on %v, reason: %v", leafCert.Subject, ocspResp.RevokedAt, ocspResp.RevocationReason))
+		errs = append(errs, fmt.Errorf("CRL verification failed for %v: %w", leafCert.Subject, err))
 	}
 	return
 }
 
-func getOCSPResponse(ctx context.Context, ocspServer string, leafCert, issuerCert *x509.Certificate) (*ocsp.Response, error) {
-	reqb, err := ocsp.CreateRequest(leafCert, issuerCert, nil)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not create OCSP request")
-	}
-	hreq, err := http.NewRequestWithContext(ctx, "POST", ocspServer, bytes.NewReader(reqb))
+func checkCertCRL(ctx context.Context, crlURL string, leafCert, issuerCert *x509.Certificate) error {
+	hreq, err := http.NewRequestWithContext(ctx, "GET", crlURL, nil)
 	if err != nil {
-		return nil, errors.Wrap(err, "could not create OCSP POST request")
+		return fmt.Errorf("could not create CRL GET request: %w", err)
 	}
-	hreq.Header.Add("Content-Type", "application/ocsp-request")
-	hreq.Header.Add("Accept", "application/ocsp-response")
 	hresp, err := http.DefaultClient.Do(hreq)
 	if err != nil {
-		return nil, errors.Wrap(err, "OCSP request failed")
+		return fmt.Errorf("CRL request failed: %w", err)
 	}
 	defer hresp.Body.Close()
 	if hresp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("ocsp: non-200 status code from OCSP server: %s", hresp.Status)
+		return fmt.Errorf("crl: non-200 status code from CRL server: %s", hresp.Status)
 	}
 	lr := io.LimitReader(hresp.Body, 10<<20) // 10MB
-	ocspB, err := io.ReadAll(lr)
+	crlB, err := io.ReadAll(lr)
 	if err != nil {
-		return nil, err
+		return err
 	}
-	return ocsp.ParseResponse(ocspB, issuerCert)
+
+	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)
+	}
+
+	for _, revoked := range crl.RevokedCertificateEntries {
+		if revoked.SerialNumber.Cmp(leafCert.SerialNumber) == 0 {
+			return fmt.Errorf("cert for %v has been revoked on %v, reason: %v", leafCert.Subject, revoked.RevocationTime, revoked.ReasonCode)
+		}
+	}
+
+	return nil
 }

prober/tls_test.go

@@ -6,7 +6,6 @@ package prober
 import (
 	"bytes"
 	"context"
-	"crypto"
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/tls"
@@ -20,8 +19,6 @@ import (
 	"strings"
 	"testing"
 	"time"
-
-	"golang.org/x/crypto/ocsp"
 )
 
 var leafCert = x509.Certificate{
@@ -118,11 +115,6 @@ func TestCertExpiration(t *testing.T) {
 			},
 			"one of the certs expires in",
 		},
-		{
-			"valid duration but no OCSP",
-			func() *x509.Certificate { return &leafCert },
-			"no OCSP server presented in leaf cert for CN=tlsprobe.test",
-		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
 			cs := &tls.ConnectionState{PeerCertificates: []*x509.Certificate{tt.cert()}}
@@ -134,93 +126,150 @@ func TestCertExpiration(t *testing.T) {
 	}
 }
 
-type ocspServer struct {
-	issuer        *x509.Certificate
-	responderCert *x509.Certificate
-	template      *ocsp.Response
-	priv          crypto.Signer
+type CRLServer struct {
+	crlBytes []byte
 }
 
-func (s *ocspServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	if s.template == nil {
+func (s *CRLServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	if s.crlBytes == nil {
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
-	resp, err := ocsp.CreateResponse(s.issuer, s.responderCert, *s.template, s.priv)
-	if err != nil {
-		panic(err)
-	}
-	w.Write(resp)
+	w.Header().Set("Content-Type", "application/pkix-crl")
+	w.WriteHeader(http.StatusOK)
+	w.Write(s.crlBytes)
 }
 
-func TestOCSP(t *testing.T) {
-	issuerKey, err := rsa.GenerateKey(rand.Reader, 4096)
+func TestCRL(t *testing.T) {
+	// Generate CA key and self-signed CA cert
+	caKey, err := rsa.GenerateKey(rand.Reader, 4096)
 	if err != nil {
 		t.Fatal(err)
 	}
-	issuerBytes, err := x509.CreateCertificate(rand.Reader, &issuerCertTpl, &issuerCertTpl, &issuerKey.PublicKey, issuerKey)
+	caTpl := issuerCertTpl
+	caTpl.BasicConstraintsValid = true
+	caTpl.IsCA = true
+	caTpl.KeyUsage = x509.KeyUsageCertSign | x509.KeyUsageCRLSign | x509.KeyUsageDigitalSignature
+	caBytes, err := x509.CreateCertificate(rand.Reader, &caTpl, &caTpl, &caKey.PublicKey, caKey)
 	if err != nil {
 		t.Fatal(err)
 	}
-	issuerCert, err := x509.ParseCertificate(issuerBytes)
+	caCert, err := x509.ParseCertificate(caBytes)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	responderKey, err := rsa.GenerateKey(rand.Reader, 4096)
+	// Issue a leaf cert signed by the CA
+	leaf := leafCert
+	leaf.SerialNumber = big.NewInt(20001)
+	leaf.Issuer = caCert.Subject
+	leafKey, err := rsa.GenerateKey(rand.Reader, 4096)
 	if err != nil {
 		t.Fatal(err)
 	}
-	// issuer cert template re-used here, but with a different key
-	responderBytes, err := x509.CreateCertificate(rand.Reader, &issuerCertTpl, &issuerCertTpl, &responderKey.PublicKey, responderKey)
+	leafBytes, err := x509.CreateCertificate(rand.Reader, &leaf, caCert, &leafKey.PublicKey, caKey)
 	if err != nil {
 		t.Fatal(err)
 	}
-	responderCert, err := x509.ParseCertificate(responderBytes)
+	leafCertParsed, err := x509.ParseCertificate(leafBytes)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	handler := &ocspServer{
-		issuer:        issuerCert,
-		responderCert: responderCert,
-		priv:          issuerKey,
+	// Catch no CRL set by Let's Encrypt date.
+	noCRLCert := leafCert
+	noCRLCert.SerialNumber = big.NewInt(20002)
+	noCRLCert.CRLDistributionPoints = []string{}
+	noCRLCert.NotBefore = time.Unix(letsEncryptStartedStaplingCRL, 0).Add(-48 * time.Hour)
+	noCRLCert.Issuer = caCert.Subject
+	noCRLCertKey, err := rsa.GenerateKey(rand.Reader, 4096)
+	if err != nil {
+		t.Fatal(err)
 	}
-	srv := httptest.NewUnstartedServer(handler)
-	srv.Start()
-	defer srv.Close()
-
-	cert := leafCert
-	cert.OCSPServer = append(cert.OCSPServer, srv.URL)
-	key, err := rsa.GenerateKey(rand.Reader, 4096)
+	noCRLStapledBytes, err := x509.CreateCertificate(rand.Reader, &noCRLCert, caCert, &noCRLCertKey.PublicKey, caKey)
 	if err != nil {
 		t.Fatal(err)
 	}
-	certBytes, err := x509.CreateCertificate(rand.Reader, &cert, issuerCert, &key.PublicKey, issuerKey)
+	noCRLStapledParsed, err := x509.ParseCertificate(noCRLStapledBytes)
 	if err != nil {
 		t.Fatal(err)
 	}
-	parsed, err := x509.ParseCertificate(certBytes)
+
+	crlServer := &CRLServer{crlBytes: nil}
+	srv := httptest.NewServer(crlServer)
+	defer srv.Close()
+
+	// Create a CRL that revokes the leaf cert using x509.CreateRevocationList
+	now := time.Now()
+	revoked := []x509.RevocationListEntry{{
+		SerialNumber:   leaf.SerialNumber,
+		RevocationTime: now,
+		ReasonCode:     1, // Key compromise
+	}}
+	rl := x509.RevocationList{
+		SignatureAlgorithm:        caCert.SignatureAlgorithm,
+		Issuer:                    caCert.Subject,
+		ThisUpdate:                now,
+		NextUpdate:                now.Add(24 * time.Hour),
+		RevokedCertificateEntries: revoked,
+		Number:                    big.NewInt(1),
+	}
+	rlBytes, err := x509.CreateRevocationList(rand.Reader, &rl, caCert, caKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	emptyRlBytes, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{Number: big.NewInt(2)}, caCert, caKey)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	for _, tt := range []struct {
-		name    string
-		resp    *ocsp.Response
-		wantErr string
+		name     string
+		cert     *x509.Certificate
+		crlBytes []byte
+		wantErr  string
 	}{
-		{"good response", &ocsp.Response{Status: ocsp.Good}, ""},
-		{"unknown response", &ocsp.Response{Status: ocsp.Unknown}, "unknown OCSP verification status for CN=tlsprobe.test"},
-		{"revoked response", &ocsp.Response{Status: ocsp.Revoked}, "cert for CN=tlsprobe.test has been revoked"},
-		{"error 500 from ocsp", nil, "non-200 status code from OCSP"},
+		{
+			"ValidCert",
+			leafCertParsed,
+			emptyRlBytes,
+			"",
+		},
+		{
+			"RevokedCert",
+			leafCertParsed,
+			rlBytes,
+			"has been revoked on",
+		},
+		{
+			"EmptyCRL",
+			leafCertParsed,
+			emptyRlBytes,
+			"",
+		},
+		{
+			"NoCRL",
+			leafCertParsed,
+			nil,
+			"",
+		},
+		{
+			"NotBeforeCRLStaplingDate",
+			noCRLStapledParsed,
+			nil,
+			"",
+		},
 	} {
 		t.Run(tt.name, func(t *testing.T) {
-			handler.template = tt.resp
-			if handler.template != nil {
-				handler.template.SerialNumber = big.NewInt(1337)
+			cs := &tls.ConnectionState{PeerCertificates: []*x509.Certificate{tt.cert, caCert}}
+			if tt.crlBytes != nil {
+				crlServer.crlBytes = tt.crlBytes
+				tt.cert.CRLDistributionPoints = []string{srv.URL}
+			} else {
+				crlServer.crlBytes = nil
+				tt.cert.CRLDistributionPoints = []string{}
 			}
-			cs := &tls.ConnectionState{PeerCertificates: []*x509.Certificate{parsed, issuerCert}}
 			err := validateConnState(context.Background(), cs)
 
 			if err == nil && tt.wantErr == "" {