shim: user switcher process management struct

This change moves all user switcher and shim management to the shim package and withholds output while shim is alive. This also eliminated all exit scenarios where revert is skipped.

Signed-off-by: Ophestra Umiker <cat@ophivana.moe>

internal/app/app.go

@@ -1,10 +1,10 @@
 package app
 
 import (
-	"os/exec"
 	"sync"
 
 	"git.ophivana.moe/security/fortify/internal"
+	"git.ophivana.moe/security/fortify/internal/shim"
 )
 
 type App interface {
@@ -26,10 +26,8 @@ type app struct {
 	id *ID
 	// operating system interface
 	os internal.System
-	// underlying user switcher process
+	// shim process manager
-	cmd *exec.Cmd
+	shim *shim.Shim
-	// shim setup abort reason and completion
-	abort chan error
 	// child process related information
 	seal *appSeal
 	// error returned waiting for process
@@ -50,8 +48,8 @@ func (a *app) String() string {
 	a.lock.RLock()
 	defer a.lock.RUnlock()
 
-	if a.cmd != nil {
+	if a.shim != nil {
-		return a.cmd.String()
+		return a.shim.String()
 	}
 
 	if a.seal != nil {

internal/app/start.go

@@ -3,12 +3,10 @@ package app
 import (
 	"errors"
 	"fmt"
-	"os"
 	"os/exec"
 	"path"
 	"path/filepath"
 	"strings"
-	"time"
 
 	"git.ophivana.moe/security/fortify/helper"
 	"git.ophivana.moe/security/fortify/internal/fmsg"
@@ -17,7 +15,8 @@ import (
 	"git.ophivana.moe/security/fortify/internal/system"
 )
 
-// Start starts the fortified child
+// Start selects a user switcher and starts shim.
+// Note that Wait must be called regardless of error returned by Start.
 func (a *app) Start() error {
 	a.lock.Lock()
 	defer a.lock.Unlock()
@@ -41,12 +40,8 @@ func (a *app) Start() error {
 		}
 	}
 
-	if err := a.seal.sys.Commit(); err != nil {
-		return err
-	}
-
 	// select command builder
-	var commandBuilder func(shimEnv string) (args []string)
+	var commandBuilder shim.CommandBuilder
 	switch a.seal.launchOption {
 	case LaunchMethodSudo:
 		commandBuilder = a.commandBuilderSudo
@@ -56,60 +51,45 @@ func (a *app) Start() error {
 		panic("unreachable")
 	}
 
-	// configure child process
+	// construct shim manager
-	confSockPath := path.Join(a.seal.share, "shim")
+	a.shim = shim.New(a.seal.toolPath, uint32(a.seal.sys.UID()), path.Join(a.seal.share, "shim"), a.seal.wl,
-	a.cmd = exec.Command(a.seal.toolPath, commandBuilder(shim.EnvShim+"="+confSockPath)...)
+		&shim.Payload{
-	a.cmd.Env = []string{}
-	a.cmd.Stdin, a.cmd.Stdout, a.cmd.Stderr = os.Stdin, os.Stdout, os.Stderr
-	a.cmd.Dir = a.seal.RunDirPath
-
-	a.abort = make(chan error)
-	procReady := make(chan struct{})
-	if err := shim.ServeConfig(confSockPath, a.abort, func() {
-		<-procReady
-		if err := a.cmd.Process.Signal(os.Interrupt); err != nil {
-			fmsg.Println("cannot kill shim on faulted setup:", err)
-		}
-		fmt.Print("\r")
-	}, a.seal.sys.UID(), &shim.Payload{
 			Argv:  a.seal.command,
 			Exec:  shimExec,
 			Bwrap: a.seal.sys.bwrap,
 			WL:    a.seal.wl != nil,
 
 			Verbose: fmsg.Verbose(),
-	}, a.seal.wl); err != nil {
+		},
-		a.abort <- err
+	)
-		<-a.abort
-		return fmsg.WrapErrorSuffix(err,
-			"cannot serve shim setup:")
-	}
 
-	// start shim
+	// startup will go ahead, commit system setup
-	fmsg.VPrintln("starting shim as target user:", a.cmd)
+	if err := a.seal.sys.Commit(); err != nil {
-	if err := a.cmd.Start(); err != nil {
+		return err
-		return fmsg.WrapErrorSuffix(err,
-			"cannot start process:")
 	}
-	startTime := time.Now().UTC()
+	a.seal.sys.needRevert = true
-	close(procReady)
 
-	// create process state
+	if startTime, err := a.shim.Start(commandBuilder); err != nil {
+		return err
+	} else {
+		// shim start and setup success, create process state
 		sd := state.State{
-		PID:        a.cmd.Process.Pid,
+			PID:        a.shim.Unwrap().Process.Pid,
 			Command:    a.seal.command,
 			Capability: a.seal.et,
 			Method:     method[a.seal.launchOption],
-		Argv:       a.cmd.Args,
+			Argv:       a.shim.Unwrap().Args,
-		Time:       startTime,
+			Time:       *startTime,
 		}
 
 		// register process state
-	var err = new(StateStoreError)
+		var err0 = new(StateStoreError)
-	err.Inner, err.DoErr = a.seal.store.Do(func(b state.Backend) {
+		err0.Inner, err0.DoErr = a.seal.store.Do(func(b state.Backend) {
-		err.InnerErr = b.Save(&sd)
+			err0.InnerErr = b.Save(&sd)
 		})
-	return err.equiv("cannot save process state:")
+		a.seal.sys.saveState = true
+		return err0.equiv("cannot save process state:")
+	}
 }
 
 // StateStoreError is returned for a failed state save
@@ -173,8 +153,12 @@ func (a *app) Wait() (int, error) {
 
 	var r int
 
+	if cmd := a.shim.Unwrap(); cmd == nil {
+		// failure prior to process start
+		r = 255
+	} else {
 		// wait for process and resolve exit code
-	if err := a.cmd.Wait(); err != nil {
+		if err := cmd.Wait(); err != nil {
 			var exitError *exec.ExitError
 			if !errors.As(err, &exitError) {
 				// should be unreachable
@@ -184,10 +168,13 @@ func (a *app) Wait() (int, error) {
 			// store non-zero return code
 			r = exitError.ExitCode()
 		} else {
-		r = a.cmd.ProcessState.ExitCode()
+			r = cmd.ProcessState.ExitCode()
+		}
+		fmsg.VPrintf("process %d exited with exit code %d", cmd.Process.Pid, r)
 	}
 
-	fmsg.VPrintf("process %d exited with exit code %d", a.cmd.Process.Pid, r)
+	// child process exited, resume output
+	fmsg.Resume()
 
 	// close wayland connection
 	if a.seal.wl != nil {
@@ -201,9 +188,11 @@ func (a *app) Wait() (int, error) {
 	e.Inner, e.DoErr = a.seal.store.Do(func(b state.Backend) {
 		e.InnerErr = func() error {
 			// destroy defunct state entry
-			if err := b.Destroy(a.cmd.Process.Pid); err != nil {
+			if cmd := a.shim.Unwrap(); cmd != nil && a.seal.sys.saveState {
+				if err := b.Destroy(cmd.Process.Pid); err != nil {
 					return err
 				}
+			}
 
 			// enablements of remaining launchers
 			rt, ec := new(system.Enablements), new(system.Criteria)
@@ -243,8 +232,7 @@ func (a *app) Wait() (int, error) {
 				}
 			}
 
-			a.abort <- errors.New("shim exited")
+			a.shim.AbortWait(errors.New("shim exited"))
-			<-a.abort
 			if err := a.seal.sys.Revert(ec); err != nil {
 				return err.(RevertCompoundError)
 			}

internal/app/system.go

@@ -22,6 +22,8 @@ type appSealSys struct {
 	// target user sealed from config
 	user *user.User
 
+	needRevert bool
+	saveState  bool
 	*system.I
 
 	// protected by upstream mutex

internal/shim/parent.go

@@ -1,106 +1,202 @@
 package shim
 
 import (
-	"encoding/gob"
 	"errors"
 	"net"
+	"os"
+	"os/exec"
+	"sync"
+	"sync/atomic"
 	"syscall"
+	"time"
 
 	"git.ophivana.moe/security/fortify/acl"
 	"git.ophivana.moe/security/fortify/internal/fmsg"
 )
 
-// called in the parent process
+// used by the parent process
 
-func ServeConfig(socket string, abort chan error, killShim func(), uid int, payload *Payload, wl *Wayland) error {
+type Shim struct {
-	if payload.WL {
+	// user switcher process
-		if f, err := net.DialUnix("unix", nil, &net.UnixAddr{Name: wl.Path, Net: "unix"}); err != nil {
+	cmd *exec.Cmd
-			return err
+	// uid of shim target user
-		} else {
+	uid uint32
-			fmsg.VPrintf("connected to wayland at %q", wl.Path)
+	// whether to check shim pid
-			wl.UnixConn = f
+	checkPid bool
-		}
+	// user switcher executable path
-	}
+	executable string
-
+	// path to setup socket
-	// setup success state accessed by abort
+	socket string
-	var success bool
+	// shim setup abort reason and completion
-
+	abort     chan error
-	if c, err := net.ListenUnix("unix", &net.UnixAddr{Name: socket, Net: "unix"}); err != nil {
+	abortErr  atomic.Pointer[error]
-		return err
+	abortOnce sync.Once
-	} else {
+	// wayland mediation, nil if disabled
-		c.SetUnlinkOnClose(true)
+	wl *Wayland
-
+	// shim setup payload
-		go func() {
+	payload *Payload
-			err1 := <-abort
+}
-			if !success {
+
-				fmsg.VPrintln("aborting shim setup, reason:", err1)
+func New(executable string, uid uint32, socket string, wl *Wayland, payload *Payload) *Shim {
-				if err1 = c.Close(); err1 != nil {
+	// checkPid is impossible at the moment since there is no way to obtain shim's pid
-					fmsg.Println("cannot abort shim setup:", err1)
+	// this feature is disabled here until sudo is replaced by fortify suid wrapper
-				}
+	return &Shim{uid: uid, executable: executable, socket: socket, wl: wl, payload: payload}
-			}
+}
-			close(abort)
+
-		}()
+func (s *Shim) String() string {
-
+	if s.cmd == nil {
-		fmsg.VPrintf("configuring shim on socket %q", socket)
+		return "(unused shim manager)"
-		if err = acl.UpdatePerm(socket, uid, acl.Read, acl.Write, acl.Execute); err != nil {
+	}
-			fmsg.Println("cannot change permissions of shim setup socket:", err)
+	return s.cmd.String()
-		}
+}
-
+
-		go func() {
+func (s *Shim) Unwrap() *exec.Cmd {
-			var conn *net.UnixConn
+	return s.cmd
-			if conn, err = c.AcceptUnix(); err != nil {
+}
-				if errors.Is(err, net.ErrClosed) {
+
-					fmsg.VPrintln("accept failed due to shim setup abort")
+func (s *Shim) Abort(err error) {
-				} else {
+	s.abortOnce.Do(func() {
-					fmsg.Println("cannot accept connection from shim:", err)
+		s.abortErr.Store(&err)
-				}
+		// s.abort is buffered so this will never block
-			} else {
+		s.abort <- err
-				if err = gob.NewEncoder(conn).Encode(*payload); err != nil {
+	})
-					fmsg.Println("cannot stream shim payload:", err)
+}
-					killShim()
+
-					return
+func (s *Shim) AbortWait(err error) {
-				}
+	s.Abort(err)
-
+	<-s.abort
-				if payload.WL {
+}
-					// get raw connection
+
-					var rc syscall.RawConn
+type CommandBuilder func(shimEnv string) (args []string)
-					if rc, err = wl.SyscallConn(); err != nil {
+
-						fmsg.Println("cannot obtain raw wayland connection:", err)
+func (s *Shim) Start(f CommandBuilder) (*time.Time, error) {
-						killShim()
+	var (
-						return
+		cf     chan *net.UnixConn
-					} else {
+		accept func()
-						go func() {
+	)
-							// pass wayland socket fd
+
-							if err = rc.Control(func(fd uintptr) {
+	// listen on setup socket
-								if _, _, err = conn.WriteMsgUnix(nil, syscall.UnixRights(int(fd)), nil); err != nil {
+	if c, a, err := s.serve(); err != nil {
-									fmsg.Println("cannot pass wayland connection to shim:", err)
+		return nil, fmsg.WrapErrorSuffix(err,
-									killShim()
+			"cannot listen on shim setup socket:")
-									return
+	} else {
-								}
+		// accepts a connection after each call to accept
-								_ = conn.Close()
+		// connections are sent to the channel cf
-
+		cf, accept = c, a
-								// block until shim exits
+	}
-								<-wl.done
+
-								fmsg.VPrintln("releasing wayland connection")
+	// start user switcher process and save time
-							}); err != nil {
+	s.cmd = exec.Command(s.executable, f(EnvShim+"="+s.socket)...)
-								fmsg.Println("cannot obtain wayland connection fd:", err)
+	s.cmd.Env = []string{}
-							}
+	s.cmd.Stdin, s.cmd.Stdout, s.cmd.Stderr = os.Stdin, os.Stdout, os.Stderr
-						}()
+	s.cmd.Dir = "/"
-					}
+	fmsg.VPrintln("starting shim via user switcher:", s.cmd)
-				} else {
+	fmsg.Withhold() // withhold messages to stderr
-					_ = conn.Close()
+	if err := s.cmd.Start(); err != nil {
-				}
+		return nil, fmsg.WrapErrorSuffix(err,
-			}
+			"cannot start user switcher:")
-
+	}
-			success = true
+	startTime := time.Now().UTC()
-			if err = c.Close(); err != nil {
+
-				if errors.Is(err, net.ErrClosed) {
+	// kill shim if something goes wrong and an error is returned
-					fmsg.VPrintln("close failed due to shim setup abort")
+	killShim := func() {
-				} else {
+		if err := s.cmd.Process.Signal(os.Interrupt); err != nil {
-					fmsg.Println("cannot close shim socket:", err)
+			fmsg.Println("cannot terminate shim on faulted setup:", err)
-				}
+		}
-			}
+	}
-		}()
+	defer func() { killShim() }()
-		return nil
+
-	}
+	accept()
+	conn := <-cf
+	if conn == nil {
+		return &startTime, fmsg.WrapErrorSuffix(*s.abortErr.Load(), "cannot accept call on setup socket:")
+	}
+
+	// authenticate against called provided uid and shim pid
+	if cred, err := peerCred(conn); err != nil {
+		return &startTime, fmsg.WrapErrorSuffix(*s.abortErr.Load(), "cannot retrieve shim credentials:")
+	} else if cred.Uid != s.uid {
+		fmsg.Printf("process %d owned by user %d tried to connect, expecting %d",
+			cred.Pid, cred.Uid, s.uid)
+		err = errors.New("compromised fortify build")
+		s.Abort(err)
+		return &startTime, err
+	} else if s.checkPid && cred.Pid != int32(s.cmd.Process.Pid) {
+		fmsg.Printf("process %d tried to connect to shim setup socket, expecting shim %d",
+			cred.Pid, s.cmd.Process.Pid)
+		err = errors.New("compromised target user")
+		s.Abort(err)
+		return &startTime, err
+	}
+
+	// serve payload and wayland fd if enabled
+	// this also closes the connection
+	err := s.payload.serve(conn, s.wl)
+	if err == nil {
+		killShim = func() {}
+	}
+	s.Abort(err) // aborting with nil indicates success
+	return &startTime, err
+}
+
+func (s *Shim) serve() (chan *net.UnixConn, func(), error) {
+	if s.abort != nil {
+		panic("attempted to serve shim setup twice")
+	}
+	s.abort = make(chan error, 1)
+
+	cf := make(chan *net.UnixConn)
+	accept := make(chan struct{}, 1)
+
+	if l, err := net.ListenUnix("unix", &net.UnixAddr{Name: s.socket, Net: "unix"}); err != nil {
+		return nil, nil, err
+	} else {
+		l.SetUnlinkOnClose(true)
+
+		fmsg.VPrintf("listening on shim setup socket %q", s.socket)
+		if err = acl.UpdatePerm(s.socket, int(s.uid), acl.Read, acl.Write, acl.Execute); err != nil {
+			fmsg.Println("cannot append ACL entry to shim setup socket:", err)
+			s.Abort(err) // ensures setup socket cleanup
+		}
+
+		go func() {
+			for {
+				select {
+				case err = <-s.abort:
+					if err != nil {
+						fmsg.VPrintln("aborting shim setup, reason:", err)
+					}
+					if err = l.Close(); err != nil {
+						fmsg.Println("cannot close setup socket:", err)
+					}
+					close(s.abort)
+					close(cf)
+					return
+				case <-accept:
+					if conn, err0 := l.AcceptUnix(); err0 != nil {
+						s.Abort(err0) // does not block, breaks loop
+						cf <- nil     // receiver sees nil value and loads err0 stored during abort
+					} else {
+						cf <- conn
+					}
+				}
+			}
+		}()
+	}
+
+	return cf, func() { accept <- struct{}{} }, nil
+}
+
+// peerCred fetches peer credentials of conn
+func peerCred(conn *net.UnixConn) (ucred *syscall.Ucred, err error) {
+	var raw syscall.RawConn
+	if raw, err = conn.SyscallConn(); err != nil {
+		return
+	}
+
+	err0 := raw.Control(func(fd uintptr) {
+		ucred, err = syscall.GetsockoptUcred(int(fd), syscall.SOL_SOCKET, syscall.SO_PEERCRED)
+	})
+	err = errors.Join(err, err0)
+	return
 }

internal/shim/payload.go

@@ -1,6 +1,13 @@
 package shim
 
-import "git.ophivana.moe/security/fortify/helper/bwrap"
+import (
+	"encoding/gob"
+	"errors"
+	"net"
+
+	"git.ophivana.moe/security/fortify/helper/bwrap"
+	"git.ophivana.moe/security/fortify/internal/fmsg"
+)
 
 const EnvShim = "FORTIFY_SHIM"
 
@@ -17,3 +24,19 @@ type Payload struct {
 	// verbosity pass through
 	Verbose bool
 }
+
+func (p *Payload) serve(conn *net.UnixConn, wl *Wayland) error {
+	if err := gob.NewEncoder(conn).Encode(*p); err != nil {
+		return fmsg.WrapErrorSuffix(err,
+			"cannot stream shim payload:")
+	}
+
+	if wl != nil {
+		if err := wl.WriteUnix(conn); err != nil {
+			return errors.Join(err, conn.Close())
+		}
+	}
+
+	return fmsg.WrapErrorSuffix(conn.Close(),
+		"cannot close setup connection:")
+}

internal/shim/wayland.go

@@ -1,8 +1,12 @@
 package shim
 
 import (
+	"fmt"
 	"net"
 	"sync"
+	"syscall"
+
+	"git.ophivana.moe/security/fortify/internal/fmsg"
 )
 
 // Wayland implements wayland mediation.
@@ -11,7 +15,7 @@ type Wayland struct {
 	Path string
 
 	// wayland connection
-	*net.UnixConn
+	conn *net.UnixConn
 
 	connErr error
 	sync.Once
@@ -19,10 +23,46 @@ type Wayland struct {
 	done chan struct{}
 }
 
+func (wl *Wayland) WriteUnix(conn *net.UnixConn) error {
+	// connect to host wayland socket
+	if f, err := net.DialUnix("unix", nil, &net.UnixAddr{Name: wl.Path, Net: "unix"}); err != nil {
+		return fmsg.WrapErrorSuffix(err,
+			fmt.Sprintf("cannot connect to wayland at %q:", wl.Path))
+	} else {
+		fmsg.VPrintf("connected to wayland at %q", wl.Path)
+		wl.conn = f
+	}
+
+	// set up for passing wayland socket
+	if rc, err := wl.conn.SyscallConn(); err != nil {
+		return fmsg.WrapErrorSuffix(err, "cannot obtain raw wayland connection:")
+	} else {
+		ec := make(chan error)
+		go func() {
+			// pass wayland connection fd
+			if err = rc.Control(func(fd uintptr) {
+				if _, _, err = conn.WriteMsgUnix(nil, syscall.UnixRights(int(fd)), nil); err != nil {
+					ec <- fmsg.WrapErrorSuffix(err, "cannot pass wayland connection to shim:")
+					return
+				}
+				ec <- nil
+
+				// block until shim exits
+				<-wl.done
+				fmsg.VPrintln("releasing wayland connection")
+			}); err != nil {
+				ec <- fmsg.WrapErrorSuffix(err, "cannot obtain wayland connection fd:")
+				return
+			}
+		}()
+		return <-ec
+	}
+}
+
 func (wl *Wayland) Close() error {
 	wl.Do(func() {
 		close(wl.done)
-		wl.connErr = wl.UnixConn.Close()
+		wl.connErr = wl.conn.Close()
 	})
 
 	return wl.connErr

main.go

@@ -53,15 +53,18 @@ func main() {
 	tryState()
 
 	// invoke app
-	r := 1
 	a, err := app.New(os)
 	if err != nil {
 		fmsg.Fatalf("cannot create app: %s\n", err)
 	} else if err = a.Seal(loadConfig()); err != nil {
-		logBaseError(err, "fortify: cannot seal app:")
+		logBaseError(err, "cannot seal app:")
 	} else if err = a.Start(); err != nil {
-		logBaseError(err, "fortify: cannot start app:")
+		logBaseError(err, "cannot start app:")
-	} else if r, err = a.Wait(); err != nil {
+	}
+
+	var r int
+	// wait must be called regardless of result of start
+	if r, err = a.Wait(); err != nil {
 		if r < 1 {
 			r = 1
 		}
@@ -70,5 +73,5 @@ func main() {
 	if err = a.WaitErr(); err != nil {
 		fmsg.Println("inner wait failed:", err)
 	}
-	os.Exit(r)
+	fmsg.Exit(r)
 }