From 40833a752498206b0fe3cb076881343e4bb77dd9 Mon Sep 17 00:00:00 2001
From: Andrew Dunham <andrew@du.nham.ca>
Date: Mon, 16 Sep 2024 11:27:04 -0400
Subject: [PATCH] wgengine/magicsock: disable raw disco by default; add envknob
 to enable

Updates #13140

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ica85b2ac8ac7eab4ec5413b212f004aecc453279
---
 tstest/integration/nat/nat_test.go      | 14 +++++++++-----
 wgengine/magicsock/magicsock.go         |  4 ++--
 wgengine/magicsock/magicsock_default.go |  3 ++-
 wgengine/magicsock/magicsock_linux.go   | 15 +++++++++++----
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/tstest/integration/nat/nat_test.go b/tstest/integration/nat/nat_test.go
index a8c20d6bd..535515588 100644
--- a/tstest/integration/nat/nat_test.go
+++ b/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)
 }
 
diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index c9f032371..221d2d62f 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/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)
 	}
 
diff --git a/wgengine/magicsock/magicsock_default.go b/wgengine/magicsock/magicsock_default.go
index 321765b8c..7614c64c9 100644
--- a/wgengine/magicsock/magicsock_default.go
+++ b/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) {
diff --git a/wgengine/magicsock/magicsock_linux.go b/wgengine/magicsock/magicsock_linux.go
index f658c016b..c5df555cd 100644
--- a/wgengine/magicsock/magicsock_linux.go
+++ b/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