From 1fb8521942dfd0aab577e4417d0e8b8b9aa7d21d Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:50:19 +0800 Subject: [PATCH] fix(envd/port): detect :::PORT wildcard listeners and fix dual-stack key collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three bugs in the port forwarder left services bound to the IPv6 wildcard address (:::PORT) silently unreachable from outside the sandbox: - Scan filter missing "::": on a dual-stack kernel many frameworks bind "::" rather than "0.0.0.0", and gopsutil reports Laddr.IP = "::"; it was not in the forwarder ScannerFilter, so the port was never detected and no socat started. Add "::" to the filter and normalize a "::" wildcard to IPv4 so socat connects via 127.0.0.1. - Port key collision: the ports map key was "pid-port", so a service listening on both 127.0.0.1:PORT and ::1:PORT produced one key and the second socat overwrote the first — one address went dark. Include the IP in the key ("pid-port-ip"). ImportForwards normalizes old single-dash keys from a live upgrade to the new format so no duplicate socat is spawned. - TCP%d:localhost backend: for family=6 this is TCP6:localhost, which needs /etc/hosts to contain "::1 localhost"; Alpine and many minimal images only have 127.0.0.1 localhost, so resolution failed. Use literal 127.0.0.1 / [::1]. Rebased onto current main (the earlier attempt #3588 went stale after envd was refactored). Scoped to forward.go and its tests; no version bump. Fixes #3586. --- packages/envd/internal/port/forward.go | 56 ++++++++++++++-- .../envd/internal/port/forward_frozen_test.go | 6 +- .../internal/port/forward_handover_test.go | 7 +- .../envd/internal/port/ipv6_wildcard_test.go | 64 +++++++++++++++++++ 4 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 packages/envd/internal/port/ipv6_wildcard_test.go diff --git a/packages/envd/internal/port/forward.go b/packages/envd/internal/port/forward.go index 7dc151dbad..952a510e51 100644 --- a/packages/envd/internal/port/forward.go +++ b/packages/envd/internal/port/forward.go @@ -11,6 +11,7 @@ import ( "fmt" "net" "os/exec" + "strings" "sync" "syscall" @@ -81,7 +82,10 @@ func NewForwarder( "port-forwarder", // We only want to forward ports that are actively listening on localhost. &ScannerFilter{ - IPs: []string{"127.0.0.1", "localhost", "::1"}, + // "::" is included for IPv6 wildcard sockets (:::PORT): on a dual-stack + // kernel many frameworks bind "::" rather than "0.0.0.0", so gopsutil + // reports Laddr.IP = "::" for those listeners. + IPs: []string{"127.0.0.1", "localhost", "::1", "::"}, State: "LISTEN", }, ) @@ -129,7 +133,10 @@ func (f *Forwarder) StartForwarding(ctx context.Context) { // Let's refresh our map of currently forwarded ports and mark the currently opened ones with the "FORWARD" state. // This will make sure we won't delete them later. for _, p := range procs { - key := fmt.Sprintf("%d-%d", p.Pid, p.Laddr.Port) + // Include IP in the key so a service listening on both 127.0.0.1:PORT + // and ::1:PORT gets two independent socats instead of the second + // entry silently overwriting the first. + key := fmt.Sprintf("%d-%d-%s", p.Pid, p.Laddr.Port, p.Laddr.IP) // We check if the opened port is in our map of forwarded ports. val, portOk := f.ports[key] @@ -138,10 +145,18 @@ func (f *Forwarder) StartForwarding(ctx context.Context) { // The actual socat process that handles forwarding should be running from the last iteration. val.state = PortStateForward } else { + // A "::" wildcard socket accepts both IPv4 and IPv6; connect via + // IPv4 so socat targets 127.0.0.1 and does not depend on /etc/hosts + // containing "::1 localhost" (absent on minimal images). + family := familyToIPVersion(p.Family) + if p.Laddr.IP == "::" { + family = 4 + } + f.logger.Debug(). Str("ip", p.Laddr.IP). Uint32("port", p.Laddr.Port). - Uint32("family", familyToIPVersion(p.Family)). + Uint32("family", family). Str("state", p.Status). Msg("Detected new opened port on localhost that is not forwarded") @@ -150,7 +165,7 @@ func (f *Forwarder) StartForwarding(ctx context.Context) { pid: p.Pid, port: p.Laddr.Port, state: PortStateForward, - family: familyToIPVersion(p.Family), + family: family, } f.ports[key] = ptf if retry := f.startPortForwarding(ctx, ptf); retry { @@ -186,7 +201,10 @@ func (f *Forwarder) startPortForwarding(ctx context.Context, p *PortToForward) ( cmd := exec.CommandContext(ctx, "socat", "-d", "-d", "-d", fmt.Sprintf("TCP4-LISTEN:%v,bind=%s,reuseaddr,fork", p.port, f.sourceIP.To4()), - fmt.Sprintf("TCP%d:localhost:%v", p.family, p.port), + // Use a literal backend address rather than "localhost" so socat does + // not depend on /etc/hosts resolving "::1 localhost" (absent on Alpine + // and many minimal images). + fmt.Sprintf("TCP%d:%s:%v", p.family, backendAddr(p.family), p.port), ) cgroupFD, ok := f.freezer.Manager().GetFileDescriptor(cgroups.ProcessTypeSocat) @@ -344,7 +362,7 @@ func (f *Forwarder) ImportForwards(forwards []*upgrade.ForwardedPort) (readopted if syscall.Kill(pid, 0) != nil { continue } - f.ports[fp.GetKey()] = &PortToForward{ + f.ports[normalizeForwardKey(fp)] = &PortToForward{ pid: fp.GetListenerPid(), port: fp.GetPort(), family: fp.GetFamily(), @@ -358,6 +376,32 @@ func (f *Forwarder) ImportForwards(forwards []*upgrade.ForwardedPort) (readopted return readopted } +// backendAddr returns the literal loopback address socat should connect to +// for the given IP family, avoiding reliance on "localhost" name resolution. +func backendAddr(family uint32) string { + if family == 6 { + return "[::1]" + } + return "127.0.0.1" +} + +// normalizeForwardKey returns the canonical map key for a re-adopted port. +// Old envd binaries exported Key as "-" (one dash); new binaries +// use "--" (two dashes). When the old format is detected the +// IP is inferred from Family so the seeded key matches what the scan loop +// produces, preventing a duplicate socat after a live upgrade. +func normalizeForwardKey(fp *upgrade.ForwardedPort) string { + key := fp.GetKey() + if strings.Count(key, "-") == 1 { + ip := "127.0.0.1" + if fp.GetFamily() == 6 { + ip = "::1" + } + key += "-" + ip + } + return key +} + func familyToIPVersion(family uint32) uint32 { switch family { case syscall.AF_INET: diff --git a/packages/envd/internal/port/forward_frozen_test.go b/packages/envd/internal/port/forward_frozen_test.go index 29fb8a4564..907652ff3f 100644 --- a/packages/envd/internal/port/forward_frozen_test.go +++ b/packages/envd/internal/port/forward_frozen_test.go @@ -45,7 +45,7 @@ func (m *socatCgroupManager) FrozenAt(string) (bool, error) { return m.frozen.Lo // TestStartForwarding_RetriesAPortSkippedWhileFrozen pins that a refused forward is not // remembered as a forwarded one. The scan loop records a port before starting its socat, -// and the map is keyed by listener pid+port; left in place after a refusal, the entry reads +// and the map is keyed by listener pid+port+ip; left in place after a refusal, the entry reads // as forwarded on every later scan and the port stays dark until the listener reopens. func TestStartForwarding_RetriesAPortSkippedWhileFrozen(t *testing.T) { t.Parallel() @@ -71,7 +71,9 @@ func TestStartForwarding_RetriesAPortSkippedWhileFrozen(t *testing.T) { Status: "LISTEN", Pid: 4242, } - key := fmt.Sprintf("%d-%d", listener.Pid, listener.Laddr.Port) + // Key includes the listener IP (pid-port-ip) so dual-stack listeners get + // independent entries; mirror that here when looking the entry back up. + key := fmt.Sprintf("%d-%d-%s", listener.Pid, listener.Laddr.Port, listener.Laddr.IP) scan := func() { f.scannerSubscriber.Messages <- []net.ConnectionStat{listener} } tracked := func() bool { f.mu.Lock() diff --git a/packages/envd/internal/port/forward_handover_test.go b/packages/envd/internal/port/forward_handover_test.go index 05d0fcf12a..f73c3eb115 100644 --- a/packages/envd/internal/port/forward_handover_test.go +++ b/packages/envd/internal/port/forward_handover_test.go @@ -76,8 +76,11 @@ func TestForwarder_ImportForwards_ReadoptsLiveSocat(t *testing.T) { }) require.Equal(t, 1, n) - p, ok := f.ports["100-8080"] - require.True(t, ok, "a live socat must be re-adopted into the ports map") + // ImportForwards normalizes an old single-dash key ("pid-port") to the new + // "pid-port-ip" format (IP inferred from Family) so a live upgrade does not + // spawn a duplicate socat alongside the scan loop's own entry. + p, ok := f.ports["100-8080-127.0.0.1"] + require.True(t, ok, "a live socat must be re-adopted into the ports map under the normalized key") assert.Equal(t, pid, p.socatPID()) assert.Nil(t, p.socat, "a re-adopted socat has no *exec.Cmd") } diff --git a/packages/envd/internal/port/ipv6_wildcard_test.go b/packages/envd/internal/port/ipv6_wildcard_test.go new file mode 100644 index 0000000000..e24d35e72d --- /dev/null +++ b/packages/envd/internal/port/ipv6_wildcard_test.go @@ -0,0 +1,64 @@ +package port + +import ( + "testing" + + gopsnet "github.com/shirou/gopsutil/v4/net" + + "github.com/e2b-dev/infra/packages/envd/internal/services/spec/upgrade" +) + +// Bug A: the "::" IPv6 wildcard listener must match the port-forwarder filter. +func TestScannerFilter_MatchesIPv6Wildcard(t *testing.T) { + sf := &ScannerFilter{ + IPs: []string{"127.0.0.1", "localhost", "::1", "::"}, + State: "LISTEN", + } + cases := []struct { + ip, state string + want bool + }{ + {"::", "LISTEN", true}, // IPv6 wildcard — the bug this fixes + {"127.0.0.1", "LISTEN", true}, + {"::1", "LISTEN", true}, + {"::", "ESTABLISHED", false}, // wrong state + {"10.0.0.1", "LISTEN", false}, // not a loopback/wildcard + } + for _, c := range cases { + proc := &gopsnet.ConnectionStat{Status: c.state, Laddr: gopsnet.Addr{IP: c.ip, Port: 8080}} + if got := sf.Match(proc); got != c.want { + t.Errorf("Match(ip=%q,state=%q) = %v, want %v", c.ip, c.state, got, c.want) + } + } +} + +// Bug C: socat backend must use literal loopback addresses, not "localhost". +func TestBackendAddr(t *testing.T) { + if got := backendAddr(4); got != "127.0.0.1" { + t.Errorf("backendAddr(4) = %q, want 127.0.0.1", got) + } + if got := backendAddr(6); got != "[::1]" { + t.Errorf("backendAddr(6) = %q, want [::1]", got) + } +} + +// Handover: old single-dash keys are migrated to the new pid-port-ip format so a +// live upgrade does not spawn a duplicate socat for the same listener. +func TestNormalizeForwardKey(t *testing.T) { + cases := []struct { + key string + family uint32 + want string + }{ + {"42-8080", 4, "42-8080-127.0.0.1"}, // old format, IPv4 inferred + {"42-8080", 6, "42-8080-::1"}, // old format, IPv6 inferred + {"42-8080-127.0.0.1", 4, "42-8080-127.0.0.1"}, // new format, unchanged + {"42-8080-::1", 6, "42-8080-::1"}, // new format, unchanged + } + for _, c := range cases { + fp := &upgrade.ForwardedPort{Key: c.key, Family: c.family} + if got := normalizeForwardKey(fp); got != c.want { + t.Errorf("normalizeForwardKey(%q,fam=%d) = %q, want %q", c.key, c.family, got, c.want) + } + } +}