[client] Keep NetBird traffic out of third-party fwmark rules (#7314)

client/firewall/nftables/router_linux.go

@@ -763,7 +763,7 @@ func (r *router) addNatRule(pair firewall.RouterPair) error {
 	exprs = append(exprs, sourceExp...)
 	exprs = append(exprs, destExp...)
 
-	var markValue uint32 = nbnet.PreroutingFwmarkMasquerade
+	markValue := nbnet.PreroutingFwmarkMasquerade
 	if pair.Inverse {
 		markValue = nbnet.PreroutingFwmarkMasqueradeReturn
 	}

client/iface/configurer/usp.go

@@ -502,7 +502,7 @@ func toBytes(s string) (int64, error) {
 
 func getFwmark() int {
 	if nbnet.AdvancedRouting() && runtime.GOOS == "linux" {
-		return nbnet.ControlPlaneMark
+		return int(nbnet.ControlPlaneMark)
 	}
 	return 0
 }

client/iface/wgproxy/rawsocket/rawsocket.go

@@ -10,8 +10,6 @@ import (
 
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
-
-	nbnet "github.com/netbirdio/netbird/client/net"
 )
 
 // PrepareSenderRawSocketIPv4 creates and configures a raw socket for sending IPv4 packets
@@ -60,14 +58,12 @@ func prepareSenderRawSocket(family int, isIPv4 bool) (net.PacketConn, error) {
 		return nil, fmt.Errorf("binding to lo interface failed: %w", err)
 	}
 
-	// Set the fwmark on the socket.
-	err = nbnet.SetSocketOpt(fd)
-	if err != nil {
-		if closeErr := syscall.Close(fd); closeErr != nil {
-			log.Warnf("failed to close raw socket fd: %v", closeErr)
-		}
-		return nil, fmt.Errorf("setting fwmark failed: %w", err)
-	}
+	// The socket is bound to lo and only ever sends to the local WireGuard
+	// instance, a destination the local routing table resolves without help, so
+	// it carries no fwmark. Staying unmarked also keeps these packets out of
+	// third-party NAT rules that match on marks: such a rule rewriting the
+	// source would make WireGuard adopt the rewritten address as the peer
+	// endpoint.
 
 	// Convert the file descriptor to a PacketConn.
 	file := os.NewFile(uintptr(fd), fmt.Sprintf("fd %d", fd))

client/iface/wgproxy/rawsocket/rawsocket_privileged_test.go

@@ -0,0 +1,77 @@
+//go:build linux && !android && privileged
+
+package rawsocket
+
+import (
+	"net"
+	"syscall"
+	"testing"
+
+	"golang.org/x/sys/unix"
+
+	nbnet "github.com/netbirdio/netbird/client/net"
+)
+
+// The sender sockets must stay unmarked: a NAT rule matching on fwmark that
+// rewrites the source of an injected packet makes WireGuard adopt the rewritten
+// address as the peer endpoint.
+func TestSenderRawSocketsCarryNoFwmark(t *testing.T) {
+	// the mark is only ever applied when advanced routing is available, so
+	// without it the assertion below would hold for the wrong reason
+	nbnet.Init()
+	if !nbnet.AdvancedRouting() {
+		t.Skip("advanced routing unsupported, the sockets carry no mark either way")
+	}
+
+	tests := []struct {
+		name    string
+		prepare func() (net.PacketConn, error)
+		// the proxy treats the IPv6 socket as optional, so a host without IPv6
+		// is a reason to skip rather than to fail
+		optional bool
+	}{
+		{name: "IPv4", prepare: PrepareSenderRawSocketIPv4},
+		{name: "IPv6", prepare: PrepareSenderRawSocketIPv6, optional: true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			conn, err := tc.prepare()
+			if err != nil {
+				if tc.optional {
+					t.Skipf("prepare raw socket: %v", err)
+				}
+				t.Fatalf("prepare raw socket: %v", err)
+			}
+			defer func() {
+				if err := conn.Close(); err != nil {
+					t.Logf("close raw socket: %v", err)
+				}
+			}()
+
+			syscallConn, ok := conn.(syscall.Conn)
+			if !ok {
+				t.Fatalf("raw socket %T does not expose a syscall conn", conn)
+			}
+			raw, err := syscallConn.SyscallConn()
+			if err != nil {
+				t.Fatalf("syscall conn: %v", err)
+			}
+
+			var mark int
+			var markErr error
+			if err := raw.Control(func(fd uintptr) {
+				mark, markErr = unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_MARK)
+			}); err != nil {
+				t.Fatalf("control: %v", err)
+			}
+			if markErr != nil {
+				t.Fatalf("get SO_MARK: %v", markErr)
+			}
+
+			if mark != 0 {
+				t.Errorf("SO_MARK = %#x, want 0", mark)
+			}
+		})
+	}
+}

client/net/fwmark.go

@@ -0,0 +1,110 @@
+package net
+
+import (
+	"fmt"
+	"os"
+	"strconv"
+	"strings"
+
+	log "github.com/sirupsen/logrus"
+)
+
+const (
+	// envFwmarkBase overrides the base of the fwmark range. Container network
+	// plugins, CNIs and other VPNs claim bits of the mark space for themselves,
+	// and a rule of theirs matching one of our bits acts on our traffic, so
+	// hosts running such software may need to move the range out of the way.
+	envFwmarkBase = "NB_FWMARK_BASE"
+
+	// defaultFwmarkBase is the base of the fwmark range used when the
+	// environment does not override it.
+	defaultFwmarkBase uint32 = 0x1BD00
+
+	// fwmarkOffsetMask is the part of a mark that identifies the individual mark
+	// within the range, so the base occupies everything above it.
+	fwmarkOffsetMask uint32 = 0xFF
+)
+
+// Offsets of the individual marks within the range.
+const (
+	offsetControlPlane     uint32 = 0x00
+	offsetDataPlaneIn      uint32 = 0x10
+	offsetDataPlaneOut     uint32 = 0x11
+	offsetRedirected       uint32 = 0x20
+	offsetMasquerade       uint32 = 0x21
+	offsetMasqueradeReturn uint32 = 0x22
+	offsetDataPlaneLower   uint32 = 0x10
+	offsetDataPlaneUpper   uint32 = fwmarkOffsetMask
+)
+
+var (
+	fwmarkBase = loadFwmarkBase()
+
+	// ControlPlaneMark is the fwmark value used to mark packets that should not be routed through the NetBird interface to
+	// avoid routing loops.
+	// This includes all control plane traffic (mgmt, signal, flows), relay, ICE/stun/turn and everything that is emitted by the wireguard socket.
+	// It doesn't collide with the other marks, as the others are used for data plane traffic only.
+	ControlPlaneMark = fwmarkBase | offsetControlPlane
+
+	// DataPlaneMarkLower is the lowest value for the data plane range
+	DataPlaneMarkLower = fwmarkBase | offsetDataPlaneLower
+	// DataPlaneMarkUpper is the highest value for the data plane range
+	DataPlaneMarkUpper = fwmarkBase | offsetDataPlaneUpper
+
+	// DataPlaneMarkIn is the mark for inbound data plane traffic.
+	DataPlaneMarkIn = fwmarkBase | offsetDataPlaneIn
+
+	// DataPlaneMarkOut is the mark for outbound data plane traffic.
+	DataPlaneMarkOut = fwmarkBase | offsetDataPlaneOut
+
+	// PreroutingFwmarkRedirected is applied to packets that were redirected (input -> forward, e.g. by Docker or Podman) for special handling.
+	PreroutingFwmarkRedirected = fwmarkBase | offsetRedirected
+
+	// PreroutingFwmarkMasquerade is applied to packets that arrive from the NetBird interface and should be masqueraded.
+	PreroutingFwmarkMasquerade = fwmarkBase | offsetMasquerade
+
+	// PreroutingFwmarkMasqueradeReturn is applied to packets that will leave through the NetBird interface and should be masqueraded.
+	PreroutingFwmarkMasqueradeReturn = fwmarkBase | offsetMasqueradeReturn
+)
+
+// IsDataPlaneMark determines if a fwmark is in the data plane range.
+func IsDataPlaneMark(fwmark uint32) bool {
+	return fwmark >= DataPlaneMarkLower && fwmark <= DataPlaneMarkUpper
+}
+
+func loadFwmarkBase() uint32 {
+	val := os.Getenv(envFwmarkBase)
+	if val == "" {
+		return defaultFwmarkBase
+	}
+
+	base, err := parseFwmarkBase(val)
+	if err != nil {
+		log.Warnf("failed to parse %s=%q, using the default range: %v", envFwmarkBase, val, err)
+		return defaultFwmarkBase
+	}
+
+	log.Infof("using fwmark range %#x-%#x from %s", base, base|fwmarkOffsetMask, envFwmarkBase)
+	return base
+}
+
+// parseFwmarkBase reads a mark range base. The low byte of a mark identifies the
+// individual mark within the range, so a base has to leave it free.
+func parseFwmarkBase(val string) (uint32, error) {
+	val = strings.TrimSpace(val)
+
+	base, err := strconv.ParseUint(val, 0, 32)
+	if err != nil {
+		return 0, fmt.Errorf("not a 32 bit number: %w", err)
+	}
+
+	if base == 0 {
+		return 0, fmt.Errorf("base must not be zero")
+	}
+
+	if uint32(base)&fwmarkOffsetMask != 0 {
+		return 0, fmt.Errorf("base %#x must leave the low byte free", base)
+	}
+
+	return uint32(base), nil
+}

client/net/fwmark_test.go

@@ -0,0 +1,111 @@
+package net
+
+import (
+	"testing"
+)
+
+func TestParseFwmarkBase(t *testing.T) {
+	tests := []struct {
+		name    string
+		val     string
+		want    uint32
+		wantErr bool
+	}{
+		{name: "hex", val: "0x5A000", want: 0x5A000},
+		{name: "hex upper case", val: "0X5A000", want: 0x5A000},
+		{name: "decimal", val: "65536", want: 65536},
+		{name: "octal", val: "0o400", want: 0o400},
+		{name: "surrounding space", val: "  0x5A000 ", want: 0x5A000},
+		{name: "highest usable base", val: "0xFFFFFF00", want: 0xFFFFFF00},
+		{name: "low byte in use", val: "0x1BD01", wantErr: true},
+		{name: "zero", val: "0", wantErr: true},
+		{name: "not a number", val: "wireguard", wantErr: true},
+		{name: "wider than 32 bit", val: "0x1FFFFFFFF", wantErr: true},
+		{name: "negative", val: "-0x100", wantErr: true},
+		{name: "empty", val: "", wantErr: true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := parseFwmarkBase(tc.val)
+			if tc.wantErr {
+				if err == nil {
+					t.Fatalf("parseFwmarkBase(%q) = %#x, want an error", tc.val, got)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("parseFwmarkBase(%q): %v", tc.val, err)
+			}
+			if got != tc.want {
+				t.Errorf("parseFwmarkBase(%q) = %#x, want %#x", tc.val, got, tc.want)
+			}
+		})
+	}
+}
+
+// The marks have to stay inside the range the base defines, otherwise a host
+// that moved the range to dodge a collision would still emit the old values.
+func TestMarksStayWithinTheRange(t *testing.T) {
+	lower, upper := fwmarkBase, fwmarkBase|fwmarkOffsetMask
+
+	marks := map[string]uint32{
+		"ControlPlaneMark":                 ControlPlaneMark,
+		"DataPlaneMarkLower":               DataPlaneMarkLower,
+		"DataPlaneMarkUpper":               DataPlaneMarkUpper,
+		"DataPlaneMarkIn":                  DataPlaneMarkIn,
+		"DataPlaneMarkOut":                 DataPlaneMarkOut,
+		"PreroutingFwmarkRedirected":       PreroutingFwmarkRedirected,
+		"PreroutingFwmarkMasquerade":       PreroutingFwmarkMasquerade,
+		"PreroutingFwmarkMasqueradeReturn": PreroutingFwmarkMasqueradeReturn,
+	}
+
+	for name, mark := range marks {
+		if mark < lower || mark > upper {
+			t.Errorf("%s = %#x, outside the range %#x-%#x", name, mark, lower, upper)
+		}
+	}
+
+	// the control plane mark must stay out of the data plane range, the netflow
+	// conntrack path tells them apart by it
+	if IsDataPlaneMark(ControlPlaneMark) {
+		t.Errorf("ControlPlaneMark %#x is inside the data plane range", ControlPlaneMark)
+	}
+	for name, mark := range map[string]uint32{
+		"DataPlaneMarkIn":                  DataPlaneMarkIn,
+		"DataPlaneMarkOut":                 DataPlaneMarkOut,
+		"PreroutingFwmarkRedirected":       PreroutingFwmarkRedirected,
+		"PreroutingFwmarkMasquerade":       PreroutingFwmarkMasquerade,
+		"PreroutingFwmarkMasqueradeReturn": PreroutingFwmarkMasqueradeReturn,
+	} {
+		if !IsDataPlaneMark(mark) {
+			t.Errorf("%s = %#x is outside the data plane range %#x-%#x", name, mark, DataPlaneMarkLower, DataPlaneMarkUpper)
+		}
+	}
+}
+
+func TestDefaultMarksAreUnchanged(t *testing.T) {
+	tests := map[string]struct {
+		got  uint32
+		want uint32
+	}{
+		"ControlPlaneMark":                 {ControlPlaneMark, 0x1BD00},
+		"DataPlaneMarkLower":               {DataPlaneMarkLower, 0x1BD10},
+		"DataPlaneMarkUpper":               {DataPlaneMarkUpper, 0x1BDFF},
+		"DataPlaneMarkIn":                  {DataPlaneMarkIn, 0x1BD10},
+		"DataPlaneMarkOut":                 {DataPlaneMarkOut, 0x1BD11},
+		"PreroutingFwmarkRedirected":       {PreroutingFwmarkRedirected, 0x1BD20},
+		"PreroutingFwmarkMasquerade":       {PreroutingFwmarkMasquerade, 0x1BD21},
+		"PreroutingFwmarkMasqueradeReturn": {PreroutingFwmarkMasqueradeReturn, 0x1BD22},
+	}
+
+	if fwmarkBase != defaultFwmarkBase {
+		t.Skipf("%s is set, the defaults do not apply", envFwmarkBase)
+	}
+
+	for name, tc := range tests {
+		if tc.got != tc.want {
+			t.Errorf("%s = %#x, want %#x", name, tc.got, tc.want)
+		}
+	}
+}

client/net/net.go

@@ -7,41 +7,6 @@ import (
 	"net/netip"
 )
 
-const (
-	// ControlPlaneMark is the fwmark value used to mark packets that should not be routed through the NetBird interface to
-	// avoid routing loops.
-	// This includes all control plane traffic (mgmt, signal, flows), relay, ICE/stun/turn and everything that is emitted by the wireguard socket.
-	// It doesn't collide with the other marks, as the others are used for data plane traffic only.
-	ControlPlaneMark = 0x1BD00
-
-	// Data plane marks (0x1BD10 - 0x1BDFF)
-
-	// DataPlaneMarkLower is the lowest value for the data plane range
-	DataPlaneMarkLower = 0x1BD10
-	// DataPlaneMarkUpper is the highest value for the data plane range
-	DataPlaneMarkUpper = 0x1BDFF
-
-	// DataPlaneMarkIn is the mark for inbound data plane traffic.
-	DataPlaneMarkIn = 0x1BD10
-
-	// DataPlaneMarkOut is the mark for outbound data plane traffic.
-	DataPlaneMarkOut = 0x1BD11
-
-	// PreroutingFwmarkRedirected is applied to packets that are were redirected (input -> forward, e.g. by Docker or Podman) for special handling.
-	PreroutingFwmarkRedirected = 0x1BD20
-
-	// PreroutingFwmarkMasquerade is applied to packets that arrive from the NetBird interface and should be masqueraded.
-	PreroutingFwmarkMasquerade = 0x1BD21
-
-	// PreroutingFwmarkMasqueradeReturn is applied to packets that will leave through the NetBird interface and should be masqueraded.
-	PreroutingFwmarkMasqueradeReturn = 0x1BD22
-)
-
-// IsDataPlaneMark determines if a fwmark is in the data plane range (0x1BD10-0x1BDFF)
-func IsDataPlaneMark(fwmark uint32) bool {
-	return fwmark >= DataPlaneMarkLower && fwmark <= DataPlaneMarkUpper
-}
-
 func GetLastIPFromNetwork(network netip.Prefix, fromEnd int) (netip.Addr, error) {
 	var endIP net.IP
 	addr := network.Addr().AsSlice()

client/net/net_linux.go

@@ -21,15 +21,6 @@ func SetSocketMark(conn syscall.Conn) error {
 	return setRawSocketMark(sysconn)
 }
 
-// SetSocketOpt sets the SO_MARK option on the given file descriptor
-func SetSocketOpt(fd int) error {
-	if !AdvancedRouting() {
-		return nil
-	}
-
-	return setSocketOptInt(fd)
-}
-
 func setRawSocketMark(conn syscall.RawConn) error {
 	var setErr error
 
@@ -51,5 +42,5 @@ func setRawSocketMark(conn syscall.RawConn) error {
 }
 
 func setSocketOptInt(fd int) error {
-	return syscall.SetsockoptInt(fd, syscall.SOL_SOCKET, syscall.SO_MARK, ControlPlaneMark)
+	return syscall.SetsockoptInt(fd, syscall.SOL_SOCKET, syscall.SO_MARK, int(ControlPlaneMark))
 }