From eae3034260b3abbb8da1deeddfef135b36eeb390 Mon Sep 17 00:00:00 2001 From: Ophestra Umiker Date: Thu, 19 Dec 2024 21:36:17 +0900 Subject: [PATCH] state: expose aids and use instance id as key Fortify state store instances was specific to aids due to outdated design decisions carried over from the ego rewrite. That no longer makes sense in the current application, so the interface now enables a single store object to manage all transient state. Signed-off-by: Ophestra Umiker --- internal/app/seal.go | 2 +- internal/app/start.go | 8 +- internal/state/multi.go | 284 ++++++++++++++++++++++------------- internal/state/multi_test.go | 11 ++ internal/state/print.go | 55 +++---- internal/state/state.go | 18 ++- internal/state/state_test.go | 126 ++++++++++++++++ test.nix | 3 + 8 files changed, 353 insertions(+), 154 deletions(-) create mode 100644 internal/state/multi_test.go create mode 100644 internal/state/state_test.go diff --git a/internal/app/seal.go b/internal/app/seal.go index d71553a..3c49584 100644 --- a/internal/app/seal.go +++ b/internal/app/seal.go @@ -218,7 +218,7 @@ func (a *app) Seal(config *fst.Config) error { // open process state store // the simple store only starts holding an open file after first action // store activity begins after Start is called and must end before Wait - seal.store = state.NewSimple(seal.RunDirPath, seal.sys.user.as) + seal.store = state.NewMulti(seal.RunDirPath) // initialise system interface with full uid seal.sys.I = system.New(seal.sys.user.uid) diff --git a/internal/app/start.go b/internal/app/start.go index 1ebc94f..8de7821 100644 --- a/internal/app/start.go +++ b/internal/app/start.go @@ -76,8 +76,8 @@ func (a *app) Start() error { // register process state var err0 = new(StateStoreError) - err0.Inner, err0.DoErr = a.seal.store.Do(func(b state.Backend) { - err0.InnerErr = b.Save(&sd) + err0.Inner, err0.DoErr = a.seal.store.Do(a.seal.sys.user.aid, func(c state.Cursor) { + err0.InnerErr = c.Save(&sd) }) a.seal.sys.saveState = true return err0.equiv("cannot save process state:") @@ -199,11 +199,11 @@ func (a *app) Wait() (int, error) { // update store and revert app setup transaction e := new(StateStoreError) - e.Inner, e.DoErr = a.seal.store.Do(func(b state.Backend) { + e.Inner, e.DoErr = a.seal.store.Do(a.seal.sys.user.aid, func(b state.Cursor) { e.InnerErr = func() error { // destroy defunct state entry if cmd := a.shim.Unwrap(); cmd != nil && a.seal.sys.saveState { - if err := b.Destroy(cmd.Process.Pid); err != nil { + if err := b.Destroy(*a.id); err != nil { return err } } diff --git a/internal/state/multi.go b/internal/state/multi.go index f014b3d..16afc08 100644 --- a/internal/state/multi.go +++ b/internal/state/multi.go @@ -3,54 +3,135 @@ package state import ( "encoding/gob" "errors" + "fmt" "io/fs" "os" "path" "strconv" "sync" "syscall" + + "git.ophivana.moe/security/fortify/fst" + "git.ophivana.moe/security/fortify/internal/fmsg" ) // fine-grained locking and access type multiStore struct { - path []string + base string - // created/opened by prepare - lockfile *os.File - // enforce prepare method - init sync.Once - // error returned by prepare - initErr error + // initialised backends + backends *sync.Map - lock sync.Mutex + lock sync.RWMutex } -func (s *multiStore) Do(f func(b Backend)) (bool, error) { - s.init.Do(s.prepare) - if s.initErr != nil { - return false, s.initErr +func (s *multiStore) Do(aid int, f func(c Cursor)) (bool, error) { + s.lock.RLock() + defer s.lock.RUnlock() + + // load or initialise new backend + b := new(multiBackend) + if v, ok := s.backends.LoadOrStore(aid, b); ok { + b = v.(*multiBackend) + } else { + b.lock.Lock() + b.path = path.Join(s.base, strconv.Itoa(aid)) + + // ensure directory + if err := os.MkdirAll(b.path, 0700); err != nil && !errors.Is(err, fs.ErrExist) { + s.backends.CompareAndDelete(aid, b) + return false, err + } + + // open locker file + if l, err := os.OpenFile(b.path+".lock", os.O_RDWR|os.O_CREATE, 0600); err != nil { + s.backends.CompareAndDelete(aid, b) + return false, err + } else { + b.lockfile = l + } + b.lock.Unlock() } - s.lock.Lock() - defer s.lock.Unlock() - - // lock store - if err := s.lockFile(); err != nil { + // lock backend + if err := b.lockFile(); err != nil { return false, err } - // initialise new backend for caller - b := new(multiBackend) - b.path = path.Join(s.path...) + // expose backend methods without exporting the pointer + c := new(struct{ *multiBackend }) + c.multiBackend = b f(b) - // disable backend - b.lock.Lock() + // disable access to the backend on a best-effort basis + c.multiBackend = nil - // unlock store - return true, s.unlockFile() + // unlock backend + return true, b.unlockFile() } -func (s *multiStore) lockFileAct(lt int) (err error) { +func (s *multiStore) List() ([]int, error) { + var entries []os.DirEntry + + // read base directory to get all aids + if v, err := os.ReadDir(s.base); err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, err + } else { + entries = v + } + + aidsBuf := make([]int, 0, len(entries)) + for _, e := range entries { + // skip non-directories + if !e.IsDir() { + fmsg.VPrintf("skipped non-directory entry %q", e.Name()) + continue + } + + // skip non-numerical names + if v, err := strconv.Atoi(e.Name()); err != nil { + fmsg.VPrintf("skipped non-aid entry %q", e.Name()) + continue + } else { + if v < 0 || v > 9999 { + fmsg.VPrintf("skipped out of bounds entry %q", e.Name()) + continue + } + + aidsBuf = append(aidsBuf, v) + } + } + + return append([]int(nil), aidsBuf...), nil +} + +func (s *multiStore) Close() error { + s.lock.Lock() + defer s.lock.Unlock() + + var errs []error + s.backends.Range(func(_, value any) bool { + b := value.(*multiBackend) + errs = append(errs, b.close()) + return true + }) + + return errors.Join(errs...) +} + +type multiBackend struct { + path string + + // created/opened by prepare + lockfile *os.File + + lock sync.RWMutex +} + +func (b *multiBackend) filename(id *fst.ID) string { + return path.Join(b.path, id.String()) +} + +func (b *multiBackend) lockFileAct(lt int) (err error) { op := "LockAct" switch lt { case syscall.LOCK_EX: @@ -60,7 +141,7 @@ func (s *multiStore) lockFileAct(lt int) (err error) { } for { - err = syscall.Flock(int(s.lockfile.Fd()), lt) + err = syscall.Flock(int(b.lockfile.Fd()), lt) if !errors.Is(err, syscall.EINTR) { break } @@ -68,103 +149,80 @@ func (s *multiStore) lockFileAct(lt int) (err error) { if err != nil { return &fs.PathError{ Op: op, - Path: s.lockfile.Name(), + Path: b.lockfile.Name(), Err: err, } } return nil } -func (s *multiStore) lockFile() error { - return s.lockFileAct(syscall.LOCK_EX) +func (b *multiBackend) lockFile() error { + return b.lockFileAct(syscall.LOCK_EX) } -func (s *multiStore) unlockFile() error { - return s.lockFileAct(syscall.LOCK_UN) -} - -func (s *multiStore) prepare() { - s.initErr = func() error { - prefix := path.Join(s.path...) - // ensure directory - if err := os.MkdirAll(prefix, 0700); err != nil && !errors.Is(err, fs.ErrExist) { - return err - } - - // open locker file - if f, err := os.OpenFile(prefix+".lock", os.O_RDWR|os.O_CREATE, 0600); err != nil { - return err - } else { - s.lockfile = f - } - - return nil - }() -} - -func (s *multiStore) Close() error { - s.lock.Lock() - defer s.lock.Unlock() - - err := s.lockfile.Close() - if err == nil || errors.Is(err, os.ErrInvalid) || errors.Is(err, os.ErrClosed) { - return nil - } - return err -} - -type multiBackend struct { - path string - - lock sync.RWMutex -} - -func (b *multiBackend) filename(pid int) string { - return path.Join(b.path, strconv.Itoa(pid)) +func (b *multiBackend) unlockFile() error { + return b.lockFileAct(syscall.LOCK_UN) } // reads all launchers in simpleBackend // file contents are ignored if decode is false -func (b *multiBackend) load(decode bool) ([]*State, error) { +func (b *multiBackend) load(decode bool) (Entries, error) { b.lock.RLock() defer b.lock.RUnlock() - var ( - r []*State - f *os.File - ) - - // read directory contents, should only contain files named after PIDs + // read directory contents, should only contain files named after ids + var entries []os.DirEntry if pl, err := os.ReadDir(b.path); err != nil { return nil, err } else { - for _, e := range pl { - // run in a function to better handle file closing - if err = func() error { - // open state file for reading - if f, err = os.Open(path.Join(b.path, e.Name())); err != nil { - return err - } else { - defer func() { - if f.Close() != nil { - // unreachable - panic("foreign state file closed prematurely") - } - }() + entries = pl + } - var s State - r = append(r, &s) + // allocate as if every entry is valid + // since that should be the case assuming no external interference happens + r := make(Entries, len(entries)) - // append regardless, but only parse if required, used to implement Len - if decode { - return gob.NewDecoder(f).Decode(&s) - } else { - return nil + for _, e := range entries { + if e.IsDir() { + return nil, fmt.Errorf("unexpected directory %q in store", e.Name()) + } + + id := new(fst.ID) + if err := fst.ParseAppID(id, e.Name()); err != nil { + return nil, err + } + + // run in a function to better handle file closing + if err := func() error { + // open state file for reading + if f, err := os.Open(path.Join(b.path, e.Name())); err != nil { + return err + } else { + defer func() { + if f.Close() != nil { + // unreachable + panic("foreign state file closed prematurely") + } + }() + + s := new(State) + r[*id] = s + + // append regardless, but only parse if required, used to implement Len + if decode { + if err = gob.NewDecoder(f).Decode(s); err != nil { + return err + } + + if s.ID != *id { + return fmt.Errorf("state entry %s has unexpected id %s", id, &s.ID) } } - }(); err != nil { - return nil, err + + return nil } + }(); err != nil { + return nil, err } } @@ -180,7 +238,7 @@ func (b *multiBackend) Save(state *State) error { return errors.New("state does not contain config") } - statePath := b.filename(state.PID) + statePath := b.filename(&state.ID) // create and open state data file if f, err := os.OpenFile(statePath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0600); err != nil { @@ -197,14 +255,14 @@ func (b *multiBackend) Save(state *State) error { } } -func (b *multiBackend) Destroy(pid int) error { +func (b *multiBackend) Destroy(id fst.ID) error { b.lock.Lock() defer b.lock.Unlock() - return os.Remove(b.filename(pid)) + return os.Remove(b.filename(&id)) } -func (b *multiBackend) Load() ([]*State, error) { +func (b *multiBackend) Load() (Entries, error) { return b.load(true) } @@ -214,9 +272,21 @@ func (b *multiBackend) Len() (int, error) { return len(rn), err } -// NewSimple returns an instance of a file-based store. -func NewSimple(runDir string, prefix ...string) Store { +func (b *multiBackend) close() error { + b.lock.Lock() + defer b.lock.Unlock() + + err := b.lockfile.Close() + if err == nil || errors.Is(err, os.ErrInvalid) || errors.Is(err, os.ErrClosed) { + return nil + } + return err +} + +// NewMulti returns an instance of the multi-file store. +func NewMulti(runDir string) Store { b := new(multiStore) - b.path = append([]string{runDir, "state"}, prefix...) + b.base = path.Join(runDir, "state") + b.backends = new(sync.Map) return b } diff --git a/internal/state/multi_test.go b/internal/state/multi_test.go new file mode 100644 index 0000000..13b2937 --- /dev/null +++ b/internal/state/multi_test.go @@ -0,0 +1,11 @@ +package state_test + +import ( + "testing" + + "git.ophivana.moe/security/fortify/internal/state" +) + +func TestMulti(t *testing.T) { + testStore(t, state.NewMulti(t.TempDir())) +} diff --git a/internal/state/print.go b/internal/state/print.go index 485fd67..2309bd6 100644 --- a/internal/state/print.go +++ b/internal/state/print.go @@ -1,11 +1,8 @@ package state import ( - "errors" "fmt" "os" - "path" - "strconv" "strings" "text/tabwriter" "time" @@ -18,45 +15,31 @@ import ( // in an implementation-specific way. func MustPrintLauncherStateSimpleGlobal(w **tabwriter.Writer, runDir string) { now := time.Now().UTC() + s := NewMulti(runDir) // read runtime directory to get all UIDs - if dirs, err := os.ReadDir(path.Join(runDir, "state")); err != nil && !errors.Is(err, os.ErrNotExist) { - fmsg.Fatal("cannot read runtime directory:", err) + if aids, err := s.List(); err != nil { + fmsg.Fatal("cannot list store:", err) } else { - for _, e := range dirs { - // skip non-directories - if !e.IsDir() { - fmsg.VPrintf("skipped non-directory entry %q", e.Name()) - continue - } - - // skip non-numerical names - if _, err = strconv.Atoi(e.Name()); err != nil { - fmsg.VPrintf("skipped non-uid entry %q", e.Name()) - continue - } - - // obtain temporary store - s := NewSimple(runDir, e.Name()).(*multiStore) - + for _, aid := range aids { // print states belonging to this store - s.mustPrintLauncherState(w, now) - - // mustPrintLauncherState causes store activity so store needs to be closed - if err = s.Close(); err != nil { - fmsg.Printf("cannot close store for user %q: %s", e.Name(), err) - } + s.(*multiStore).mustPrintLauncherState(aid, w, now) } } + + // mustPrintLauncherState causes store activity so store needs to be closed + if err := s.Close(); err != nil { + fmsg.Printf("cannot close store: %v", err) + } } -func (s *multiStore) mustPrintLauncherState(w **tabwriter.Writer, now time.Time) { +func (s *multiStore) mustPrintLauncherState(aid int, w **tabwriter.Writer, now time.Time) { var innerErr error - if ok, err := s.Do(func(b Backend) { + if ok, err := s.Do(aid, func(c Cursor) { innerErr = func() error { // read launcher states - states, err := b.Load() + states, err := c.Load() if err != nil { return err } @@ -111,25 +94,25 @@ func (s *multiStore) mustPrintLauncherState(w **tabwriter.Writer, now time.Time) } if !fmsg.Verbose() { - _, _ = fmt.Fprintf(*w, "\t%d\t%s\t%s\t%s\t%s\n", - state.PID, s.path[len(s.path)-1], now.Sub(state.Time).Round(time.Second).String(), strings.TrimPrefix(ets.String(), ", "), cs) + _, _ = fmt.Fprintf(*w, "\t%d\t%d\t%s\t%s\t%s\n", + state.PID, aid, now.Sub(state.Time).Round(time.Second).String(), strings.TrimPrefix(ets.String(), ", "), cs) } else { // emit argv instead when verbose - _, _ = fmt.Fprintf(*w, "\t%d\t%s\t%s\n", - state.PID, s.path[len(s.path)-1], state.ID) + _, _ = fmt.Fprintf(*w, "\t%d\t%d\t%s\n", + state.PID, aid, state.ID) } } return nil }() }); err != nil { - fmsg.Printf("cannot perform action on store %q: %s", path.Join(s.path...), err) + fmsg.Printf("cannot perform action on app %d: %v", aid, err) if !ok { fmsg.Fatal("store faulted before printing") } } if innerErr != nil { - fmsg.Fatalf("cannot print launcher state for store %q: %s", path.Join(s.path...), innerErr) + fmsg.Fatalf("cannot print launcher state of app %d: %s", aid, innerErr) } } diff --git a/internal/state/state.go b/internal/state/state.go index cf345aa..ccc44b1 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -6,21 +6,27 @@ import ( "git.ophivana.moe/security/fortify/fst" ) +type Entries map[fst.ID]*State + type Store interface { // Do calls f exactly once and ensures store exclusivity until f returns. // Returns whether f is called and any errors during the locking process. - // Backend provided to f becomes invalid as soon as f returns. - Do(f func(b Backend)) (bool, error) + // Cursor provided to f becomes invalid as soon as f returns. + Do(aid int, f func(c Cursor)) (ok bool, err error) + + // List queries the store and returns a list of aids known to the store. + // Note that some or all returned aids might not have any active apps. + List() (aids []int, err error) // Close releases any resources held by Store. Close() error } -// Backend provides access to the store -type Backend interface { +// Cursor provides access to the store +type Cursor interface { Save(state *State) error - Destroy(pid int) error - Load() ([]*State, error) + Destroy(id fst.ID) error + Load() (Entries, error) Len() (int, error) } diff --git a/internal/state/state_test.go b/internal/state/state_test.go new file mode 100644 index 0000000..0a4ba57 --- /dev/null +++ b/internal/state/state_test.go @@ -0,0 +1,126 @@ +package state_test + +import ( + "math/rand/v2" + "reflect" + "slices" + "testing" + "time" + + "git.ophivana.moe/security/fortify/fst" + "git.ophivana.moe/security/fortify/internal/state" +) + +func testStore(t *testing.T, s state.Store) { + t.Run("list empty store", func(t *testing.T) { + if aids, err := s.List(); err != nil { + t.Fatalf("List: error = %v", err) + } else if len(aids) != 0 { + t.Fatalf("List: aids = %#v", aids) + } + }) + + const ( + insertEntryChecked = iota + insertEntryNoCheck + insertEntryOtherApp + + tl + ) + + var tc [tl]state.State + for i := 0; i < tl; i++ { + makeState(t, &tc[i]) + } + + do := func(aid int, f func(c state.Cursor)) { + if ok, err := s.Do(aid, f); err != nil { + t.Fatalf("Do: ok = %v, error = %v", ok, err) + } + } + + insert := func(i, aid int) { + do(aid, func(c state.Cursor) { + if err := c.Save(&tc[i]); err != nil { + t.Fatalf("Save(&tc[%v]): error = %v", i, err) + } + }) + } + + check := func(i, aid int) { + do(aid, func(c state.Cursor) { + if entries, err := c.Load(); err != nil { + t.Fatalf("Load: error = %v", err) + } else if got, ok := entries[tc[i].ID]; !ok { + t.Fatalf("Load: entry %s missing", + &tc[i].ID) + } else { + got.Time = tc[i].Time + if !reflect.DeepEqual(got, &tc[i]) { + t.Fatalf("Load: entry %s got %#v, want %#v", + &tc[i].ID, got, &tc[i]) + } + } + }) + } + + t.Run("insert entry checked", func(t *testing.T) { + insert(insertEntryChecked, 0) + check(insertEntryChecked, 0) + }) + + t.Run("insert entry unchecked", func(t *testing.T) { + insert(insertEntryNoCheck, 0) + }) + + t.Run("insert entry different aid", func(t *testing.T) { + insert(insertEntryOtherApp, 1) + check(insertEntryOtherApp, 1) + }) + + t.Run("check previous insertion", func(t *testing.T) { + check(insertEntryNoCheck, 0) + }) + + t.Run("list aids", func(t *testing.T) { + if aids, err := s.List(); err != nil { + t.Fatalf("List: error = %v", err) + } else { + slices.Sort(aids) + want := []int{0, 1} + if slices.Compare(aids, want) != 0 { + t.Fatalf("List() = %#v, want %#v", aids, want) + } + } + }) + + t.Run("clear aid 1", func(t *testing.T) { + do(1, func(c state.Cursor) { + if err := c.Destroy(tc[insertEntryOtherApp].ID); err != nil { + t.Fatalf("Destroy: error = %v", err) + } + }) + do(1, func(c state.Cursor) { + if l, err := c.Len(); err != nil { + t.Fatalf("Len: error = %v", err) + } else if l != 0 { + t.Fatalf("Len() = %d, want 0", l) + } + }) + }) + + t.Run("close store", func(t *testing.T) { + if err := s.Close(); err != nil { + t.Fatalf("Close: error = %v", err) + } + }) +} + +func makeState(t *testing.T, s *state.State) { + if err := fst.NewAppID(&s.ID); err != nil { + t.Fatalf("cannot create dummy state: %v", err) + } + s.Config = fst.Template() + s.PID = rand.Int() + s.Time = time.Now() +} diff --git a/test.nix b/test.nix index 00bcba4..12384bc 100644 --- a/test.nix +++ b/test.nix @@ -214,5 +214,8 @@ nixosTest { swaymsg("exit", succeed=False) machine.wait_until_fails("pgrep -x sway") machine.wait_for_file("/tmp/sway-exit-ok") + + # Print fortify runDir contents: + print(machine.succeed("find /run/user/1000/fortify")) ''; }