From 62011d257eba8a19cbd2c979c60bf4c79e093c23 Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Thu, 28 May 2026 05:03:25 +0000 Subject: [PATCH] fix(gateway): add privilege check before loopback alias commands (PILOT-91) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit startProxy() now verifies process privileges before attempting to install loopback aliases via ip/ifconfig. When running as non-root the gateway previously logged an error but continued as if setup succeeded — subsequent connects failed in mysterious ways because no alias was actually installed. Changes: - Add loopbackPrivilegeCheck (euid gate, overridable in tests) - startProxy() bails out with a clear error if either the privilege check or the actual alias command fails - addLoopbackAlias() now returns error instead of silently logging - Tests bypass the privilege gate where they use exec stubs Closes PILOT-91 --- gateway.go | 21 ++++++++++++++++++++- loopback_darwin.go | 6 ++++-- loopback_linux.go | 6 ++++-- loopback_other.go | 5 +++-- zz_coverage_test.go | 19 +++++++++++++++++-- 5 files changed, 48 insertions(+), 9 deletions(-) diff --git a/gateway.go b/gateway.go index 16506ab..a4b30d6 100644 --- a/gateway.go +++ b/gateway.go @@ -7,6 +7,7 @@ import ( "io" "log/slog" "net" + "os" "sync" "github.com/TeoSlayer/pilotprotocol/pkg/protocol" @@ -42,6 +43,15 @@ type Gateway struct { done chan struct{} } +// loopbackPrivilegeCheck verifies the process can manage loopback aliases. +// Overridden in tests to bypass the OS euid gate. +var loopbackPrivilegeCheck = func() error { + if os.Geteuid() != 0 { + return fmt.Errorf("loopback alias management requires root (or CAP_NET_ADMIN on Linux); effective UID %d — restart gateway with sudo or grant ambient capabilities", os.Geteuid()) + } + return nil +} + // New creates a new Gateway bound to the given Dialer. The Dialer is // typically a *driver.Driver constructed by cmd/gateway. func New(cfg Config, d Dialer) (*Gateway, error) { @@ -156,7 +166,16 @@ func (gw *Gateway) Unmap(localIP string) error { } func (gw *Gateway) startProxy(localIP net.IP, pilotAddr protocol.Addr) { - gw.addLoopbackAlias(localIP) + if err := loopbackPrivilegeCheck(); err != nil { + slog.Error("gateway startProxy: privilege check failed — loopback alias not created", + "err", err) + return + } + if err := gw.addLoopbackAlias(localIP); err != nil { + slog.Error("gateway startProxy: loopback alias setup failed — proxy not started", + "ip", localIP, "pilot_addr", pilotAddr, "err", err) + return + } gw.mu.Lock() gw.aliases = append(gw.aliases, localIP) diff --git a/loopback_darwin.go b/loopback_darwin.go index d988f02..79b1656 100644 --- a/loopback_darwin.go +++ b/loopback_darwin.go @@ -3,15 +3,17 @@ package gateway import ( + "fmt" "log/slog" "net" "os/exec" ) -func (gw *Gateway) addLoopbackAlias(ip net.IP) { +func (gw *Gateway) addLoopbackAlias(ip net.IP) error { if err := exec.Command("ifconfig", "lo0", "alias", ip.String()).Run(); err != nil { - slog.Error("addLoopbackAlias failed", "ip", ip, "os", "darwin", "err", err) + return fmt.Errorf("ifconfig lo0 alias %s: %w", ip, err) } + return nil } func (gw *Gateway) removeLoopbackAlias(ip net.IP) { diff --git a/loopback_linux.go b/loopback_linux.go index c5e95d3..4e29fd4 100644 --- a/loopback_linux.go +++ b/loopback_linux.go @@ -3,15 +3,17 @@ package gateway import ( + "fmt" "log/slog" "net" "os/exec" ) -func (gw *Gateway) addLoopbackAlias(ip net.IP) { +func (gw *Gateway) addLoopbackAlias(ip net.IP) error { if err := exec.Command("ip", "addr", "add", ip.String()+"/32", "dev", "lo").Run(); err != nil { - slog.Error("addLoopbackAlias failed", "ip", ip, "os", "linux", "err", err) + return fmt.Errorf("ip addr add %s/32 dev lo: %w", ip, err) } + return nil } func (gw *Gateway) removeLoopbackAlias(ip net.IP) { diff --git a/loopback_other.go b/loopback_other.go index 350b0fb..e7b2b45 100644 --- a/loopback_other.go +++ b/loopback_other.go @@ -5,13 +5,14 @@ package gateway import ( + "fmt" "log/slog" "net" "runtime" ) -func (gw *Gateway) addLoopbackAlias(ip net.IP) { - slog.Error("addLoopbackAlias: unsupported OS", "ip", ip, "os", runtime.GOOS) +func (gw *Gateway) addLoopbackAlias(ip net.IP) error { + return fmt.Errorf("loopback alias unsupported on %s", runtime.GOOS) } func (gw *Gateway) removeLoopbackAlias(ip net.IP) { diff --git a/zz_coverage_test.go b/zz_coverage_test.go index 13d8c97..c3f2382 100644 --- a/zz_coverage_test.go +++ b/zz_coverage_test.go @@ -279,6 +279,12 @@ func TestMap_StartsProxyWithStubs(t *testing.T) { } withExecStubs(t, true) + // Bypass the privilege check so the test can exercise the real path + // through addLoopbackAlias without requiring root. + old := loopbackPrivilegeCheck + loopbackPrivilegeCheck = func() error { return nil } + defer func() { loopbackPrivilegeCheck = old }() + // Use ports=[] so listenPort never actually attempts a bind — we only // need to exercise Map → startProxy → addLoopbackAlias. (The bind path // is already covered by TestListenPort_AcceptsAndBridges.) @@ -326,6 +332,10 @@ func TestMap_ExplicitIPWithStubs(t *testing.T) { } withExecStubs(t, true) + old := loopbackPrivilegeCheck + loopbackPrivilegeCheck = func() error { return nil } + defer func() { loopbackPrivilegeCheck = old }() + d := &pipeDialerSync{} gw, err := New(Config{Subnet: "10.66.0.0/16", Ports: []uint16{}}, d) if err != nil { @@ -401,7 +411,10 @@ func TestAddRemoveLoopbackAlias_StubExec(t *testing.T) { // Both must complete without panic. Errors from the stub (rc=0) are // expected to be nil; logging is the only side effect we observe. - gw.addLoopbackAlias(ip) + // The function now returns an error; with stubs it should succeed. + if err := gw.addLoopbackAlias(ip); err != nil { + t.Fatalf("addLoopbackAlias with successful stub: %v", err) + } gw.removeLoopbackAlias(ip) } @@ -420,7 +433,9 @@ func TestAddRemoveLoopbackAlias_StubExecFailure(t *testing.T) { t.Fatalf("New: %v", err) } ip := net.ParseIP("10.4.0.200") - gw.addLoopbackAlias(ip) // logs error + if err := gw.addLoopbackAlias(ip); err != nil { + t.Logf("addLoopbackAlias (rc=1 stub) returned error: %v", err) + } gw.removeLoopbackAlias(ip) // logs error }