From c64b8163e79d47f1e710c1d067589bb393f8463c Mon Sep 17 00:00:00 2001 From: Ophestra Date: Fri, 21 Feb 2025 16:00:31 +0900 Subject: [PATCH] app: separate instance from process state This works better for the implementation. Signed-off-by: Ophestra --- fst/app.go | 16 +++++++---- internal/app/app.go | 34 ++++++++++++++-------- internal/app/app_test.go | 12 +++++--- internal/app/export_test.go | 8 ++++-- internal/app/process.go | 56 ++++++++++++++++++++----------------- internal/app/seal.go | 33 +++++++++++++--------- main.go | 12 ++++---- 7 files changed, 102 insertions(+), 69 deletions(-) diff --git a/fst/app.go b/fst/app.go index e1a17f1..3addb46 100644 --- a/fst/app.go +++ b/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. diff --git a/internal/app/app.go b/internal/app/app.go index be59a7d..c4668a0 100644 --- a/internal/app/app.go +++ b/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.appSeal.user.uid == nil { + if a.outcome != 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) - err = seal.finalise(a.sys, config, a.id.String()) + seal := new(outcome) + seal.id = a.id + err := seal.finalise(a.sys, config) if err == nil { - a.appSeal = seal + a.outcome = seal } - return + return seal, err } diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 6bdc043..b448153 100644 --- a/internal/app/app_test.go +++ b/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", diff --git a/internal/app/export_test.go b/internal/app/export_test.go index af52912..199b6c1 100644 --- a/internal/app/export_test.go +++ b/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 } diff --git a/internal/app/process.go b/internal/app/process.go index c401eb3..1302d81 100644 --- a/internal/app/process.go +++ b/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 { - a.mu.Lock() - defer a.mu.Unlock() +func (seal *outcome) Run(ctx context.Context, rs *fst.RunState) error { + if !seal.f.CompareAndSwap(false, true) { + // 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 { - shimExec[1] = a.appSeal.command[0] + if len(seal.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(), - a.appSeal.user.supp, - a.appSeal.bwrapSync, + seal.user.aid.String(), + seal.user.supp, + 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)) - defer shimSetupCancel() + c, cancel := context.WithTimeout(ctx, shimSetupTimeout) + 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{ - Argv: a.appSeal.command, + if err := cmd.Serve(c, &shim.Payload{ + Argv: seal.command, Exec: shimExec, - Bwrap: a.appSeal.container, - Home: a.appSeal.user.data, + Bwrap: seal.container, + 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:") diff --git a/internal/app/seal.go b/internal/app/seal.go index d39ee12..b133b46 100644 --- a/internal/app/seal.go +++ b/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] -type appSeal struct { +// outcome stores copies of various parts of [fst.Config] +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 -type appUser struct { +// fsuUser stores post-fsu credentials and metadata +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") diff --git a/main.go b/main.go index 7896a19..067071a 100644 --- a/main.go +++ b/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 { - log.Fatalf("cannot create app: %s", err) - } else if err = a.Seal(config); err != nil { + if sa, 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 {