From 9700e610c02634fcd18761761ea9b908218ca783 Mon Sep 17 00:00:00 2001 From: Andrew Dunham 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 Change-Id: Ica85b2ac8ac7eab4ec5413b212f004aecc453279 (cherry picked from commit 40833a752498206b0fe3cb076881343e4bb77dd9) --- 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 de6b13fc1..a056cff22 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 @@ import ( "errors" + "fmt" "io" "tailscale.com/types/logger" @@ -14,7 +15,7 @@ ) 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 @@ 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 @@ // 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