diff --git a/client/firewall/nftables/router_linux.go b/client/firewall/nftables/router_linux.go index d3e031c5f..c79f9b8c2 100644 --- a/client/firewall/nftables/router_linux.go +++ b/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 } diff --git a/client/iface/configurer/usp.go b/client/iface/configurer/usp.go index 0a25c55bc..2be1b861e 100644 --- a/client/iface/configurer/usp.go +++ b/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 } diff --git a/client/iface/wgproxy/rawsocket/rawsocket.go b/client/iface/wgproxy/rawsocket/rawsocket.go index bc785b43a..37aaa160f 100644 --- a/client/iface/wgproxy/rawsocket/rawsocket.go +++ b/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)) diff --git a/client/iface/wgproxy/rawsocket/rawsocket_privileged_test.go b/client/iface/wgproxy/rawsocket/rawsocket_privileged_test.go new file mode 100644 index 000000000..03748c6f9 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/client/net/fwmark.go b/client/net/fwmark.go new file mode 100644 index 000000000..b526feee4 --- /dev/null +++ b/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 +} diff --git a/client/net/fwmark_test.go b/client/net/fwmark_test.go new file mode 100644 index 000000000..2dbebec2a --- /dev/null +++ b/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) + } + } +} diff --git a/client/net/net.go b/client/net/net.go index a97de9d59..77fba36d1 100644 --- a/client/net/net.go +++ b/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() diff --git a/client/net/net_linux.go b/client/net/net_linux.go index 9e7d13702..8ed8a1944 100644 --- a/client/net/net_linux.go +++ b/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)) }