From e92bf2d7d88d4167b54e5145b594d61e0b2339ff Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Tue, 30 Sep 2025 15:11:59 +0000 Subject: [PATCH 1/9] feat: lowering perms to CAP_NET_ADMIN --- .github/workflows/ci.yml | 30 +++ Makefile | 2 +- boundary.go | 8 +- cli/cli.go | 56 ++++- e2e_tests/boundary_integration_test.go | 47 ++-- jail/jail.go | 3 +- jail/linux.go | 333 +++++++++++++++---------- jail/macos.go | 10 +- jail/unprivileged.go | 6 +- 9 files changed, 316 insertions(+), 179 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 22fb82a..1931848 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,6 +73,36 @@ jobs: - name: Download and verify dependencies run: make deps + # Before (default): + # - /etc/resolv.conf -> /run/systemd/resolve/stub-resolv.conf + # - stub-resolv.conf points to 127.0.0.53 (systemd-resolved stub listener) + # - systemd-resolved forwards to the real upstream file: + # /run/systemd/resolve/resolv.conf + # Flow: /etc/resolv.conf -> stub-resolv.conf (127.0.0.53) -> systemd-resolved -> /run/systemd/resolve/resolv.conf + # + # After (bind-mount): + # - /etc/resolv.conf is bind-mounted to /run/systemd/resolve/resolv.conf + # - processes read upstream nameservers directly from /run/systemd/resolve/resolv.conf + # Flow: /etc/resolv.conf -> /run/systemd/resolve/resolv.conf + # + # This makes processes talk directly to the upstream DNS servers and + # bypasses the systemd-resolved *stub listener* (127.0.0.53). + # + # Important nuance: systemd-resolved itself is NOT stopped; it still runs and updates + # /run/systemd/resolve/resolv.conf. Because this is a bind (not a copy), updates to the + # upstream list are visible. Trade-off: we lose the stub’s features (caching, + # split-DNS/VPN per-interface behavior, DNSSEC/DoT/DoH mediation, mDNS/LLMNR). + # + # Reason: network namespaces have their own network stack (interfaces, routes and loopback). + # A process inside a network namespace resolves 127.0.0.53 against that namespace’s loopback, not the host’s, + # and systemd-resolved usually listens on the host loopback. As a result the stub at 127.0.0.53 is often + # unreachable from an isolated namespace and DNS lookups fail. + # Bind-mounting /run/systemd/resolve/resolv.conf over /etc/resolv.conf forces processes to use the upstream + # nameservers directly, avoiding that failure. + - name: Change DNS configuration + if: runner.os == 'Linux' + run: sudo mount --bind /run/systemd/resolve/resolv.conf /etc/resolv.conf + - name: Run unit tests run: make unit-test diff --git a/Makefile b/Makefile index efc16de..e2c2beb 100644 --- a/Makefile +++ b/Makefile @@ -62,7 +62,7 @@ e2e-test: echo "E2E tests require Linux platform. Current platform: $$(uname)"; \ exit 1; \ fi - sudo $(shell which go) test -v -race ./e2e_tests + sudo $(shell which go) test -v -race ./e2e_tests -count=1 @echo "✓ E2E tests passed!" # Run tests with coverage (needs sudo for E2E tests) diff --git a/boundary.go b/boundary.go index d3e98a6..4993f32 100644 --- a/boundary.go +++ b/boundary.go @@ -55,8 +55,8 @@ func New(ctx context.Context, config Config) (*Boundary, error) { } func (b *Boundary) Start() error { - // Start the jailer (network isolation) - err := b.jailer.Start() + // Configure the jailer (network isolation) + err := b.jailer.ConfigureBeforeCommandExecution() if err != nil { return fmt.Errorf("failed to start jailer: %v", err) } @@ -78,6 +78,10 @@ func (b *Boundary) Command(command []string) *exec.Cmd { return b.jailer.Command(command) } +func (b *Boundary) ConfigureAfterCommandExecution(processPID int) error { + return b.jailer.ConfigureAfterCommandExecution(processPID) +} + func (b *Boundary) Close() error { // Stop proxy server if b.proxyServer != nil { diff --git a/cli/cli.go b/cli/cli.go index dabaa46..3935595 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -3,8 +3,10 @@ package cli import ( "context" "fmt" + "log" "log/slog" "os" + "os/exec" "os/signal" "path/filepath" "strings" @@ -92,8 +94,39 @@ func BaseCommand() *serpent.Command { } } +func isChild() bool { + return os.Getenv("CHILD") == "true" +} + // Run executes the boundary command with the given configuration and arguments func Run(ctx context.Context, config Config, args []string) error { + if isChild() { + log.Printf("boundary CHILD process is started") + + vethNetJail := os.Getenv("VETH_JAIL_NAME") + err := jail.SetupChildNetworking(vethNetJail) + if err != nil { + return fmt.Errorf("failed to setup child networking: %v", err) + } + log.Printf("child networking is successfully configured") + + // Program to run + bin := args[0] + args = args[1:] + + cmd := exec.Command(bin, args...) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + err = cmd.Run() + if err != nil { + log.Printf("failed to run %s: %v", bin, err) + return err + } + + return nil + } + ctx, cancel := context.WithCancel(ctx) defer cancel() @@ -191,15 +224,26 @@ func Run(ctx context.Context, config Config, args []string) error { // Execute command in boundary go func() { defer cancel() - cmd := boundaryInstance.Command(args) - cmd.Stderr = os.Stderr - cmd.Stdout = os.Stdout - cmd.Stdin = os.Stdin + cmd := boundaryInstance.Command(os.Args) + + logger.Debug("Executing command in boundary", "command", strings.Join(os.Args, " ")) + err := cmd.Start() + if err != nil { + logger.Error("Command failed to start", "error", err) + return + } + + err = boundaryInstance.ConfigureAfterCommandExecution(cmd.Process.Pid) + if err != nil { + logger.Error("configuration after command execution failed", "error", err) + return + } - logger.Debug("Executing command in boundary", "command", strings.Join(args, " ")) - err := cmd.Run() + logger.Debug("waiting on a child process to finish") + err = cmd.Wait() if err != nil { logger.Error("Command execution failed", "error", err) + return } }() diff --git a/e2e_tests/boundary_integration_test.go b/e2e_tests/boundary_integration_test.go index 84ff284..bf642cd 100644 --- a/e2e_tests/boundary_integration_test.go +++ b/e2e_tests/boundary_integration_test.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "testing" "time" @@ -37,29 +38,15 @@ func findProjectRoot(t *testing.T) string { } } -// getNamespaceName gets the single network namespace name -// Fails if there are 0 or multiple namespaces -func getNamespaceName(t *testing.T) string { - cmd := exec.Command("ip", "netns", "list") +func getChildProcessPID(t *testing.T) int { + cmd := exec.Command("pgrep", "-f", "boundary-test", "-n") output, err := cmd.Output() - require.NoError(t, err, "Failed to list network namespaces") - - lines := strings.Split(string(output), "\n") - var namespaces []string - - for _, line := range lines { - line = strings.TrimSpace(line) - if line != "" { - // Extract namespace name (first field) - parts := strings.Fields(line) - if len(parts) > 0 { - namespaces = append(namespaces, parts[0]) - } - } - } + require.NoError(t, err) - require.Len(t, namespaces, 1, "Expected exactly one network namespace, found %d: %v", len(namespaces), namespaces) - return namespaces[0] + pidStr := strings.TrimSpace(string(output)) + pid, err := strconv.Atoi(pidStr) + require.NoError(t, err) + return pid } func TestBoundaryIntegration(t *testing.T) { @@ -73,7 +60,7 @@ func TestBoundaryIntegration(t *testing.T) { require.NoError(t, err, "Failed to build boundary binary") // Create context for boundary process - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() // Start boundary process with sudo @@ -81,10 +68,10 @@ func TestBoundaryIntegration(t *testing.T) { "--allow", "dev.coder.com", "--allow", "jsonplaceholder.typicode.com", "--log-level", "debug", - "--", "bash", "-c", "sleep 10 && echo 'Test completed'") + "--", "/bin/bash", "-c", "/usr/bin/sleep 10 && /usr/bin/echo 'Test completed'") - // Suppress output to prevent terminal corruption - boundaryCmd.Stdout = os.Stdout // Let it go to /dev/null + boundaryCmd.Stdin = os.Stdin + boundaryCmd.Stdout = os.Stdout boundaryCmd.Stderr = os.Stderr // Start the process @@ -94,13 +81,13 @@ func TestBoundaryIntegration(t *testing.T) { // Give boundary time to start time.Sleep(2 * time.Second) - // Get the namespace name that boundary created - namespaceName := getNamespaceName(t) + pidInt := getChildProcessPID(t) + pid := fmt.Sprintf("%v", pidInt) // Test HTTP request through boundary (from inside the jail) t.Run("HTTPRequestThroughBoundary", func(t *testing.T) { // Run curl directly in the namespace using ip netns exec - curlCmd := exec.Command("sudo", "ip", "netns", "exec", namespaceName, + curlCmd := exec.Command("sudo", "nsenter", "-t", pid, "-n", "--", "curl", "http://jsonplaceholder.typicode.com/todos/1") // Capture stderr separately @@ -128,7 +115,7 @@ func TestBoundaryIntegration(t *testing.T) { certPath := fmt.Sprintf("%v/ca-cert.pem", configDir) // Run curl directly in the namespace using ip netns exec - curlCmd := exec.Command("sudo", "ip", "netns", "exec", namespaceName, + curlCmd := exec.Command("sudo", "sudo", "nsenter", "-t", pid, "-n", "--", "env", fmt.Sprintf("SSL_CERT_FILE=%v", certPath), "curl", "-s", "https://dev.coder.com/api/v2") // Capture stderr separately @@ -149,7 +136,7 @@ func TestBoundaryIntegration(t *testing.T) { // Test blocked domain (from inside the jail) t.Run("BlockedDomainTest", func(t *testing.T) { // Run curl directly in the namespace using ip netns exec - curlCmd := exec.Command("sudo", "ip", "netns", "exec", namespaceName, + curlCmd := exec.Command("sudo", "sudo", "nsenter", "-t", pid, "-n", "--", "curl", "-s", "http://example.com") // Capture stderr separately diff --git a/jail/jail.go b/jail/jail.go index 2c2f217..b3f9eb6 100644 --- a/jail/jail.go +++ b/jail/jail.go @@ -8,8 +8,9 @@ import ( ) type Jailer interface { - Start() error + ConfigureBeforeCommandExecution() error Command(command []string) *exec.Cmd + ConfigureAfterCommandExecution(processPID int) error Close() error } diff --git a/jail/linux.go b/jail/linux.go index a8c3a5f..7d7e349 100644 --- a/jail/linux.go +++ b/jail/linux.go @@ -7,14 +7,18 @@ import ( "log/slog" "os" "os/exec" + "syscall" "time" + + "golang.org/x/sys/unix" ) // LinuxJail implements Jailer using Linux network namespaces type LinuxJail struct { logger *slog.Logger namespace string - vethHost string // Host-side veth interface name for iptables rules + vethHostName string // Host-side veth interface name for iptables rules + vethJailName string // Jail-side veth interface name for iptables rules commandEnv []string httpProxyPort int configDir string @@ -39,58 +43,62 @@ func NewLinuxJail(config Config) (*LinuxJail, error) { }, nil } -// Start creates network namespace and configures iptables rules -func (l *LinuxJail) Start() error { - l.logger.Debug("Setup called") - - e := getEnvs(l.configDir, l.caCertPath) - l.commandEnv = mergeEnvs(e, map[string]string{}) +// ConfigureBeforeCommandExecution prepares the jail environment before the target +// process is launched. It sets environment variables, creates the veth pair, and +// installs iptables rules on the host. At this stage, the target PID and its netns +// are not yet known. +func (l *LinuxJail) ConfigureBeforeCommandExecution() error { + l.commandEnv = getEnvs(l.configDir, l.caCertPath) - // Setup DNS configuration BEFORE creating namespace - // This ensures the namespace-specific resolv.conf is available when namespace is created - err := l.setupDNS() - if err != nil { - return fmt.Errorf("failed to setup DNS: %v", err) - } - - // Create namespace - err = l.createNamespace() - if err != nil { - return fmt.Errorf("failed to create namespace: %v", err) - } - - // Setup networking within namespace - err = l.setupNetworking() - if err != nil { - return fmt.Errorf("failed to setup networking: %v", err) + if err := l.configureHostNetworkBeforeCmdExec(); err != nil { + return err } - - // Setup iptables rules on host - err = l.setupIptables() - if err != nil { - return fmt.Errorf("failed to setup iptables: %v", err) + if err := l.configureIptables(); err != nil { + return fmt.Errorf("failed to configure iptables: %v", err) } return nil } -// Command returns an exec.Cmd configured to run within the network namespace +// Command returns an exec.Cmd configured to run within the network namespace. func (l *LinuxJail) Command(command []string) *exec.Cmd { l.logger.Debug("Creating command with namespace", "namespace", l.namespace) - cmdArgs := []string{"netns", "exec", l.namespace} - cmdArgs = append(cmdArgs, command...) - - cmd := exec.Command("ip", cmdArgs...) + cmd := exec.Command(command[0], command[1:]...) cmd.Env = l.commandEnv + cmd.Env = append(cmd.Env, "CHILD=true") + cmd.Env = append(cmd.Env, fmt.Sprintf("VETH_JAIL_NAME=%v", l.vethJailName)) + cmd.Stderr = os.Stderr + cmd.Stdout = os.Stdout + cmd.Stdin = os.Stdin + + cmd.SysProcAttr = &syscall.SysProcAttr{ + Cloneflags: syscall.CLONE_NEWUSER | syscall.CLONE_NEWNET, + UidMappings: []syscall.SysProcIDMap{ + {ContainerID: 0, HostID: os.Getuid(), Size: 1}, + }, + GidMappings: []syscall.SysProcIDMap{ + {ContainerID: 0, HostID: os.Getgid(), Size: 1}, + }, + } return cmd } +// ConfigureAfterCommandExecution finalizes setup once the target process starts. +// With the child PID known, it moves the jail-side veth into the child’s network +// namespace. +func (l *LinuxJail) ConfigureAfterCommandExecution(pidInt int) error { + err := l.configureHostNetworkAfterCmdExec(pidInt) + if err != nil { + return fmt.Errorf("failed to configure parent networking: %v", err) + } + + return nil +} + // Close removes the network namespace and iptables rules func (l *LinuxJail) Close() error { - l.logger.Debug("Close called") - // Clean up iptables rules err := l.cleanupIptables() if err != nil { @@ -122,136 +130,176 @@ func (l *LinuxJail) Close() error { return nil } -// createNamespace creates a new network namespace -func (l *LinuxJail) createNamespace() error { - cmd := exec.Command("ip", "netns", "add", l.namespace) +// removeNamespace removes the network namespace +func (l *LinuxJail) removeNamespace() error { + cmd := exec.Command("ip", "netns", "del", l.namespace) err := cmd.Run() if err != nil { - return fmt.Errorf("failed to create namespace: %v", err) + return fmt.Errorf("failed to remove namespace: %v", err) } return nil } -// setupNetworking configures networking within the namespace -func (l *LinuxJail) setupNetworking() error { - // Create veth pair with short names (Linux interface names limited to 15 chars) - // Generate unique ID to avoid conflicts - uniqueID := fmt.Sprintf("%d", time.Now().UnixNano()%10000000) // 7 digits max - vethHost := fmt.Sprintf("veth_h_%s", uniqueID) // veth_h_1234567 = 14 chars - vethNetJail := fmt.Sprintf("veth_n_%s", uniqueID) // veth_n_1234567 = 14 chars +type command struct { + description string + cmd *exec.Cmd + ambientCaps []uintptr +} - // Store veth interface name for iptables rules - l.vethHost = vethHost +type commandRunner struct { + commands []*command +} - setupCmds := []struct { - description string - command *exec.Cmd - }{ - {"create veth pair", exec.Command("ip", "link", "add", vethHost, "type", "veth", "peer", "name", vethNetJail)}, - {"move veth to namespace", exec.Command("ip", "link", "set", vethNetJail, "netns", l.namespace)}, - {"configure host veth", exec.Command("ip", "addr", "add", "192.168.100.1/24", "dev", vethHost)}, - {"bring up host veth", exec.Command("ip", "link", "set", vethHost, "up")}, - {"configure namespace veth", exec.Command("ip", "netns", "exec", l.namespace, "ip", "addr", "add", "192.168.100.2/24", "dev", vethNetJail)}, - {"bring up namespace veth", exec.Command("ip", "netns", "exec", l.namespace, "ip", "link", "set", vethNetJail, "up")}, - {"bring up loopback", exec.Command("ip", "netns", "exec", l.namespace, "ip", "link", "set", "lo", "up")}, - {"set default route in namespace", exec.Command("ip", "netns", "exec", l.namespace, "ip", "route", "add", "default", "via", "192.168.100.1")}, +func newCommandRunner(commands []*command) *commandRunner { + return &commandRunner{ + commands: commands, } +} - for _, command := range setupCmds { - if err := command.command.Run(); err != nil { - return fmt.Errorf("failed to %s: %v", command.description, err) +func (r *commandRunner) run() error { + for _, command := range r.commands { + command.cmd.SysProcAttr = &syscall.SysProcAttr{ + AmbientCaps: command.ambientCaps, + } + + output, err := command.cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("failed to %s: %v, output: %s", command.description, err, output) } } return nil } -// setupDNS configures DNS resolution for the namespace -// This ensures reliable DNS resolution by using public DNS servers -// instead of relying on the host's potentially complex DNS configuration -func (l *LinuxJail) setupDNS() error { - // Always create namespace-specific resolv.conf with reliable public DNS servers - // This avoids issues with systemd-resolved, Docker DNS, and other complex setups - netnsEtc := fmt.Sprintf("/etc/netns/%s", l.namespace) - err := os.MkdirAll(netnsEtc, 0755) - if err != nil { - return fmt.Errorf("failed to create /etc/netns directory: %v", err) - } +// configureHostNetworkBeforeCmdExec prepares host-side networking before the target +// process is started. At this point the target process is not running, so its PID and network +// namespace ID are not yet known. +func (l *LinuxJail) configureHostNetworkBeforeCmdExec() error { + // Create veth pair with short names (Linux interface names limited to 15 chars) + // Generate unique ID to avoid conflicts + uniqueID := fmt.Sprintf("%d", time.Now().UnixNano()%10000000) // 7 digits max + vethHostName := fmt.Sprintf("veth_h_%s", uniqueID) // veth_h_1234567 = 14 chars + vethJailName := fmt.Sprintf("veth_n_%s", uniqueID) // veth_n_1234567 = 14 chars - // Write custom resolv.conf with multiple reliable public DNS servers - resolvConfPath := fmt.Sprintf("%s/resolv.conf", netnsEtc) - dnsConfig := `# Custom DNS for network namespace -nameserver 8.8.8.8 -nameserver 8.8.4.4 -nameserver 1.1.1.1 -nameserver 9.9.9.9 -options timeout:2 attempts:2 -` - err = os.WriteFile(resolvConfPath, []byte(dnsConfig), 0644) - if err != nil { - return fmt.Errorf("failed to write namespace-specific resolv.conf: %v", err) + // Store veth interface name for iptables rules + l.vethHostName = vethHostName + l.vethJailName = vethJailName + + runner := newCommandRunner([]*command{ + { + "create veth pair", + exec.Command("ip", "link", "add", vethHostName, "type", "veth", "peer", "name", vethJailName), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "configure host veth", + exec.Command("ip", "addr", "add", "192.168.100.1/24", "dev", vethHostName), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "bring up host veth", + exec.Command("ip", "link", "set", vethHostName, "up"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + }) + if err := runner.run(); err != nil { + return err } - l.logger.Debug("DNS setup completed") return nil } -// setupIptables configures iptables rules for comprehensive TCP traffic interception -func (l *LinuxJail) setupIptables() error { - // Enable IP forwarding - cmd := exec.Command("sysctl", "-w", "net.ipv4.ip_forward=1") - _ = cmd.Run() // Ignore error - - // NAT rules for outgoing traffic (MASQUERADE for return traffic) - cmd = exec.Command("iptables", "-t", "nat", "-A", "POSTROUTING", "-s", "192.168.100.0/24", "-j", "MASQUERADE") - err := cmd.Run() - if err != nil { - return fmt.Errorf("failed to add NAT rule: %v", err) - } - - // COMPREHENSIVE APPROACH: Route ALL TCP traffic to HTTP proxy - // The HTTP proxy will intelligently handle both HTTP and TLS traffic - cmd = exec.Command("iptables", "-t", "nat", "-A", "PREROUTING", "-i", l.vethHost, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)) - err = cmd.Run() - if err != nil { - return fmt.Errorf("failed to add comprehensive TCP redirect rule: %v", err) - } - - // TODO: clean up this rules - cmd = exec.Command("iptables", "-A", "FORWARD", "-s", "192.168.100.0/24", "-j", "ACCEPT") - err = cmd.Run() - if err != nil { +// configureHostNetworkAfterCmdExec finalizes host-side networking after the target +// process has started. It moves the jail-side veth into the target process's network +// namespace using the provided PID. This requires the process to be running so +// its PID (and thus its netns) are available. +func (l *LinuxJail) configureHostNetworkAfterCmdExec(pidInt int) error { + PID := fmt.Sprintf("%v", pidInt) + + runner := newCommandRunner([]*command{ + { + "move veth to namespace", + exec.Command("ip", "link", "set", l.vethJailName, "netns", PID), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + }) + if err := runner.run(); err != nil { return err } - cmd = exec.Command("iptables", "-A", "FORWARD", "-d", "192.168.100.0/24", "-j", "ACCEPT") - err = cmd.Run() - if err != nil { + return nil +} + +// SetupChildNetworking configures networking within the target process's network +// namespace. This runs inside the child process after it has been +// created and moved to its own network namespace. +func SetupChildNetworking(vethNetJail string) error { + runner := newCommandRunner([]*command{ + { + "configure namespace veth", + exec.Command("ip", "addr", "add", "192.168.100.2/24", "dev", vethNetJail), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "bring up namespace veth", + exec.Command("ip", "link", "set", vethNetJail, "up"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "bring up loopback", + exec.Command("ip", "link", "set", "lo", "up"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "set default route in namespace", + exec.Command("ip", "route", "add", "default", "via", "192.168.100.1"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + }) + if err := runner.run(); err != nil { return err } - l.logger.Debug("Comprehensive TCP boundarying enabled", "interface", l.vethHost, "proxy_port", l.httpProxyPort) return nil } -// cleanupIptables removes iptables rules -func (l *LinuxJail) cleanupIptables() error { - // Remove comprehensive TCP redirect rule - cmd := exec.Command("iptables", "-t", "nat", "-D", "PREROUTING", "-i", l.vethHost, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)) - err := cmd.Run() - if err != nil { - l.logger.Error("Failed to remove TCP redirect rule", "error", err) - // Continue with other cleanup even if this fails - } - - // Remove NAT rule - cmd = exec.Command("iptables", "-t", "nat", "-D", "POSTROUTING", "-s", "192.168.100.0/24", "-j", "MASQUERADE") - err = cmd.Run() - if err != nil { - l.logger.Error("Failed to remove NAT rule", "error", err) - // Continue with other cleanup even if this fails +// setupIptables configures iptables rules for comprehensive TCP traffic interception +func (l *LinuxJail) configureIptables() error { + runner := newCommandRunner([]*command{ + { + "enable IP forwarding", + exec.Command("sysctl", "-w", "net.ipv4.ip_forward=1"), + []uintptr{}, + }, + { + "NAT rules for outgoing traffic (MASQUERADE for return traffic)", + exec.Command("iptables", "-t", "nat", "-A", "POSTROUTING", "-s", "192.168.100.0/24", "-j", "MASQUERADE"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + // COMPREHENSIVE APPROACH: Route ALL TCP traffic to HTTP proxy + // The HTTP proxy will intelligently handle both HTTP and TLS traffic + "Route ALL TCP traffic to HTTP proxy", + exec.Command("iptables", "-t", "nat", "-A", "PREROUTING", "-i", l.vethHostName, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + // TODO: clean up this rules + { + "iptables FORWARD -s", + exec.Command("iptables", "-A", "FORWARD", "-s", "192.168.100.0/24", "-j", "ACCEPT"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "iptables FORWARD -d", + exec.Command("iptables", "-A", "FORWARD", "-d", "192.168.100.0/24", "-j", "ACCEPT"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + }) + if err := runner.run(); err != nil { + return err } + l.logger.Debug("Comprehensive TCP boundarying enabled", "interface", l.vethHostName, "proxy_port", l.httpProxyPort) return nil } @@ -278,12 +326,23 @@ func (l *LinuxJail) cleanupNetworking() error { return nil } -// removeNamespace removes the network namespace -func (l *LinuxJail) removeNamespace() error { - cmd := exec.Command("ip", "netns", "del", l.namespace) +// cleanupIptables removes iptables rules +func (l *LinuxJail) cleanupIptables() error { + // Remove comprehensive TCP redirect rule + cmd := exec.Command("iptables", "-t", "nat", "-D", "PREROUTING", "-i", l.vethHostName, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)) err := cmd.Run() if err != nil { - return fmt.Errorf("failed to remove namespace: %v", err) + l.logger.Error("Failed to remove TCP redirect rule", "error", err) + // Continue with other cleanup even if this fails + } + + // Remove NAT rule + cmd = exec.Command("iptables", "-t", "nat", "-D", "POSTROUTING", "-s", "192.168.100.0/24", "-j", "MASQUERADE") + err = cmd.Run() + if err != nil { + l.logger.Error("Failed to remove NAT rule", "error", err) + // Continue with other cleanup even if this fails } + return nil } diff --git a/jail/macos.go b/jail/macos.go index f10c273..0acf58d 100644 --- a/jail/macos.go +++ b/jail/macos.go @@ -54,8 +54,12 @@ func NewMacOSJail(config Config) (*MacOSJail, error) { }, nil } +func SetupChildNetworking(vethNetJail string) error { + return nil +} + // Setup creates the network boundary group and configures PF rules -func (n *MacOSJail) Start() error { +func (n *MacOSJail) ConfigureBeforeCommandExecution() error { n.logger.Debug("Setup called") // Create or get network boundary group @@ -340,3 +344,7 @@ func (n *MacOSJail) cleanupTempFiles() { } } } + +func (u *MacOSJail) ConfigureAfterCommandExecution(processPID int) error { + return nil +} diff --git a/jail/unprivileged.go b/jail/unprivileged.go index 516ac9c..8504f3d 100644 --- a/jail/unprivileged.go +++ b/jail/unprivileged.go @@ -31,7 +31,7 @@ func NewUnprivileged(config Config) (*Unprivileged, error) { }, nil } -func (u *Unprivileged) Start() error { +func (u *Unprivileged) ConfigureBeforeCommandExecution() error { u.logger.Debug("Starting in unprivileged mode") e := getEnvs(u.configDir, u.caCertPath) p := fmt.Sprintf("http://localhost:%d", u.httpProxyPort) @@ -60,3 +60,7 @@ func (u *Unprivileged) Close() error { u.logger.Debug("Closing unprivileged jail") return nil } + +func (u *Unprivileged) ConfigureAfterCommandExecution(processPID int) error { + return nil +} From da8569734f5d7370b7c6377d00f4d1805f6976ba Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Wed, 8 Oct 2025 20:16:49 +0000 Subject: [PATCH 2/9] refactor: improve cleanup --- e2e_tests/boundary_integration_test.go | 5 ++ e2e_tests/iptables_cleanup_test.go | 76 ++++++++++++++++++++++++++ jail/linux.go | 63 +++++++++++++++------ 3 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 e2e_tests/iptables_cleanup_test.go diff --git a/e2e_tests/boundary_integration_test.go b/e2e_tests/boundary_integration_test.go index bf642cd..6122389 100644 --- a/e2e_tests/boundary_integration_test.go +++ b/e2e_tests/boundary_integration_test.go @@ -150,6 +150,11 @@ func TestBoundaryIntegration(t *testing.T) { require.Contains(t, string(output), "Request Blocked by Boundary") }) + // Gracefully close process, call cleanup methods + err = boundaryCmd.Process.Signal(os.Interrupt) + require.NoError(t, err, "Failed to interrupt boundary process") + time.Sleep(time.Second * 1) + // Clean up cancel() // This will terminate the boundary process err = boundaryCmd.Wait() // Wait for process to finish diff --git a/e2e_tests/iptables_cleanup_test.go b/e2e_tests/iptables_cleanup_test.go new file mode 100644 index 0000000..ae9681e --- /dev/null +++ b/e2e_tests/iptables_cleanup_test.go @@ -0,0 +1,76 @@ +package e2e_tests + +import ( + "context" + "os" + "os/exec" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestIPTablesCleanup(t *testing.T) { + // Step 1: Capture initial iptables rules + initialCmd := exec.Command("sudo", "iptables", "-L", "-n") + initialOutput, err := initialCmd.Output() + require.NoError(t, err, "Failed to get initial iptables rules") + initialRules := string(initialOutput) + //fmt.Printf("Initial iptables rules:\n%s", initialRules) + + // Step 2: Run Boundary + // Find project root by looking for go.mod file + projectRoot := findProjectRoot(t) + + // Build the boundary binary + buildCmd := exec.Command("go", "build", "-o", "/tmp/boundary-test", "./cmd/...") + buildCmd.Dir = projectRoot + err = buildCmd.Run() + require.NoError(t, err, "Failed to build boundary binary") + + // Create context for boundary process + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + // Start boundary process with sudo + boundaryCmd := exec.CommandContext(ctx, "/tmp/boundary-test", + "--allow", "dev.coder.com", + "--allow", "jsonplaceholder.typicode.com", + "--log-level", "debug", + "--", "/bin/bash", "-c", "/usr/bin/sleep 10 && /usr/bin/echo 'Test completed'") + + boundaryCmd.Stdin = os.Stdin + boundaryCmd.Stdout = os.Stdout + boundaryCmd.Stderr = os.Stderr + + // Start the process + err = boundaryCmd.Start() + require.NoError(t, err, "Failed to start boundary process") + + // Give boundary time to start + time.Sleep(2 * time.Second) + + // Gracefully close process, call cleanup methods + err = boundaryCmd.Process.Signal(os.Interrupt) + require.NoError(t, err, "Failed to interrupt boundary process") + time.Sleep(time.Second * 1) + + // Step 3: Clean up + cancel() // This will terminate the boundary process + err = boundaryCmd.Wait() // Wait for process to finish + if err != nil { + t.Logf("Boundary process finished with error: %v", err) + } + + // Clean up binary + err = os.Remove("/tmp/boundary-test") + require.NoError(t, err, "Failed to remove /tmp/boundary-test") + + // Step 4: Capture iptables rules after boundary has executed + iptablesCmd := exec.Command("sudo", "iptables", "-L", "-n") + iptablesOutput, err := iptablesCmd.Output() + require.NoError(t, err, "Failed to get iptables rules") + iptablesRules := string(iptablesOutput) + + require.Equal(t, initialRules, iptablesRules) +} diff --git a/jail/linux.go b/jail/linux.go index 7d7e349..e8b38e8 100644 --- a/jail/linux.go +++ b/jail/linux.go @@ -4,6 +4,7 @@ package jail import ( "fmt" + "log" "log/slog" "os" "os/exec" @@ -171,6 +172,22 @@ func (r *commandRunner) run() error { return nil } +func (r *commandRunner) runIgnoreErrors() error { + for _, command := range r.commands { + command.cmd.SysProcAttr = &syscall.SysProcAttr{ + AmbientCaps: command.ambientCaps, + } + + output, err := command.cmd.CombinedOutput() + if err != nil { + log.Printf("failed to %s: %v, output: %s", command.description, err, output) + continue + } + } + + return nil +} + // configureHostNetworkBeforeCmdExec prepares host-side networking before the target // process is started. At this point the target process is not running, so its PID and network // namespace ID are not yet known. @@ -283,7 +300,6 @@ func (l *LinuxJail) configureIptables() error { exec.Command("iptables", "-t", "nat", "-A", "PREROUTING", "-i", l.vethHostName, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)), []uintptr{uintptr(unix.CAP_NET_ADMIN)}, }, - // TODO: clean up this rules { "iptables FORWARD -s", exec.Command("iptables", "-A", "FORWARD", "-s", "192.168.100.0/24", "-j", "ACCEPT"), @@ -314,12 +330,15 @@ func (l *LinuxJail) cleanupNetworking() error { description string command *exec.Cmd }{ - {"delete veth pair", exec.Command("ip", "link", "del", vethHost)}, + { + "delete veth pair", + exec.Command("ip", "link", "del", vethHost), + }, } for _, command := range cleanupCmds { if err := command.command.Run(); err != nil { - return fmt.Errorf("failed to %s: %v", command.description, err) + l.logger.Error("failed to execute command", "command", command.description, "error", err) } } @@ -328,20 +347,30 @@ func (l *LinuxJail) cleanupNetworking() error { // cleanupIptables removes iptables rules func (l *LinuxJail) cleanupIptables() error { - // Remove comprehensive TCP redirect rule - cmd := exec.Command("iptables", "-t", "nat", "-D", "PREROUTING", "-i", l.vethHostName, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)) - err := cmd.Run() - if err != nil { - l.logger.Error("Failed to remove TCP redirect rule", "error", err) - // Continue with other cleanup even if this fails - } - - // Remove NAT rule - cmd = exec.Command("iptables", "-t", "nat", "-D", "POSTROUTING", "-s", "192.168.100.0/24", "-j", "MASQUERADE") - err = cmd.Run() - if err != nil { - l.logger.Error("Failed to remove NAT rule", "error", err) - // Continue with other cleanup even if this fails + runner := newCommandRunner([]*command{ + { + "Remove comprehensive TCP redirect rule", + exec.Command("iptables", "-t", "nat", "-D", "PREROUTING", "-i", l.vethHostName, "-p", "tcp", "-j", "REDIRECT", "--to-ports", fmt.Sprintf("%d", l.httpProxyPort)), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "Remove NAT rule", + exec.Command("iptables", "-t", "nat", "-D", "POSTROUTING", "-s", "192.168.100.0/24", "-j", "MASQUERADE"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "Remove iptables FORWARD -s", + exec.Command("iptables", "-D", "FORWARD", "-s", "192.168.100.0/24", "-j", "ACCEPT"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + { + "Remove iptables FORWARD -d", + exec.Command("iptables", "-D", "FORWARD", "-d", "192.168.100.0/24", "-j", "ACCEPT"), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + }, + }) + if err := runner.runIgnoreErrors(); err != nil { + return err } return nil From 29d30b86977e75dcc1cff3130c43d9636d2e3f75 Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Wed, 8 Oct 2025 20:33:53 +0000 Subject: [PATCH 3/9] test: improve testing --- e2e_tests/iptables_cleanup_test.go | 37 ++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/e2e_tests/iptables_cleanup_test.go b/e2e_tests/iptables_cleanup_test.go index ae9681e..0fe0bbc 100644 --- a/e2e_tests/iptables_cleanup_test.go +++ b/e2e_tests/iptables_cleanup_test.go @@ -2,6 +2,7 @@ package e2e_tests import ( "context" + "fmt" "os" "os/exec" "testing" @@ -10,13 +11,28 @@ import ( "github.com/stretchr/testify/require" ) +const ( + filterTable = "filter" + natTable = "nat" +) + +func getIptablesRules(tableName string) (string, error) { + cmd := exec.Command("sudo", "iptables", "-L", "-n", "-t", tableName) + output, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("failed to get iptables rules: %v", err) + } + rules := string(output) + + return rules, nil +} + func TestIPTablesCleanup(t *testing.T) { // Step 1: Capture initial iptables rules - initialCmd := exec.Command("sudo", "iptables", "-L", "-n") - initialOutput, err := initialCmd.Output() - require.NoError(t, err, "Failed to get initial iptables rules") - initialRules := string(initialOutput) - //fmt.Printf("Initial iptables rules:\n%s", initialRules) + initialFilterRules, err := getIptablesRules(filterTable) + require.NoError(t, err) + initialNatRules, err := getIptablesRules(natTable) + require.NoError(t, err) // Step 2: Run Boundary // Find project root by looking for go.mod file @@ -67,10 +83,11 @@ func TestIPTablesCleanup(t *testing.T) { require.NoError(t, err, "Failed to remove /tmp/boundary-test") // Step 4: Capture iptables rules after boundary has executed - iptablesCmd := exec.Command("sudo", "iptables", "-L", "-n") - iptablesOutput, err := iptablesCmd.Output() - require.NoError(t, err, "Failed to get iptables rules") - iptablesRules := string(iptablesOutput) + filterRules, err := getIptablesRules(filterTable) + require.NoError(t, err) + natRules, err := getIptablesRules(natTable) + require.NoError(t, err) - require.Equal(t, initialRules, iptablesRules) + require.Equal(t, initialFilterRules, filterRules) + require.Equal(t, initialNatRules, natRules) } From 44b259003e6cae951d7cfc0a6f7fe25f1bba69b7 Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Wed, 8 Oct 2025 20:39:04 +0000 Subject: [PATCH 4/9] refactor: remove dead code --- jail/linux.go | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/jail/linux.go b/jail/linux.go index e8b38e8..6e35a39 100644 --- a/jail/linux.go +++ b/jail/linux.go @@ -17,7 +17,6 @@ import ( // LinuxJail implements Jailer using Linux network namespaces type LinuxJail struct { logger *slog.Logger - namespace string vethHostName string // Host-side veth interface name for iptables rules vethJailName string // Jail-side veth interface name for iptables rules commandEnv []string @@ -33,7 +32,6 @@ type LinuxJail struct { func NewLinuxJail(config Config) (*LinuxJail, error) { return &LinuxJail{ logger: config.Logger, - namespace: newNamespaceName(), httpProxyPort: config.HttpProxyPort, configDir: config.ConfigDir, caCertPath: config.CACertPath, @@ -63,7 +61,7 @@ func (l *LinuxJail) ConfigureBeforeCommandExecution() error { // Command returns an exec.Cmd configured to run within the network namespace. func (l *LinuxJail) Command(command []string) *exec.Cmd { - l.logger.Debug("Creating command with namespace", "namespace", l.namespace) + l.logger.Debug("Creating command with namespace") cmd := exec.Command(command[0], command[1:]...) cmd.Env = l.commandEnv @@ -114,30 +112,6 @@ func (l *LinuxJail) Close() error { // Continue with other cleanup even if this fails } - // Clean up namespace-specific DNS config directory - netnsEtc := fmt.Sprintf("/etc/netns/%s", l.namespace) - err = os.RemoveAll(netnsEtc) - if err != nil { - l.logger.Warn("Failed to remove namespace DNS config", "dir", netnsEtc, "error", err) - // Continue with other cleanup - } - - // Remove network namespace - err = l.removeNamespace() - if err != nil { - return fmt.Errorf("failed to remove namespace: %v", err) - } - - return nil -} - -// removeNamespace removes the network namespace -func (l *LinuxJail) removeNamespace() error { - cmd := exec.Command("ip", "netns", "del", l.namespace) - err := cmd.Run() - if err != nil { - return fmt.Errorf("failed to remove namespace: %v", err) - } return nil } From 30cdf17a10eca3536ecdab07698028531e7d6ef1 Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Wed, 8 Oct 2025 20:46:49 +0000 Subject: [PATCH 5/9] refactor: cleanup code --- jail/linux.go | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/jail/linux.go b/jail/linux.go index 6e35a39..40caf67 100644 --- a/jail/linux.go +++ b/jail/linux.go @@ -295,25 +295,15 @@ func (l *LinuxJail) configureIptables() error { // cleanupNetworking removes networking configuration func (l *LinuxJail) cleanupNetworking() error { - // Generate unique ID to match veth pair - uniqueID := fmt.Sprintf("%d", time.Now().UnixNano()%10000000) // 7 digits max - vethHost := fmt.Sprintf("veth_h_%s", uniqueID) // veth_h_1234567 = 14 chars - - // Clean up networking - cleanupCmds := []struct { - description string - command *exec.Cmd - }{ + runner := newCommandRunner([]*command{ { "delete veth pair", - exec.Command("ip", "link", "del", vethHost), + exec.Command("ip", "link", "del", l.vethHostName), + []uintptr{uintptr(unix.CAP_NET_ADMIN)}, }, - } - - for _, command := range cleanupCmds { - if err := command.command.Run(); err != nil { - l.logger.Error("failed to execute command", "command", command.description, "error", err) - } + }) + if err := runner.runIgnoreErrors(); err != nil { + return err } return nil From 973e3384998eebcb5ebcb348d90d914c66692cc0 Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Wed, 8 Oct 2025 20:50:32 +0000 Subject: [PATCH 6/9] refactor: cleanup code --- jail/linux.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/jail/linux.go b/jail/linux.go index 40caf67..440f0ae 100644 --- a/jail/linux.go +++ b/jail/linux.go @@ -296,11 +296,12 @@ func (l *LinuxJail) configureIptables() error { // cleanupNetworking removes networking configuration func (l *LinuxJail) cleanupNetworking() error { runner := newCommandRunner([]*command{ - { - "delete veth pair", - exec.Command("ip", "link", "del", l.vethHostName), - []uintptr{uintptr(unix.CAP_NET_ADMIN)}, - }, + // NOTE: seems that command is unnecessary, because device is automatically deleted when boundary exits + //{ + // "delete veth pair", + // exec.Command("ip", "link", "del", l.vethHostName), + // []uintptr{uintptr(unix.CAP_NET_ADMIN)}, + //}, }) if err := runner.runIgnoreErrors(); err != nil { return err From 0ea4db66884f04c3677103ff925209d13cf5d06a Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Wed, 8 Oct 2025 20:51:32 +0000 Subject: [PATCH 7/9] fix: linter --- jail/util.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/jail/util.go b/jail/util.go index 8a230d7..1861f0b 100644 --- a/jail/util.go +++ b/jail/util.go @@ -1,20 +1,10 @@ package jail import ( - "fmt" "os" "strings" - "time" ) -const ( - prefix = "coder_boundary" -) - -func newNamespaceName() string { - return fmt.Sprintf("%s_%d", prefix, time.Now().UnixNano()%10000000) -} - func getEnvs(configDir string, caCertPath string) []string { e := os.Environ() From f6a8030cc22bb5bb369967951f273d2cd9569073 Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Wed, 8 Oct 2025 20:57:02 +0000 Subject: [PATCH 8/9] fix: MacOS --- jail/macos.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/jail/macos.go b/jail/macos.go index 0acf58d..de9e069 100644 --- a/jail/macos.go +++ b/jail/macos.go @@ -12,6 +12,14 @@ import ( "syscall" ) +const ( + prefix = "coder_boundary" +) + +func newNamespaceName() string { + return fmt.Sprintf("%s_%d", prefix, time.Now().UnixNano()%10000000) +} + const ( pfAnchorName = "coder_boundary" groupName = "coder_boundary" From 5d270b0b3d03a6a78c8c0dc668ea24d9ed1cbbb0 Mon Sep 17 00:00:00 2001 From: YEVHENII SHCHERBINA Date: Wed, 8 Oct 2025 20:58:59 +0000 Subject: [PATCH 9/9] fix: MacOS --- jail/macos.go | 1 + 1 file changed, 1 insertion(+) diff --git a/jail/macos.go b/jail/macos.go index de9e069..811d702 100644 --- a/jail/macos.go +++ b/jail/macos.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" "syscall" + "time" ) const (