manager: use stricter handle inheritability

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

2 files changed, 25 insertions, 83 deletions

diff --git a/manager/ipc_pipe.go b/manager/ipc_pipe.go
deleted file mode 100644
index e1723fbd..00000000
--- a/manager/ipc_pipe.go
+++ /dev/null
@@ -1,71 +0,0 @@
-/* SPDX-License-Identifier: MIT
- *
- * Copyright (C) 2019-2021 WireGuard LLC. All Rights Reserved.
- */
-
-package manager
-
-import (
-	"os"
-	"strconv"
-
-	"golang.org/x/sys/windows"
-)
-
-func makeInheritableAndGetStr(f *os.File) (str string, err error) {
-	sc, err := f.SyscallConn()
-	if err != nil {
-		return
-	}
-	err2 := sc.Control(func(fd uintptr) {
-		err = windows.SetHandleInformation(windows.Handle(fd), windows.HANDLE_FLAG_INHERIT, windows.HANDLE_FLAG_INHERIT)
-		str = strconv.FormatUint(uint64(fd), 10)
-	})
-	if err2 != nil {
-		err = err2
-	}
-	return
-}
-
-func inheritableEvents() (ourEvents *os.File, theirEvents *os.File, theirEventStr string, err error) {
-	theirEvents, ourEvents, err = os.Pipe()
-	if err != nil {
-		return
-	}
-	theirEventStr, err = makeInheritableAndGetStr(theirEvents)
-	if err != nil {
-		theirEvents.Close()
-		ourEvents.Close()
-		return
-	}
-	return
-}
-
-func inheritableSocketpairEmulation() (ourReader *os.File, theirReader *os.File, theirReaderStr string, ourWriter *os.File, theirWriter *os.File, theirWriterStr string, err error) {
-	ourReader, theirWriter, err = os.Pipe()
-	if err != nil {
-		return
-	}
-	theirWriterStr, err = makeInheritableAndGetStr(theirWriter)
-	if err != nil {
-		ourReader.Close()
-		theirWriter.Close()
-		return
-	}
-
-	theirReader, ourWriter, err = os.Pipe()
-	if err != nil {
-		ourReader.Close()
-		theirWriter.Close()
-		return
-	}
-	theirReaderStr, err = makeInheritableAndGetStr(theirReader)
-	if err != nil {
-		ourReader.Close()
-		theirWriter.Close()
-		theirReader.Close()
-		ourWriter.Close()
-		return
-	}
-	return
-}
diff --git a/manager/service.go b/manager/service.go
index eda96817..b13870fd 100644
--- a/manager/service.go
+++ b/manager/service.go
@@ -10,6 +10,7 @@ import (
 	"log"
 	"os"
 	"runtime"
+	"strconv"
 	"sync"
 	"syscall"
 	"time"
@@ -172,22 +173,23 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
 				first = false
 			}
 
-			// TODO: we lock the OS thread so that these inheritable handles don't escape into other processes that
-			// might be running in parallel Go routines. But the Go runtime is strange and who knows what's really
-			// happening with these or what is inherited. We need to do some analysis to be certain of what's going on.
-			runtime.LockOSThread()
-			ourReader, theirReader, theirReaderStr, ourWriter, theirWriter, theirWriterStr, err := inheritableSocketpairEmulation()
+			ourReader, theirWriter, err := os.Pipe()
 			if err != nil {
-				log.Printf("Unable to create two inheritable RPC pipes: %v", err)
+				log.Printf("Unable to create pipe: %v", err)
 				return
 			}
-			ourEvents, theirEvents, theirEventStr, err := inheritableEvents()
+			theirReader, ourWriter, err := os.Pipe()
 			if err != nil {
-				log.Printf("Unable to create one inheritable events pipe: %v", err)
+				log.Printf("Unable to create pipe: %v", err)
+				return
+			}
+			theirEvents, ourEvents, err := os.Pipe()
+			if err != nil {
+				log.Printf("Unable to create pipe: %v", err)
 				return
 			}
 			IPCServerListen(ourReader, ourWriter, ourEvents, elevatedToken)
-			theirLogMapping, theirLogMappingHandle, err := ringlogger.Global.ExportInheritableMappingHandleStr()
+			theirLogMapping, err := ringlogger.Global.ExportInheritableMappingHandle()
 			if err != nil {
 				log.Printf("Unable to export inheritable mapping handle for logging: %v", err)
 				return
@@ -197,6 +199,11 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
 			attr := &os.ProcAttr{
 				Sys: &syscall.SysProcAttr{
 					Token: syscall.Token(runToken),
+					AdditionalInheritedHandles: []syscall.Handle{
+						syscall.Handle(theirReader.Fd()),
+						syscall.Handle(theirWriter.Fd()),
+						syscall.Handle(theirEvents.Fd()),
+						syscall.Handle(theirLogMapping)},
 				},
 				Files: []*os.File{devNull, devNull, devNull},
 				Dir:   userProfileDirectory,
@@ -204,7 +211,14 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
 			procsLock.Lock()
 			var proc *os.Process
 			if alive := aliveSessions[session]; alive {
-				proc, err = os.StartProcess(path, []string{path, "/ui", theirReaderStr, theirWriterStr, theirEventStr, theirLogMapping}, attr)
+				proc, err = os.StartProcess(path, []string{
+					path,
+					"/ui",
+					strconv.FormatUint(uint64(theirReader.Fd()), 10),
+					strconv.FormatUint(uint64(theirWriter.Fd()), 10),
+					strconv.FormatUint(uint64(theirEvents.Fd()), 10),
+					strconv.FormatUint(uint64(theirLogMapping), 10),
+				}, attr)
 			} else {
 				err = errors.New("Session has logged out")
 			}
@@ -212,8 +226,7 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
 			theirReader.Close()
 			theirWriter.Close()
 			theirEvents.Close()
-			windows.Close(theirLogMappingHandle)
-			runtime.UnlockOSThread()
+			windows.Close(theirLogMapping)
 			if err != nil {
 				ourReader.Close()
 				ourWriter.Close()