app: separate instance from process state

This works better for the implementation.

Signed-off-by: Ophestra <cat@gensokyo.uk>

fst/app.go

@@ -6,16 +6,22 @@ import (
 )
 
 type App interface {
-	// ID returns a copy of App's unique ID.
+	// ID returns a copy of [fst.ID] held by App.
 	ID() ID
-	// Run sets up the system and runs the App.
-	Run(ctx context.Context, rs *RunState) error
 
-	Seal(config *Config) error
+	// Seal determines the outcome of config as a [SealedApp].
+	// The value of config might be overwritten and must not be used again.
+	Seal(config *Config) (SealedApp, error)
+
 	String() string
 }
 
-// RunState stores the outcome of a call to [App.Run].
+type SealedApp interface {
+	// Run commits sealed system setup and starts the app process.
+	Run(ctx context.Context, rs *RunState) error
+}
+
+// RunState stores the outcome of a call to [SealedApp.Run].
 type RunState struct {
 	// Time is the exact point in time where the process was created.
 	// Location must be set to UTC.

internal/app/app.go

@@ -2,6 +2,7 @@ package app
 
 import (
 	"fmt"
+	"log"
 	"sync"
 
 	"git.gensokyo.uk/security/fortify/fst"
@@ -20,15 +21,23 @@ func New(os sys.State) (fst.App, error) {
 	return a, err
 }
 
+func MustNew(os sys.State) fst.App {
+	a, err := New(os)
+	if err != nil {
+		log.Fatalf("cannot create app: %v", err)
+	}
+	return a
+}
+
 type app struct {
 	id  *stringPair[fst.ID]
 	sys sys.State
 
-	*appSeal
+	*outcome
 	mu sync.RWMutex
 }
 
-func (a *app) ID() fst.ID { return a.id.unwrap() }
+func (a *app) ID() fst.ID { a.mu.RLock(); defer a.mu.RUnlock(); return a.id.unwrap() }
 
 func (a *app) String() string {
 	if a == nil {
@@ -38,32 +47,33 @@ func (a *app) String() string {
 	a.mu.RLock()
 	defer a.mu.RUnlock()
 
-	if a.appSeal != nil {
+	if a.outcome != nil {
-		if a.appSeal.user.uid == nil {
+		if a.outcome.user.uid == nil {
 			return fmt.Sprintf("(sealed app %s with invalid uid)", a.id)
 		}
-		return fmt.Sprintf("(sealed app %s as uid %s)", a.id, a.appSeal.user.uid)
+		return fmt.Sprintf("(sealed app %s as uid %s)", a.id, a.outcome.user.uid)
 	}
 
 	return fmt.Sprintf("(unsealed app %s)", a.id)
 }
 
-func (a *app) Seal(config *fst.Config) (err error) {
+func (a *app) Seal(config *fst.Config) (fst.SealedApp, error) {
 	a.mu.Lock()
 	defer a.mu.Unlock()
 
-	if a.appSeal != nil {
+	if a.outcome != nil {
 		panic("app sealed twice")
 	}
 	if config == nil {
-		return fmsg.WrapError(ErrConfig,
+		return nil, fmsg.WrapError(ErrConfig,
 			"attempted to seal app with nil config")
 	}
 
-	seal := new(appSeal)
+	seal := new(outcome)
-	err = seal.finalise(a.sys, config, a.id.String())
+	seal.id = a.id
+	err := seal.finalise(a.sys, config)
 	if err == nil {
-		a.appSeal = seal
+		a.outcome = seal
 	}
-	return
+	return seal, err
 }

internal/app/app_test.go

@@ -29,17 +29,21 @@ func TestApp(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			a := app.NewWithID(tc.id, tc.os)
-
+			var (
+				gotSys   *system.I
+				gotBwrap *bwrap.Config
+			)
 			if !t.Run("seal", func(t *testing.T) {
-				if err := a.Seal(tc.config); err != nil {
+				if sa, err := a.Seal(tc.config); err != nil {
 					t.Errorf("Seal: error = %v", err)
+					return
+				} else {
+					gotSys, gotBwrap = app.AppSystemBwrap(a, sa)
 				}
 			}) {
 				return
 			}
 
-			gotSys, gotBwrap := app.AppSystemBwrap(a)
-
 			t.Run("compare sys", func(t *testing.T) {
 				if !gotSys.Equal(tc.wantSys) {
 					t.Errorf("Seal: sys = %#v, want %#v",

internal/app/export_test.go

@@ -14,7 +14,11 @@ func NewWithID(id fst.ID, os sys.State) fst.App {
 	return a
 }
 
-func AppSystemBwrap(a fst.App) (*system.I, *bwrap.Config) {
+func AppSystemBwrap(a fst.App, sa fst.SealedApp) (*system.I, *bwrap.Config) {
 	v := a.(*app)
-	return v.appSeal.sys, v.appSeal.container
+	seal := sa.(*outcome)
+	if v.outcome != seal || v.id != seal.id {
+		panic("broken app/outcome link")
+	}
+	return seal.sys, seal.container
 }

internal/app/process.go

@@ -20,12 +20,16 @@ import (
 
 const shimSetupTimeout = 5 * time.Second
 
-func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
+func (seal *outcome) Run(ctx context.Context, rs *fst.RunState) error {
-	a.mu.Lock()
+	if !seal.f.CompareAndSwap(false, true) {
-	defer a.mu.Unlock()
+		// run does much more than just starting a process; calling it twice, even if the first call fails, will result
+		// in inconsistent state that is impossible to clean up; return here to limit damage and hopefully give the
+		// other Run a chance to return
+		panic("attempted to run twice")
+	}
 
 	if rs == nil {
-		panic("attempted to pass nil state to run")
+		panic("invalid state")
 	}
 
 	/*
@@ -33,8 +37,8 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
 	*/
 
 	shimExec := [2]string{helper.BubblewrapName}
-	if len(a.appSeal.command) > 0 {
+	if len(seal.command) > 0 {
-		shimExec[1] = a.appSeal.command[0]
+		shimExec[1] = seal.command[0]
 	}
 	for i, n := range shimExec {
 		if len(n) == 0 {
@@ -54,15 +58,15 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
 		prepare/revert os state
 	*/
 
-	if err := a.appSeal.sys.Commit(ctx); err != nil {
+	if err := seal.sys.Commit(ctx); err != nil {
 		return err
 	}
-	store := state.NewMulti(a.sys.Paths().RunDirPath)
+	store := state.NewMulti(seal.runDirPath)
 	deferredStoreFunc := func(c state.Cursor) error { return nil }
 	defer func() {
 		var revertErr error
 		storeErr := new(StateStoreError)
-		storeErr.Inner, storeErr.DoErr = store.Do(a.appSeal.user.aid.unwrap(), func(c state.Cursor) {
+		storeErr.Inner, storeErr.DoErr = store.Do(seal.user.aid.unwrap(), func(c state.Cursor) {
 			revertErr = func() error {
 				storeErr.InnerErr = deferredStoreFunc(c)
 
@@ -75,7 +79,7 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
 				ec.Set(system.Process)
 				if states, err := c.Load(); err != nil {
 					// revert per-process state here to limit damage
-					return errors.Join(err, a.appSeal.sys.Revert(ec))
+					return errors.Join(err, seal.sys.Revert(ec))
 				} else {
 					if l := len(states); l == 0 {
 						fmsg.Verbose("no other launchers active, will clean up globals")
@@ -111,7 +115,7 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
 					}
 				}
 
-				err := a.appSeal.sys.Revert(ec)
+				err := seal.sys.Revert(ec)
 				if err != nil {
 					err = err.(RevertCompoundError)
 				}
@@ -129,9 +133,9 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
 	waitErr := make(chan error, 1)
 	cmd := new(shim.Shim)
 	if startTime, err := cmd.Start(
-		a.appSeal.user.aid.String(),
+		seal.user.aid.String(),
-		a.appSeal.user.supp,
+		seal.user.supp,
-		a.appSeal.bwrapSync,
+		seal.bwrapSync,
 	); err != nil {
 		return err
 	} else {
@@ -139,20 +143,20 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
 		rs.Time = startTime
 	}
 
-	shimSetupCtx, shimSetupCancel := context.WithDeadline(ctx, time.Now().Add(shimSetupTimeout))
+	c, cancel := context.WithTimeout(ctx, shimSetupTimeout)
-	defer shimSetupCancel()
+	defer cancel()
 
 	go func() {
 		waitErr <- cmd.Unwrap().Wait()
 		// cancel shim setup in case shim died before receiving payload
-		shimSetupCancel()
+		cancel()
 	}()
 
-	if err := cmd.Serve(shimSetupCtx, &shim.Payload{
+	if err := cmd.Serve(c, &shim.Payload{
-		Argv:  a.appSeal.command,
+		Argv:  seal.command,
 		Exec:  shimExec,
-		Bwrap: a.appSeal.container,
+		Bwrap: seal.container,
-		Home:  a.appSeal.user.data,
+		Home:  seal.user.data,
 
 		Verbose: fmsg.Load(),
 	}); err != nil {
@@ -161,14 +165,14 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
 
 	// shim accepted setup payload, create process state
 	sd := state.State{
-		ID:   a.id.unwrap(),
+		ID:   seal.id.unwrap(),
 		PID:  cmd.Unwrap().Process.Pid,
 		Time: *rs.Time,
 	}
 	var earlyStoreErr = new(StateStoreError) // returned after blocking on waitErr
-	earlyStoreErr.Inner, earlyStoreErr.DoErr = store.Do(a.appSeal.user.aid.unwrap(), func(c state.Cursor) { earlyStoreErr.InnerErr = c.Save(&sd, a.appSeal.ct) })
+	earlyStoreErr.Inner, earlyStoreErr.DoErr = store.Do(seal.user.aid.unwrap(), func(c state.Cursor) { earlyStoreErr.InnerErr = c.Save(&sd, seal.ct) })
 	// destroy defunct state entry
-	deferredStoreFunc = func(c state.Cursor) error { return c.Destroy(a.id.unwrap()) }
+	deferredStoreFunc = func(c state.Cursor) error { return c.Destroy(seal.id.unwrap()) }
 
 	select {
 	case err := <-waitErr: // block until fsu/shim returns
@@ -201,9 +205,9 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error {
 	}
 
 	fmsg.Resume()
-	if a.appSeal.dbusMsg != nil {
+	if seal.dbusMsg != nil {
 		// dump dbus message buffer
-		a.appSeal.dbusMsg()
+		seal.dbusMsg()
 	}
 
 	return earlyStoreErr.equiv("cannot save process state:")

internal/app/seal.go

@@ -11,6 +11,7 @@ import (
 	"path"
 	"regexp"
 	"strings"
+	"sync/atomic"
 
 	"git.gensokyo.uk/security/fortify/acl"
 	"git.gensokyo.uk/security/fortify/dbus"
@@ -57,8 +58,13 @@ var (
 
 var posixUsername = regexp.MustCompilePOSIX("^[a-z_]([A-Za-z0-9_-]{0,31}|[A-Za-z0-9_-]{0,30}\\$)$")
 
-// appSeal stores copies of various parts of [fst.Config]
+// outcome stores copies of various parts of [fst.Config]
-type appSeal struct {
+type outcome struct {
+	// copied from initialising [app]
+	id *stringPair[fst.ID]
+	// copied from [sys.State] response
+	runDirPath string
+
 	// passed through from [fst.Config]
 	command []string
 
@@ -68,16 +74,16 @@ type appSeal struct {
 	// dump dbus proxy message buffer
 	dbusMsg func()
 
-	user      appUser
+	user      fsuUser
 	sys       *system.I
 	container *bwrap.Config
 	bwrapSync *os.File
 
-	// protected by upstream mutex
+	f atomic.Bool
 }
 
-// appUser stores post-fsu credentials and metadata
+// fsuUser stores post-fsu credentials and metadata
-type appUser struct {
+type fsuUser struct {
 	// application id
 	aid *stringPair[int]
 	// target uid resolved by fid:aid
@@ -94,7 +100,7 @@ type appUser struct {
 	username string
 }
 
-func (seal *appSeal) finalise(sys sys.State, config *fst.Config, id string) error {
+func (seal *outcome) finalise(sys sys.State, config *fst.Config) error {
 	{
 		// encode initial configuration for state tracking
 		ct := new(bytes.Buffer)
@@ -118,7 +124,7 @@ func (seal *appSeal) finalise(sys sys.State, config *fst.Config, id string) erro
 		Resolve post-fsu user state
 	*/
 
-	seal.user = appUser{
+	seal.user = fsuUser{
 		aid:      newInt(config.Confinement.AppID),
 		data:     config.Confinement.Outer,
 		home:     config.Confinement.Inner,
@@ -223,6 +229,7 @@ func (seal *appSeal) finalise(sys sys.State, config *fst.Config, id string) erro
 	*/
 
 	sc := sys.Paths()
+	seal.runDirPath = sc.RunDirPath
 	seal.sys = system.New(seal.user.uid.unwrap())
 	seal.sys.IsVerbose = fmsg.Load
 	seal.sys.Verbose = fmsg.Verbose
@@ -243,10 +250,10 @@ func (seal *appSeal) finalise(sys sys.State, config *fst.Config, id string) erro
 	seal.sys.UpdatePermType(system.User, sc.RuntimePath, acl.Execute)
 
 	// outer process-specific share directory
-	sharePath := path.Join(sc.SharePath, id)
+	sharePath := path.Join(sc.SharePath, seal.id.String())
 	seal.sys.Ephemeral(system.Process, sharePath, 0711)
 	// similar to share but within XDG_RUNTIME_DIR
-	sharePathLocal := path.Join(sc.RunDirPath, id)
+	sharePathLocal := path.Join(sc.RunDirPath, seal.id.String())
 	seal.sys.Ephemeral(system.Process, sharePathLocal, 0700)
 	seal.sys.UpdatePerm(sharePathLocal, acl.Execute)
 
@@ -327,14 +334,14 @@ func (seal *appSeal) finalise(sys sys.State, config *fst.Config, id string) erro
 
 		if !config.Confinement.Sandbox.DirectWayland { // set up security-context-v1
 			socketDir := path.Join(sc.SharePath, "wayland")
-			outerPath := path.Join(socketDir, id)
+			outerPath := path.Join(socketDir, seal.id.String())
 			seal.sys.Ensure(socketDir, 0711)
 			appID := config.ID
 			if appID == "" {
 				// use instance ID in case app id is not set
-				appID = "uk.gensokyo.fortify." + id
+				appID = "uk.gensokyo.fortify." + seal.id.String()
 			}
-			seal.sys.Wayland(&seal.bwrapSync, outerPath, socketPath, appID, id)
+			seal.sys.Wayland(&seal.bwrapSync, outerPath, socketPath, appID, seal.id.String())
 			seal.container.Bind(outerPath, innerPath)
 		} else { // bind mount wayland socket (insecure)
 			fmsg.Verbose("direct wayland access, PROCEED WITH CAUTION")

main.go

@@ -173,7 +173,7 @@ func main() {
 		config.Command = append(config.Command, args[2:]...)
 
 		// invoke app
-		runApp(config)
+		runApp(app.MustNew(std), config)
 		panic("unreachable")
 
 	case "run": // run app in permissive defaults usage pattern
@@ -300,7 +300,7 @@ func main() {
 		}
 
 		// invoke app
-		runApp(config)
+		runApp(app.MustNew(std), config)
 		panic("unreachable")
 
 	// internal commands
@@ -318,7 +318,7 @@ func main() {
 	panic("unreachable")
 }
 
-func runApp(config *fst.Config) {
+func runApp(a fst.App, config *fst.Config) {
 	rs := new(fst.RunState)
 	ctx, stop := signal.NotifyContext(context.Background(),
 		syscall.SIGINT, syscall.SIGTERM)
@@ -328,12 +328,10 @@ func runApp(config *fst.Config) {
 		seccomp.CPrintln = log.Println
 	}
 
-	if a, err := app.New(std); err != nil {
+	if sa, err := a.Seal(config); err != nil {
-		log.Fatalf("cannot create app: %s", err)
-	} else if err = a.Seal(config); err != nil {
 		fmsg.PrintBaseError(err, "cannot seal app:")
 		internal.Exit(1)
-	} else if err = a.Run(ctx, rs); err != nil {
+	} else if err = sa.Run(ctx, rs); err != nil {
 		if rs.Time == nil {
 			fmsg.PrintBaseError(err, "cannot start app:")
 		} else {