From 672c2c8de86af4066f5b5694c7a1e8c0fbc28699 Mon Sep 17 00:00:00 2001
From: James Tucker <james@tailscale.com>
Date: Fri, 2 Sep 2022 10:33:46 -0700
Subject: [PATCH] wgengine/magicsock: add filter to ignore disco to old/other
 ports

Incoming disco packets are now dropped unless they match one of the
current bound ports, or have a zero port*.

The BPF filter passes all packets with a disco header to the raw packet
sockets regardless of destination port (in order to avoid needing to
reconfigure BPF on rebind).

If a BPF enabled node has just rebound, due to restart or rebind, it may
receive and reply to disco ping packets destined for ports other than
those which are presently bound. If the pong is accepted, the pinging
node will now assume that it can send WireGuard traffic to the pinged
port - such traffic will not reach the node as it is not destined for a
bound port.

*The zero port is ignored, if received. This is a speculative defense
and would indicate a problem in the receive path, or the BPF filter.
This condition is allowed to pass as it may enable traffic to flow,
however it will also enable problems with the same symptoms this patch
otherwise fixes.

Fixes #5536

Signed-off-by: James Tucker <james@tailscale.com>
---
 wgengine/magicsock/magicsock.go       |  9 +++++++++
 wgengine/magicsock/magicsock_linux.go | 28 +++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index cf3945816..a12ef1a31 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/wgengine/magicsock/magicsock.go
@@ -2991,11 +2991,13 @@ type RebindingUDPConn struct {
 
 	mu    sync.Mutex // held while changing pconn (and pconnAtomic)
 	pconn nettype.PacketConn
+	port  uint16
 }
 
 func (c *RebindingUDPConn) setConnLocked(p nettype.PacketConn) {
 	c.pconn = p
 	c.pconnAtomic.Store(p)
+	c.port = uint16(c.localAddrLocked().Port)
 }
 
 // currentConn returns c's current pconn, acquiring c.mu in the process.
@@ -3057,6 +3059,12 @@ func (c *RebindingUDPConn) ReadFromNetaddr(b []byte) (n int, ipp netip.AddrPort,
 	}
 }
 
+func (c *RebindingUDPConn) Port() uint16 {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	return c.port
+}
+
 func (c *RebindingUDPConn) LocalAddr() *net.UDPAddr {
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -3081,6 +3089,7 @@ func (c *RebindingUDPConn) closeLocked() error {
 	if c.pconn == nil {
 		return errNilPConn
 	}
+	c.port = 0
 	return c.pconn.Close()
 }
 
diff --git a/wgengine/magicsock/magicsock_linux.go b/wgengine/magicsock/magicsock_linux.go
index 796682de7..ae2c42c52 100644
--- a/wgengine/magicsock/magicsock_linux.go
+++ b/wgengine/magicsock/magicsock_linux.go
@@ -195,11 +195,11 @@ func (c *Conn) listenRawDisco(family string) (io.Closer, error) {
 	}
 	pc.SetReadDeadline(time.Time{})
 
-	go c.receiveDisco(pc)
+	go c.receiveDisco(pc, family == "ip6")
 	return pc, nil
 }
 
-func (c *Conn) receiveDisco(pc net.PacketConn) {
+func (c *Conn) receiveDisco(pc net.PacketConn, isIPV6 bool) {
 	var buf [1500]byte
 	for {
 		n, src, err := pc.ReadFrom(buf[:])
@@ -213,6 +213,30 @@ func (c *Conn) receiveDisco(pc net.PacketConn) {
 			// Too small to be a valid UDP datagram, drop.
 			continue
 		}
+
+		dstPort := binary.BigEndian.Uint16(buf[2:4])
+		if dstPort == 0 {
+			c.logf("[unexpected] disco raw: received packet for port 0")
+		}
+
+		var acceptPort uint16
+		if isIPV6 {
+			acceptPort = c.pconn6.Port()
+		} else {
+			acceptPort = c.pconn4.Port()
+		}
+		if acceptPort == 0 {
+			// This should only typically happen if the receiving address family
+			// was recently disabled.
+			c.logf("[v1] disco raw: dropping packet for port %d as acceptPort=0", dstPort)
+			continue
+		}
+
+		if dstPort != acceptPort {
+			c.logf("[v1] disco raw: dropping packet for port %d", dstPort)
+			continue
+		}
+
 		srcIP, ok := netip.AddrFromSlice(src.(*net.IPAddr).IP)
 		if !ok {
 			c.logf("[unexpected] PacketConn.ReadFrom returned not-an-IP %v in from", src)