prober: expand certificate verification logic in the TLS prober

TLS prober now checks validity period for all server certificates
and verifies OCSP revocation status for the leaf cert.

Signed-off-by: Anton Tolchanov <anton@tailscale.com>

go.mod

@@ -39,6 +39,7 @@ require (
 	github.com/miekg/dns v1.1.43
 	github.com/mitchellh/go-ps v1.0.0
 	github.com/peterbourgon/ff/v3 v3.1.2
+	github.com/pkg/errors v0.9.1
 	github.com/pkg/sftp v1.13.4
 	github.com/skip2/go-qrcode v0.0.0-20200617195104-da1b6568686e
 	github.com/tailscale/certstore v0.1.1-0.20220316223106-78d6e1c49d8d
@@ -215,7 +216,6 @@ require (
 	github.com/pelletier/go-toml v1.9.4 // indirect
 	github.com/phayes/checkstyle v0.0.0-20170904204023-bfd46e6a821d // indirect
 	github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e // indirect
-	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/polyfloyd/go-errorlint v0.0.0-20211125173453-6d6d39c5bb8b // indirect
 	github.com/prometheus/client_golang v1.11.0 // indirect

prober/http.go

@@ -18,8 +18,7 @@
 //
 // The ProbeFunc sends a GET request for url, expects an HTTP 200
 // response, and verifies that want is present in the response
-// body. If the URL is HTTPS, the probe further checks that the TLS
-// certificate is good for at least the next 7 days.
+// body.
 func HTTP(url, wantText string) ProbeFunc {
 	return func(ctx context.Context) error {
 		return probeHTTP(ctx, url, []byte(wantText))

prober/tls.go

@@ -5,18 +5,28 @@
 package prober
 
 import (
+	"bytes"
 	"context"
 	"crypto/tls"
+	"crypto/x509"
 	"fmt"
+	"io"
 	"net"
+	"net/http"
 	"time"
+
+	"github.com/pkg/errors"
+	"golang.org/x/crypto/ocsp"
+	"tailscale.com/util/multierr"
 )
 
+const expiresSoon = 7 * 24 * time.Hour // 7 days from now
+
 // TLS returns a Probe that healthchecks a TLS endpoint.
 //
-// The ProbeFunc connects to hostname, does a TLS handshake, verifies
-// that the hostname matches the presented certificate, and that the
-// certificate expires in more than 7 days from the probe time.
+// The ProbeFunc connects to a hostname (host:port string), does a TLS
+// handshake, verifies that the hostname matches the presented certificate,
+// checks certificate validity time and OCSP revocation status.
 func TLS(hostname string) ProbeFunc {
 	return func(ctx context.Context) error {
 		return probeTLS(ctx, hostname)
@@ -24,23 +34,108 @@ func TLS(hostname string) ProbeFunc {
 }
 
 func probeTLS(ctx context.Context, hostname string) error {
-	var d net.Dialer
-	conn, err := tls.DialWithDialer(&d, "tcp", hostname+":443", nil)
+	host, _, err := net.SplitHostPort(hostname)
+	if err != nil {
+		return err
+	}
+
+	dialer := &tls.Dialer{Config: &tls.Config{ServerName: host}}
+	conn, err := dialer.DialContext(ctx, "tcp", hostname)
 	if err != nil {
 		return fmt.Errorf("connecting to %q: %w", hostname, err)
 	}
-	if err := conn.Handshake(); err != nil {
-		return fmt.Errorf("TLS handshake error with %q: %w", hostname, err)
-	}
-	if err := conn.VerifyHostname(hostname); err != nil {
-		return fmt.Errorf("Host %q TLS verification failed: %w", hostname, err)
-	}
+	defer conn.Close()
 
-	latestAllowedExpiration := time.Now().Add(7 * 24 * time.Hour) // 7 days from now
-	if expires := conn.ConnectionState().PeerCertificates[0].NotAfter; latestAllowedExpiration.After(expires) {
-		left := expires.Sub(time.Now())
-		return fmt.Errorf("TLS certificate for %q expires in %v", hostname, left)
-	}
-
-	return nil
+	tlsConnState := conn.(*tls.Conn).ConnectionState()
+	return validateConnState(ctx, &tlsConnState)
+}
+
+// validateConnState verifies certificate validity time in all certificates
+// returned by the TLS server and checks OCSP revocation status for the
+// leaf cert.
+func validateConnState(ctx context.Context, cs *tls.ConnectionState) (returnerr error) {
+	var errs []error
+	defer func() {
+		returnerr = multierr.New(errs...)
+	}()
+	latestAllowedExpiration := time.Now().Add(expiresSoon)
+
+	var leafCert *x509.Certificate
+	var issuerCert *x509.Certificate
+	var leafAuthorityKeyID string
+	// PeerCertificates will never be len == 0 on the client side
+	for i, cert := range cs.PeerCertificates {
+		if i == 0 {
+			leafCert = cert
+			leafAuthorityKeyID = string(cert.AuthorityKeyId)
+		}
+		if i > 0 {
+			if leafAuthorityKeyID == string(cert.SubjectKeyId) {
+				issuerCert = cert
+			}
+		}
+
+		// Do not check certificate validity period for self-signed certs.
+		// The practical reason is to avoid raising alerts for expiring
+		// DERP metaCert certificates that are returned as part of regular
+		// TLS handshake.
+		if string(cert.SubjectKeyId) == string(cert.AuthorityKeyId) {
+			continue
+		}
+
+		if time.Now().Before(cert.NotBefore) {
+			errs = append(errs, fmt.Errorf("one of the certs has NotBefore in the future (%v): %v", cert.NotBefore, cert.Subject))
+		}
+		if latestAllowedExpiration.After(cert.NotAfter) {
+			left := cert.NotAfter.Sub(time.Now())
+			errs = append(errs, fmt.Errorf("one of the certs expires in %v: %v", left, cert.Subject))
+		}
+	}
+
+	if len(leafCert.OCSPServer) == 0 {
+		errs = append(errs, fmt.Errorf("no OCSP server presented in leaf cert for %v", leafCert.Subject))
+		return
+	}
+
+	ocspResp, err := getOCSPResponse(ctx, leafCert.OCSPServer[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))
+	}
+	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))
+	if err != nil {
+		return nil, errors.Wrap(err, "could not create OCSP POST request")
+	}
+	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")
+	}
+	defer hresp.Body.Close()
+	if hresp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("ocsp: non-200 status code from OCSP server: %s", hresp.Status)
+	}
+	lr := io.LimitReader(hresp.Body, 10<<20) // 10MB
+	ocspB, err := io.ReadAll(lr)
+	if err != nil {
+		return nil, err
+	}
+	return ocsp.ParseResponse(ocspB, issuerCert)
 }

prober/tls_test.go

@@ -0,0 +1,236 @@
+// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package prober
+
+import (
+	"bytes"
+	"context"
+	"crypto"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/tls"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"math/big"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"golang.org/x/crypto/ocsp"
+)
+
+var leafCert = x509.Certificate{
+	SerialNumber:       big.NewInt(10001),
+	Subject:            pkix.Name{CommonName: "tlsprobe.test"},
+	SignatureAlgorithm: x509.SHA256WithRSA,
+	PublicKeyAlgorithm: x509.RSA,
+	Version:            3,
+	IPAddresses:        []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback},
+	NotBefore:          time.Now().Add(-5 * time.Minute),
+	NotAfter:           time.Now().Add(60 * 24 * time.Hour),
+	SubjectKeyId:       []byte{1, 2, 3},
+	AuthorityKeyId:     []byte{1, 2, 3, 4, 5}, // issuerCert below
+	ExtKeyUsage:        []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth},
+	KeyUsage:           x509.KeyUsageDigitalSignature,
+}
+
+var issuerCertTpl = x509.Certificate{
+	SerialNumber:       big.NewInt(10002),
+	Subject:            pkix.Name{CommonName: "tlsprobe.ca.test"},
+	SignatureAlgorithm: x509.SHA256WithRSA,
+	PublicKeyAlgorithm: x509.RSA,
+	Version:            3,
+	IPAddresses:        []net.IP{net.IPv4(127, 0, 0, 1), net.IPv6loopback},
+	NotBefore:          time.Now().Add(-5 * time.Minute),
+	NotAfter:           time.Now().Add(60 * 24 * time.Hour),
+	SubjectKeyId:       []byte{1, 2, 3, 4, 5},
+	ExtKeyUsage:        []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth, x509.ExtKeyUsageServerAuth},
+	KeyUsage:           x509.KeyUsageDigitalSignature,
+}
+
+func simpleCert() (tls.Certificate, error) {
+	certPrivKey, err := rsa.GenerateKey(rand.Reader, 4096)
+	if err != nil {
+		return tls.Certificate{}, err
+	}
+	certPrivKeyPEM := new(bytes.Buffer)
+	pem.Encode(certPrivKeyPEM, &pem.Block{
+		Type:  "RSA PRIVATE KEY",
+		Bytes: x509.MarshalPKCS1PrivateKey(certPrivKey),
+	})
+	certBytes, err := x509.CreateCertificate(rand.Reader, &leafCert, &leafCert, &certPrivKey.PublicKey, certPrivKey)
+	if err != nil {
+		return tls.Certificate{}, err
+	}
+	certPEM := new(bytes.Buffer)
+	pem.Encode(certPEM, &pem.Block{
+		Type:  "CERTIFICATE",
+		Bytes: certBytes,
+	})
+	return tls.X509KeyPair(certPEM.Bytes(), certPrivKeyPEM.Bytes())
+}
+
+func TestTLSConnection(t *testing.T) {
+	crt, err := simpleCert()
+	if err != nil {
+		t.Fatal(err)
+	}
+	srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	srv.TLS = &tls.Config{Certificates: []tls.Certificate{crt}}
+	srv.StartTLS()
+	defer srv.Close()
+
+	err = probeTLS(context.Background(), srv.Listener.Addr().String())
+	// The specific error message here is platform-specific ("certificate is not trusted"
+	// on macOS and "certificate signed by unknown authority" on Linux), so only check
+	// that it contains the word 'certificate'.
+	if err == nil || !strings.Contains(err.Error(), "certificate") {
+		t.Errorf("unexpected error: %q", err)
+	}
+}
+
+func TestCertExpiration(t *testing.T) {
+	for _, tt := range []struct {
+		name    string
+		cert    func() *x509.Certificate
+		wantErr string
+	}{
+		{
+			"cert not valid yet",
+			func() *x509.Certificate {
+				c := leafCert
+				c.NotBefore = time.Now().Add(time.Hour)
+				return &c
+			},
+			"one of the certs has NotBefore in the future",
+		},
+		{
+			"cert expiring soon",
+			func() *x509.Certificate {
+				c := leafCert
+				c.NotAfter = time.Now().Add(time.Hour)
+				return &c
+			},
+			"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()}}
+			err := validateConnState(context.Background(), cs)
+			if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
+				t.Errorf("unexpected error %q; want %q", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+type ocspServer struct {
+	issuer        *x509.Certificate
+	responderCert *x509.Certificate
+	template      *ocsp.Response
+	priv          crypto.Signer
+}
+
+func (s *ocspServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	if s.template == 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)
+}
+
+func TestOCSP(t *testing.T) {
+	issuerKey, err := rsa.GenerateKey(rand.Reader, 4096)
+	if err != nil {
+		t.Fatal(err)
+	}
+	issuerBytes, err := x509.CreateCertificate(rand.Reader, &issuerCertTpl, &issuerCertTpl, &issuerKey.PublicKey, issuerKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	issuerCert, err := x509.ParseCertificate(issuerBytes)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	responderKey, 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)
+	if err != nil {
+		t.Fatal(err)
+	}
+	responderCert, err := x509.ParseCertificate(responderBytes)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	handler := &ocspServer{
+		issuer:        issuerCert,
+		responderCert: responderCert,
+		priv:          issuerKey,
+	}
+	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)
+	if err != nil {
+		t.Fatal(err)
+	}
+	certBytes, err := x509.CreateCertificate(rand.Reader, &cert, issuerCert, &key.PublicKey, issuerKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	parsed, err := x509.ParseCertificate(certBytes)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, tt := range []struct {
+		name    string
+		resp    *ocsp.Response
+		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"},
+	} {
+		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{parsed, issuerCert}}
+			err := validateConnState(context.Background(), cs)
+
+			if err == nil && tt.wantErr == "" {
+				return
+			}
+
+			if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
+				t.Errorf("unexpected error %q; want %q", err, tt.wantErr)
+			}
+		})
+	}
+}