derphttp: add TLSConfigBypassesTLSDial opt-in flag

derp/derphttp/derphttp_client.go

@@ -53,10 +53,42 @@ import (
 // has been called).
 type Client struct {
 	Header    http.Header
-	TLSConfig *tls.Config        // optional; nil means default
-	DNSCache  *dnscache.Resolver // optional; nil means no caching
-	MeshKey   string             // optional; for trusted clients
-	IsProber  bool               // optional; for probers to optional declare themselves as such
+	TLSConfig *tls.Config // optional; nil means default
+
+	// TLSConfigBypassesTLSDial, if true, causes TLSConfig to be used as-is
+	// (after a Clone and ServerName housekeeping) instead of being passed
+	// through tlsdial.Config. The default behavior wraps TLSConfig in
+	// Tailscale's hosted-DERP verification helper, which installs a
+	// VerifyConnection hook with a baked-in Let's Encrypt fallback and
+	// rejects (panics on) base configs that already set InsecureSkipVerify
+	// or VerifyConnection.
+	//
+	// Set this to true when the caller is responsible for server
+	// verification — e.g. when DERP is fronted by a reverse proxy that
+	// presents a non-publicly-trusted certificate, when using an mTLS
+	// framework that performs its own peer verification (custom CAs,
+	// SPIFFE-style identity), or any case in which the caller has supplied
+	// VerifyPeerCertificate / VerifyConnection on TLSConfig and does not
+	// want the bake-in fallback.
+	//
+	// SECURITY: when this flag is true, the supplied TLSConfig is the
+	// SOLE source of server verification. A TLSConfig with
+	// InsecureSkipVerify=true and no VerifyPeerCertificate /
+	// VerifyConnection callback will result in a TLS handshake that
+	// performs no server identity check at all. Callers MUST either rely
+	// on stock RootCAs + hostname matching (i.e. leave InsecureSkipVerify
+	// false), or provide a VerifyPeerCertificate / VerifyConnection that
+	// implements an equivalent check.
+	//
+	// When true, TLSConfig must be non-nil; the flag is ignored otherwise.
+	// Per-DERPNode overrides are still honored: InsecureForTests still
+	// disables verification (intentionally), but CertName (which is
+	// implemented by tlsdial) is ignored.
+	TLSConfigBypassesTLSDial bool
+
+	DNSCache *dnscache.Resolver // optional; nil means no caching
+	MeshKey  string             // optional; for trusted clients
+	IsProber bool               // optional; for probers to optional declare themselves as such
 
 	// Allow forcing WebSocket fallback for situations where proxies do not
 	// play well with `Upgrade: derp`. Turning this on will cause the client to
@@ -704,14 +736,34 @@ func (c *Client) DialRegion(ctx context.Context, reg *tailcfg.DERPRegion) (net.C
 }
 
 func (c *Client) tlsConfig(node *tailcfg.DERPNode) *tls.Config {
-	tlsConf := tlsdial.Config(c.tlsServerName(node), c.TLSConfig)
-	if node != nil {
-		if node.InsecureForTests {
+	var tlsConf *tls.Config
+	if c.TLSConfigBypassesTLSDial && c.TLSConfig != nil {
+		// Caller has opted to bring their own server verification (custom
+		// CAs / mTLS / SPIFFE-style identity / non-publicly-trusted PKI).
+		// Use the supplied config as-is, only filling in ServerName when
+		// the caller didn't pin one. node.CertName (a tlsdial-specific
+		// domain-fronting hook) is intentionally not applied here; callers
+		// using bypass are expected to encode any cert-pinning in their
+		// own VerifyPeerCertificate / VerifyConnection.
+		tlsConf = c.TLSConfig.Clone()
+		if tlsConf.ServerName == "" {
+			tlsConf.ServerName = c.tlsServerName(node)
+		}
+		if node != nil && node.InsecureForTests {
 			tlsConf.InsecureSkipVerify = true
 			tlsConf.VerifyConnection = nil
+			tlsConf.VerifyPeerCertificate = nil
 		}
-		if node.CertName != "" {
-			tlsdial.SetConfigExpectedCert(tlsConf, node.CertName)
+	} else {
+		tlsConf = tlsdial.Config(c.tlsServerName(node), c.TLSConfig)
+		if node != nil {
+			if node.InsecureForTests {
+				tlsConf.InsecureSkipVerify = true
+				tlsConf.VerifyConnection = nil
+			}
+			if node.CertName != "" {
+				tlsdial.SetConfigExpectedCert(tlsConf, node.CertName)
+			}
 		}
 	}
 	tlsConf.NextProtos = []string{"http/1.1"}

derp/derphttp/derphttp_test.go

@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
+	"crypto/x509"
 	"net"
 	"net/http"
 	"net/http/httptest"
@@ -17,6 +18,7 @@ import (
 
 	"github.com/coder/websocket"
 	"tailscale.com/derp"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 )
 
@@ -324,3 +326,101 @@ func TestForceWebsockets(t *testing.T) {
 
 	c.Close()
 }
+
+func TestTLSConfigBypassesTLSDial(t *testing.T) {
+	node := &tailcfg.DERPNode{HostName: "derp.example.com"}
+
+	t.Run("default path wraps via tlsdial.Config", func(t *testing.T) {
+		// tlsdial.Config installs its own VerifyConnection. Confirming it
+		// runs is the easiest way to assert we did NOT bypass it. We must
+		// not pass a base config that would cause tlsdial.Config to panic.
+		c := &Client{TLSConfig: &tls.Config{RootCAs: x509.NewCertPool()}}
+		got := c.tlsConfig(node)
+		if got.VerifyConnection == nil {
+			t.Fatal("expected default path to install tlsdial's VerifyConnection")
+		}
+		if !got.InsecureSkipVerify {
+			t.Fatal("expected default path to set InsecureSkipVerify (tlsdial does its own verify)")
+		}
+		if got.ServerName != node.HostName {
+			t.Fatalf("ServerName: got %q, want %q", got.ServerName, node.HostName)
+		}
+	})
+
+	t.Run("bypass path uses caller config as-is", func(t *testing.T) {
+		// A typical "I'm bringing my own verifier" config that would make
+		// tlsdial.Config panic.
+		var verifyCalls int
+		base := &tls.Config{
+			InsecureSkipVerify: true,
+			VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error {
+				verifyCalls++
+				return nil
+			},
+		}
+		c := &Client{TLSConfig: base, TLSConfigBypassesTLSDial: true}
+		got := c.tlsConfig(node)
+		if got == base {
+			t.Fatal("expected tlsConfig to clone the base config")
+		}
+		if got.VerifyConnection != nil {
+			t.Fatal("bypass path must not install tlsdial's VerifyConnection")
+		}
+		if !got.InsecureSkipVerify {
+			t.Fatal("bypass path must preserve caller's InsecureSkipVerify")
+		}
+		if got.VerifyPeerCertificate == nil {
+			t.Fatal("bypass path must preserve caller's VerifyPeerCertificate")
+		}
+		if got.ServerName != node.HostName {
+			t.Fatalf("ServerName fallback: got %q, want %q", got.ServerName, node.HostName)
+		}
+		if want := []string{"http/1.1"}; len(got.NextProtos) != 1 || got.NextProtos[0] != want[0] {
+			t.Fatalf("NextProtos: got %v, want %v", got.NextProtos, want)
+		}
+	})
+
+	t.Run("bypass path preserves caller-set ServerName", func(t *testing.T) {
+		base := &tls.Config{
+			InsecureSkipVerify: true,
+			VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error {
+				return nil
+			},
+			ServerName: "explicit.example.com",
+		}
+		c := &Client{TLSConfig: base, TLSConfigBypassesTLSDial: true}
+		got := c.tlsConfig(node)
+		if got.ServerName != "explicit.example.com" {
+			t.Fatalf("ServerName: got %q, want explicit.example.com", got.ServerName)
+		}
+	})
+
+	t.Run("bypass flag without TLSConfig falls back to default path", func(t *testing.T) {
+		c := &Client{TLSConfigBypassesTLSDial: true}
+		got := c.tlsConfig(node)
+		if got.VerifyConnection == nil {
+			t.Fatal("expected fallback to tlsdial path when TLSConfig is nil")
+		}
+	})
+
+	t.Run("bypass path honors node.InsecureForTests", func(t *testing.T) {
+		base := &tls.Config{
+			InsecureSkipVerify: true,
+			VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error {
+				return nil
+			},
+		}
+		c := &Client{TLSConfig: base, TLSConfigBypassesTLSDial: true}
+		insecureNode := &tailcfg.DERPNode{HostName: "derp.example.com", InsecureForTests: true}
+		got := c.tlsConfig(insecureNode)
+		if !got.InsecureSkipVerify {
+			t.Fatal("expected InsecureForTests to keep InsecureSkipVerify=true")
+		}
+		if got.VerifyPeerCertificate != nil {
+			t.Fatal("expected InsecureForTests to clear VerifyPeerCertificate")
+		}
+		if got.VerifyConnection != nil {
+			t.Fatal("expected InsecureForTests to clear VerifyConnection")
+		}
+	})
+}

net/netcheck/netcheck.go

@@ -208,9 +208,24 @@ type Client struct {
 	// probes.
 	GetDERPHeaders func() http.Header
 
+	// GetDERPTLSConfig, if non-nil, returns the TLS config and bypass-tlsdial
+	// setting to use for the next DERP probe. When set, it takes precedence
+	// over DERPTLSConfig and DERPTLSConfigBypassesTLSDial. This is useful when
+	// the DERP TLS config can be updated after the Client is constructed.
+	GetDERPTLSConfig func() (*tls.Config, bool)
+
 	// DERPTLSConfig is an optional TLS config for DERP connections.
 	DERPTLSConfig *tls.Config
 
+	// DERPTLSConfigBypassesTLSDial mirrors
+	// derphttp.Client.TLSConfigBypassesTLSDial: when true (and
+	// DERPTLSConfig is non-nil), the netcheck DERP client uses the
+	// supplied TLS config as-is instead of wrapping it via tlsdial.Config.
+	// Use this when DERPTLSConfig performs its own server verification
+	// (e.g. mTLS frameworks with custom CAs / SPIFFE-style identity), as
+	// such configs cause tlsdial.Config to panic.
+	DERPTLSConfigBypassesTLSDial bool
+
 	// For tests
 	testEnoughRegions      int
 	testCaptivePortalDelay time.Duration
@@ -1305,8 +1320,14 @@ func (c *Client) measureHTTPLatency(ctx context.Context, reg *tailcfg.DERPRegion
 
 	dc := derphttp.NewNetcheckClient(c.logf)
 	dc.Header = derpHeaders
-	if c.DERPTLSConfig != nil {
-		dc.TLSConfig = c.DERPTLSConfig
+	derpTLSConfig := c.DERPTLSConfig
+	derpTLSConfigBypassesTLSDial := c.DERPTLSConfigBypassesTLSDial
+	if c.GetDERPTLSConfig != nil {
+		derpTLSConfig, derpTLSConfigBypassesTLSDial = c.GetDERPTLSConfig()
+	}
+	if derpTLSConfig != nil {
+		dc.TLSConfig = derpTLSConfig
+		dc.TLSConfigBypassesTLSDial = derpTLSConfigBypassesTLSDial
 	}
 	defer dc.Close()
 

net/netcheck/netcheck_test.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/tls"
+	"crypto/x509"
 	"fmt"
 	"net"
 	"net/http"
@@ -1107,6 +1108,95 @@ func TestProbeHeaders(t *testing.T) {
 	}
 }
 
+func TestProbeTLSConfigBypass(t *testing.T) {
+	logf, closeLogf := logger.LogfCloser(t.Logf)
+	defer closeLogf()
+
+	// Create a DERP server manually, without a STUN server and with a custom
+	// handler.
+	derpServer := derp.NewServer(key.NewNode(), logf)
+	derpHandler := derphttp.Handler(derpServer)
+
+	var called atomic.Bool
+	httpsrv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		called.Store(true)
+		if r.URL.Path == "/derp/latency-check" {
+			w.WriteHeader(http.StatusOK)
+			return
+		}
+		if r.URL.Path == "/derp" {
+			derpHandler.ServeHTTP(w, r)
+			return
+		}
+
+		t.Errorf("unexpected request: %v", r.URL)
+		w.WriteHeader(http.StatusNotFound)
+	}))
+	httpsrv.Config.ErrorLog = logger.StdLogger(logf)
+	httpsrv.Config.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler))
+	httpsrv.StartTLS()
+	t.Cleanup(func() {
+		httpsrv.CloseClientConnections()
+		httpsrv.Close()
+		derpServer.Close()
+	})
+
+	derpMap := &tailcfg.DERPMap{
+		Regions: map[int]*tailcfg.DERPRegion{
+			1: {
+				RegionID:   1,
+				RegionCode: "derpy",
+				Nodes: []*tailcfg.DERPNode{
+					{
+						Name:     "d1",
+						RegionID: 1,
+						HostName: "localhost",
+						// Don't specify an IP address to avoid ICMP pinging,
+						// which will bypass the artificial latency.
+						IPv4:             "",
+						IPv6:             "",
+						STUNPort:         -1,
+						DERPPort:         httpsrv.Listener.Addr().(*net.TCPAddr).Port,
+						InsecureForTests: true,
+					},
+				},
+			},
+		},
+	}
+
+	var getterCalled atomic.Bool
+	c := &Client{
+		Logf:        t.Logf,
+		UDPBindAddr: "127.0.0.1:0",
+		GetDERPTLSConfig: func() (*tls.Config, bool) {
+			getterCalled.Store(true)
+			return &tls.Config{
+				// This would panic inside tlsdial.Config if the bypass flag
+				// returned alongside it were not propagated to derphttp.Client.
+				InsecureSkipVerify: true,
+				VerifyPeerCertificate: func(_ [][]byte, _ [][]*x509.Certificate) error {
+					return nil
+				},
+			}, true
+		},
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+
+	_, err := c.GetReport(ctx, derpMap)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if !getterCalled.Load() {
+		t.Error("didn't call GetDERPTLSConfig")
+	}
+	if !called.Load() {
+		t.Error("didn't call test handler")
+	}
+}
+
 func TestNeverPickSTUNOnlyRegionAsPreferredDERP(t *testing.T) {
 	// Create two DERP regions, one with a STUN server only and one with only a
 	// DERP node. Add artificial latency of 300ms to the DERP region, and test

wgengine/magicsock/derp.go

@@ -347,8 +347,14 @@ func (c *Conn) derpWriteChanOfAddr(addr netip.AddrPort, peer key.NodePublic) cha
 	dc.SetAddressFamilySelector(derpAddrFamSelector{c})
 	dc.SetForcedWebsocketCallback(c.derpForcedWebsocketFunc)
 	dc.DNSCache = dnscache.Get()
-	if tlsCfg := c.derpTLSConfig.Load(); tlsCfg != nil {
-		dc.TLSConfig = tlsCfg
+	// Read the (TLS config, bypass-tlsdial) pair atomically so we never
+	// observe a new custom-verifier TLS config paired with a stale
+	// bypass=false (which would panic in tlsdial.Config) nor a new
+	// publicly-trusted config paired with a stale bypass=true (which would
+	// silently skip server verification). See Conn.derpTLSConfig.
+	if pair := c.derpTLSConfig.Load(); pair != nil && pair.cfg != nil {
+		dc.TLSConfig = pair.cfg
+		dc.TLSConfigBypassesTLSDial = pair.bypassTLSDial
 	}
 	header := c.derpHeader.Load()
 	if header != nil {