From b7745456d67f48f56ba94e47946e40805b6ef1ee Mon Sep 17 00:00:00 2001 From: James O'Doherty Date: Fri, 29 May 2026 20:42:23 -0400 Subject: refactor: improve resource management and cleanup patterns - Simplify namespace bootstrapping by introducing `prepareLauncher` helper - Implement a cleanup stack in `StartTunnel` to ensure orderly resource release on error - Streamline temporary file and mount lifecycles in `ConfigureResolvConf` and `BlockHostServices` - Ensure `Tunnel.Close()` also closes the underlying TUN device - Reduce redundant manual cleanup calls using defer-based error handling --- internal/namespace/namespace.go | 121 ++++++++++++++-------------------------- 1 file changed, 42 insertions(+), 79 deletions(-) (limited to 'internal/namespace') diff --git a/internal/namespace/namespace.go b/internal/namespace/namespace.go index 0f2618b..6f56a84 100644 --- a/internal/namespace/namespace.go +++ b/internal/namespace/namespace.go @@ -84,7 +84,6 @@ func Bootstrap() (err error) { }() // 0. Validate current arguments for null bytes before proceeding. - // If any argument contains a null byte, syscall.Exec will fail with 'invalid argument'. for i, arg := range os.Args { for j := 0; j < len(arg); j++ { if arg[j] == 0 { @@ -98,57 +97,22 @@ func Bootstrap() (err error) { return fmt.Errorf("failed to get executable path: %w", err) } - // 1. Create a secure temporary file for the launcher binary. - // os.CreateTemp ensures a unique, unpredictable filename and restrictive permissions. - tmpFile, err := os.CreateTemp("", "wg-wrap-launcher-") - if err != nil { - return fmt.Errorf("failed to create temp launcher file: %w", err) - } - launcherPath := tmpFile.Name() - - // 2. Write the embedded launcher binary to the temp file. - if _, err := tmpFile.Write(launcherBytes); err != nil { - _ = tmpFile.Close() - _ = os.Remove(launcherPath) - return fmt.Errorf("failed to write launcher binary: %w", err) - } - - // Ensure the binary is executable (0700) - if err := tmpFile.Chmod(0700); err != nil { - _ = tmpFile.Close() - _ = os.Remove(launcherPath) - return fmt.Errorf("failed to set launcher permissions: %w", err) - } - - // 2b. Open a read-only fd of the launcher to exec - execFd, err := syscall.Open(launcherPath, syscall.O_RDONLY, 0) + execFd, launcherPath, err := prepareLauncher() if err != nil { - _ = tmpFile.Close() - _ = os.Remove(launcherPath) - return fmt.Errorf("failed to open launcher for exec: %w", err) + return err } fdsToClose = append(fdsToClose, execFd) + _ = os.Remove(launcherPath) // Unlink early; fd remains valid - // Close the write file descriptor (to avoid ETXTBSY) - _ = tmpFile.Close() - - // Unlink the file from disk (makes it invisible and ensures it is deleted on exit) - _ = os.Remove(launcherPath) - - // Clear close-on-exec so it remains open across syscall.Exec + // Clear close-on-exec if flags, err := unix.FcntlInt(uintptr(execFd), unix.F_GETFD, 0); err == nil { _, _ = unix.FcntlInt(uintptr(execFd), unix.F_SETFD, flags&^unix.FD_CLOEXEC) } // 3. Prepare arguments for the launcher. - // The launcher expects: launcher [args...] args := []string{self} args = append(args, os.Args[1:]...) - // 4. Replace the current process with the launcher. - // We must check for null bytes in the arguments here because syscall.Exec - // (which calls execve) will return 'invalid argument' (EINVAL) if any - // string in the argv array contains a null byte. for i, arg := range args { for j := 0; j < len(arg); j++ { if arg[j] == 0 { @@ -164,7 +128,7 @@ func Bootstrap() (err error) { } fdsToClose = append(fdsToClose, hostNetFd) - // Clear close-on-exec so it remains open across syscall.Exec + // Clear close-on-exec if flags, err := unix.FcntlInt(uintptr(hostNetFd), unix.F_GETFD, 0); err == nil { _, _ = unix.FcntlInt(uintptr(hostNetFd), unix.F_SETFD, flags&^unix.FD_CLOEXEC) } @@ -203,12 +167,13 @@ func BootstrapJoin(targetPid int) (err error) { var fdsToClose []int defer func() { - for _, fd := range fdsToClose { - _ = syscall.Close(fd) + if err != nil { + for _, fd := range fdsToClose { + _ = syscall.Close(fd) + } } }() - // Validate current arguments for null bytes before proceeding. for i, arg := range os.Args { for j := 0; j < len(arg); j++ { if arg[j] == 0 { @@ -222,48 +187,17 @@ func BootstrapJoin(targetPid int) (err error) { return fmt.Errorf("failed to get executable path: %w", err) } - // 1. Create a secure temporary file for the launcher binary. - tmpFile, err := os.CreateTemp("", "wg-wrap-launcher-") + execFd, launcherPath, err := prepareLauncher() if err != nil { - return fmt.Errorf("failed to create temp launcher file: %w", err) - } - launcherPath := tmpFile.Name() - - // 2. Write the embedded launcher binary to the temp file. - if _, err := tmpFile.Write(launcherBytes); err != nil { - _ = tmpFile.Close() - _ = os.Remove(launcherPath) - return fmt.Errorf("failed to write launcher binary: %w", err) - } - - // Ensure the binary is executable (0700) - if err := tmpFile.Chmod(0700); err != nil { - _ = tmpFile.Close() - _ = os.Remove(launcherPath) - return fmt.Errorf("failed to set launcher permissions: %w", err) - } - - // 2b. Open a read-only fd of the launcher to exec - execFd, err := syscall.Open(launcherPath, syscall.O_RDONLY, 0) - if err != nil { - _ = tmpFile.Close() - _ = os.Remove(launcherPath) - return fmt.Errorf("failed to open launcher for exec: %w", err) + return err } fdsToClose = append(fdsToClose, execFd) - - // Close the write file descriptor (to avoid ETXTBSY) - _ = tmpFile.Close() - - // Unlink the file from disk (makes it invisible and ensures it is deleted on exit) _ = os.Remove(launcherPath) - // Clear close-on-exec so it remains open across syscall.Exec if flags, err := unix.FcntlInt(uintptr(execFd), unix.F_GETFD, 0); err == nil { _, _ = unix.FcntlInt(uintptr(execFd), unix.F_SETFD, flags&^unix.FD_CLOEXEC) } - // 3. Prepare arguments for the launcher. args := []string{self} args = append(args, os.Args[1:]...) @@ -275,8 +209,6 @@ func BootstrapJoin(targetPid int) (err error) { } } - // Set environment variables to tell the C launcher to join, - // and to tell the second wg-wrap instance that we are in a joined session. env := append(os.Environ(), fmt.Sprintf("WG_WRAP_JOIN_PID=%d", targetPid), "WG_WRAP_JOINED=1", @@ -289,3 +221,34 @@ func BootstrapJoin(targetPid int) (err error) { return nil } + +func prepareLauncher() (int, string, error) { + tmpFile, err := os.CreateTemp("", "wg-wrap-launcher-") + if err != nil { + return 0, "", fmt.Errorf("failed to create temp launcher file: %w", err) + } + launcherPath := tmpFile.Name() + + defer func() { + if err != nil { + _ = tmpFile.Close() + _ = os.Remove(launcherPath) + } + }() + + if _, err = tmpFile.Write(launcherBytes); err != nil { + return 0, "", fmt.Errorf("failed to write launcher binary: %w", err) + } + + if err = tmpFile.Chmod(0700); err != nil { + return 0, "", fmt.Errorf("failed to set launcher permissions: %w", err) + } + + execFd, err := syscall.Open(launcherPath, syscall.O_RDONLY, 0) + if err != nil { + return 0, "", fmt.Errorf("failed to open launcher for exec: %w", err) + } + + _ = tmpFile.Close() + return execFd, launcherPath, nil +} -- cgit v1.2.3