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..584be8cea --- /dev/null +++ b/client/iface/wgproxy/rawsocket/rawsocket_privileged_test.go @@ -0,0 +1,71 @@ +//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) + }{ + {"IPv4", PrepareSenderRawSocketIPv4}, + {"IPv6", PrepareSenderRawSocketIPv6}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + conn, err := tc.prepare() + if err != nil { + t.Skipf("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) + } + }) + } +}