wgengine/magicsock: disable raw disco by default; add envknob to enable

Updates #13140

Signed-off-by: Andrew Dunham <[email protected]>
Change-Id: Ica85b2ac8ac7eab4ec5413b212f004aecc453279

tstest/integration/nat/nat_test.go

@@ -171,11 +171,15 @@ func easyPMP(c *vnet.Config) *vnet.Node {
 		fmt.Sprintf("192.168.%d.1/24", n), vnet.EasyNAT, vnet.NATPMP))
 }
 
-// easy + port mapping + host firewall
-func easyPMPFW(c *vnet.Config) *vnet.Node {
+// easy + port mapping + host firewall + BPF
+func easyPMPFWPlusBPF(c *vnet.Config) *vnet.Node {
 	n := c.NumNodes() + 1
 	return c.AddNode(
 		vnet.HostFirewall,
+		vnet.TailscaledEnv{
+			Key:   "TS_ENABLE_RAW_DISCO",
+			Value: "true",
+		},
 		vnet.TailscaledEnv{
 			Key:   "TS_DEBUG_RAW_DISCO",
 			Value: "1",
@@ -199,8 +203,8 @@ func easyPMPFWNoBPF(c *vnet.Config) *vnet.Node {
 	return c.AddNode(
 		vnet.HostFirewall,
 		vnet.TailscaledEnv{
-			Key:   "TS_DEBUG_DISABLE_RAW_DISCO",
-			Value: "1",
+			Key:   "TS_ENABLE_RAW_DISCO",
+			Value: "false",
 		},
 		c.AddNetwork(
 			fmt.Sprintf("2.%d.%d.%d", n, n, n), // public IP
@@ -531,7 +535,7 @@ func TestSameLAN(t *testing.T) {
 // * client machine has a stateful host firewall (e.g. ufw)
 func TestBPFDisco(t *testing.T) {
 	nt := newNatTest(t)
-	nt.runTest(easyPMPFW, hard)
+	nt.runTest(easyPMPFWPlusBPF, hard)
 	nt.want(routeDirect)
 }
 

wgengine/magicsock/magicsock.go

@@ -508,13 +508,13 @@ func NewConn(opts Options) (*Conn, error) {
 	if d4, err := c.listenRawDisco("ip4"); err == nil {
 		c.logf("[v1] using BPF disco receiver for IPv4")
 		c.closeDisco4 = d4
-	} else {
+	} else if !errors.Is(err, errors.ErrUnsupported) {
 		c.logf("[v1] couldn't create raw v4 disco listener, using regular listener instead: %v", err)
 	}
 	if d6, err := c.listenRawDisco("ip6"); err == nil {
 		c.logf("[v1] using BPF disco receiver for IPv6")
 		c.closeDisco6 = d6
-	} else {
+	} else if !errors.Is(err, errors.ErrUnsupported) {
 		c.logf("[v1] couldn't create raw v6 disco listener, using regular listener instead: %v", err)
 	}
 

wgengine/magicsock/magicsock_default.go

@@ -7,6 +7,7 @@ package magicsock
 
 import (
 	"errors"
+	"fmt"
 	"io"
 
 	"tailscale.com/types/logger"
@@ -14,7 +15,7 @@ import (
 )
 
 func (c *Conn) listenRawDisco(family string) (io.Closer, error) {
-	return nil, errors.New("raw disco listening not supported on this OS")
+	return nil, fmt.Errorf("raw disco listening not supported on this OS: %w", errors.ErrUnsupported)
 }
 
 func trySetSocketBuffer(pconn nettype.PacketConn, logf logger.Logf) {

wgengine/magicsock/magicsock_linux.go

@@ -38,8 +38,11 @@ const (
 	discoMinHeaderSize = len(disco.Magic) + 32 /* key length */ + disco.NonceLen
 )
 
-// Enable/disable using raw sockets to receive disco traffic.
-var debugDisableRawDisco = envknob.RegisterBool("TS_DEBUG_DISABLE_RAW_DISCO")
+var (
+	// Opt-in for using raw sockets to receive disco traffic; added for
+	// #13140 and replaces the older "TS_DEBUG_DISABLE_RAW_DISCO".
+	envknobEnableRawDisco = envknob.RegisterBool("TS_ENABLE_RAW_DISCO")
+)
 
 // debugRawDiscoReads enables logging of raw disco reads.
 var debugRawDiscoReads = envknob.RegisterBool("TS_DEBUG_RAW_DISCO")
@@ -166,8 +169,12 @@ var (
 // and BPF filter.
 // https://github.com/tailscale/tailscale/issues/3824
 func (c *Conn) listenRawDisco(family string) (io.Closer, error) {
-	if debugDisableRawDisco() {
-		return nil, errors.New("raw disco listening disabled by debug flag")
+	if !envknobEnableRawDisco() {
+		// Return an 'errors.ErrUnsupported' to prevent the callee from
+		// logging; when we switch this to an opt-out (vs. an opt-in),
+		// drop the ErrUnsupported so that the callee logs that it was
+		// disabled.
+		return nil, fmt.Errorf("raw disco not enabled: %w", errors.ErrUnsupported)
 	}
 
 	// https://github.com/tailscale/tailscale/issues/5607