From b7b0d5796e988ac5f369d51d3a16b162d5fb9522 Mon Sep 17 00:00:00 2001 From: Riccardo Manfrin <3090891+riccardomanfrin@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:09:09 +0200 Subject: [PATCH] [client] Bind netstack SOCKS5 proxy to 127.0.0.1 by default (#6812) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Describe your changes In netstack mode the SOCKS5 proxy bridges local host applications into the userspace WireGuard stack (`client/iface/netstack/proxy.go`), so it only needs to be reachable from the same machine. It was binding to `0.0.0.0`, making an unauthenticated proxy reachable from the network — any host able to reach the port could relay traffic through the client into its NetBird overlay. Bind to `127.0.0.1` by default. Add `NB_SOCKS5_LISTENER_ADDRESS` to override the bind host for the rare case the proxy must be reachable from other hosts (e.g. a container gateway); it is validated as an IP and falls back to loopback. `ListenAddr` is split into `listenHost`/`listenPort` helpers, with tests. Behavior change: setups that relied on reaching the netstack SOCKS5 proxy from another host must now set `NB_SOCKS5_LISTENER_ADDRESS=0.0.0.0` explicitly. ## Issue ticket number and link Internal security hardening of the netstack SOCKS5 listener bind address ([client/iface/netstack/env.go](https://github.com/netbirdio/netbird/blob/main/client/iface/netstack/env.go)). ## Stack - `0.74.7-branch` - :warning: No PR associated with branch - \#6812 :point\_left: ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [x] I added/updated documentation for this change - [ ] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from here: *** View with Codesmith Autofix with Codesmith Need help on this PR? Tag /codesmith with what you need. Autofix is disabled. ## Summary by CodeRabbit - **New Features** - Added configuration options for the SOCKS5 listener’s bind address and port. - SOCKS5 now defaults to listening only on the local machine for improved security. - Valid address and port overrides are supported, with safe defaults used for invalid values. - **Tests** - Added coverage for default settings and valid or invalid address and port configurations. --- client/iface/netstack/env.go | 55 +++++++++++++++++++++------ client/iface/netstack/env_test.go | 63 +++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 client/iface/netstack/env_test.go diff --git a/client/iface/netstack/env.go b/client/iface/netstack/env.go index dd8cf29a3..b069301c1 100644 --- a/client/iface/netstack/env.go +++ b/client/iface/netstack/env.go @@ -3,14 +3,31 @@ package netstack import ( - "fmt" + "net" "os" "strconv" log "github.com/sirupsen/logrus" ) -const EnvUseNetstackMode = "NB_USE_NETSTACK_MODE" +const ( + EnvUseNetstackMode = "NB_USE_NETSTACK_MODE" + + // EnvSocks5ListenerPort overrides the port the SOCKS5 proxy listens on. + EnvSocks5ListenerPort = "NB_SOCKS5_LISTENER_PORT" + + // EnvSocks5ListenerAddress overrides the host/IP the SOCKS5 proxy binds to. + // The proxy is a bridge for local host applications into the userspace + // WireGuard netstack, so it binds to loopback by default. Override this only + // when the proxy must be reachable from other hosts (e.g. a container + // gateway); doing so exposes an unauthenticated SOCKS5 proxy on that + // address. + EnvSocks5ListenerAddress = "NB_SOCKS5_LISTENER_ADDRESS" + + // defaultSocks5Host is the loopback address the SOCKS5 proxy binds to unless + // overridden via EnvSocks5ListenerAddress. + defaultSocks5Host = "127.0.0.1" +) // IsEnabled todo: move these function to cmd layer func IsEnabled() bool { @@ -18,24 +35,40 @@ func IsEnabled() bool { } func ListenAddr() string { - sPort := os.Getenv("NB_SOCKS5_LISTENER_PORT") + return net.JoinHostPort(listenHost(), strconv.Itoa(listenPort())) +} + +// listenHost returns the host/IP the SOCKS5 proxy binds to. It defaults to +// loopback and only honors EnvSocks5ListenerAddress when it holds a valid IP. +func listenHost() string { + addr := os.Getenv(EnvSocks5ListenerAddress) + if addr == "" { + return defaultSocks5Host + } + if net.ParseIP(addr) == nil { + log.Warnf("invalid socks5 listener address %q, falling back to default: %s", addr, defaultSocks5Host) + return defaultSocks5Host + } + return addr +} + +// listenPort returns the port the SOCKS5 proxy binds to, defaulting to +// DefaultSocks5Port when EnvSocks5ListenerPort is unset or invalid. +func listenPort() int { + sPort := os.Getenv(EnvSocks5ListenerPort) if sPort == "" { - return listenAddr(DefaultSocks5Port) + return DefaultSocks5Port } port, err := strconv.Atoi(sPort) if err != nil { log.Warnf("invalid socks5 listener port, unable to convert it to int, falling back to default: %d", DefaultSocks5Port) - return listenAddr(DefaultSocks5Port) + return DefaultSocks5Port } if port < 1 || port > 65535 { log.Warnf("invalid socks5 listener port, it should be in the range 1-65535, falling back to default: %d", DefaultSocks5Port) - return listenAddr(DefaultSocks5Port) + return DefaultSocks5Port } - return listenAddr(port) -} - -func listenAddr(port int) string { - return fmt.Sprintf("0.0.0.0:%d", port) + return port } diff --git a/client/iface/netstack/env_test.go b/client/iface/netstack/env_test.go new file mode 100644 index 000000000..1083435a4 --- /dev/null +++ b/client/iface/netstack/env_test.go @@ -0,0 +1,63 @@ +//go:build !js + +package netstack + +import ( + "net" + "strconv" + "testing" +) + +func TestListenAddr_DefaultsToLoopback(t *testing.T) { + // No env overrides: must bind loopback, never all interfaces. + got := ListenAddr() + want := net.JoinHostPort("127.0.0.1", strconv.Itoa(DefaultSocks5Port)) + if got != want { + t.Fatalf("ListenAddr() = %q, want %q", got, want) + } +} + +func TestListenAddr_AddressOverride(t *testing.T) { + tests := []struct { + name string + env string + want string + }{ + {name: "valid override honored", env: "0.0.0.0", want: "0.0.0.0"}, + {name: "valid specific ip honored", env: "10.0.0.5", want: "10.0.0.5"}, + {name: "ipv6 loopback bracketed", env: "::1", want: "::1"}, + {name: "invalid falls back to loopback", env: "not-an-ip", want: "127.0.0.1"}, + {name: "empty falls back to loopback", env: "", want: "127.0.0.1"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(EnvSocks5ListenerAddress, tc.env) + want := net.JoinHostPort(tc.want, strconv.Itoa(DefaultSocks5Port)) + if got := ListenAddr(); got != want { + t.Fatalf("ListenAddr() = %q, want %q", got, want) + } + }) + } +} + +func TestListenAddr_PortOverride(t *testing.T) { + tests := []struct { + name string + env string + want int + }{ + {name: "valid port honored", env: "1081", want: 1081}, + {name: "non-numeric falls back", env: "abc", want: DefaultSocks5Port}, + {name: "out of range falls back", env: "70000", want: DefaultSocks5Port}, + {name: "zero falls back", env: "0", want: DefaultSocks5Port}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(EnvSocks5ListenerPort, tc.env) + want := net.JoinHostPort("127.0.0.1", strconv.Itoa(tc.want)) + if got := ListenAddr(); got != want { + t.Fatalf("ListenAddr() = %q, want %q", got, want) + } + }) + } +}