From a748d40745611b471ee2a8dd79656aaa174eb03c Mon Sep 17 00:00:00 2001 From: Ophestra Date: Wed, 19 Feb 2025 00:25:00 +0900 Subject: [PATCH] app: store values with string representation Improves code readability without changing memory layout. Signed-off-by: Ophestra --- internal/app/app.go | 24 +++++++++++++++--------- internal/app/export_test.go | 2 +- internal/app/seal.go | 30 +++++++++++++++--------------- internal/app/share.go | 8 ++++---- internal/app/start.go | 10 +++++----- internal/app/strings.go | 19 +++++++++++++++++++ internal/app/system.go | 17 +++++------------ 7 files changed, 64 insertions(+), 46 deletions(-) create mode 100644 internal/app/strings.go diff --git a/internal/app/app.go b/internal/app/app.go index cf21ac7..aa798af 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -1,6 +1,7 @@ package app import ( + "fmt" "sync" "git.gensokyo.uk/security/fortify/fst" @@ -10,14 +11,18 @@ import ( func New(os sys.State) (fst.App, error) { a := new(app) - a.id = new(fst.ID) a.os = os - return a, fst.NewAppID(a.id) + + id := new(fst.ID) + err := fst.NewAppID(id) + a.id = newID(id) + + return a, err } type app struct { // application unique identifier - id *fst.ID + id *stringPair[fst.ID] // operating system interface os sys.State // shim process manager @@ -28,13 +33,11 @@ type app struct { lock sync.RWMutex } -func (a *app) ID() fst.ID { - return *a.id -} +func (a *app) ID() fst.ID { return a.id.unwrap() } func (a *app) String() string { if a == nil { - return "(invalid fortified app)" + return "(invalid app)" } a.lock.RLock() @@ -45,8 +48,11 @@ func (a *app) String() string { } if a.seal != nil { - return "(sealed fortified app as uid " + a.seal.sys.user.us + ")" + if a.seal.sys.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.seal.sys.user.uid) } - return "(unsealed fortified app)" + return fmt.Sprintf("(unsealed app %s)", a.id) } diff --git a/internal/app/export_test.go b/internal/app/export_test.go index 836e04c..1dd1aa1 100644 --- a/internal/app/export_test.go +++ b/internal/app/export_test.go @@ -9,7 +9,7 @@ import ( func NewWithID(id fst.ID, os sys.State) fst.App { a := new(app) - a.id = &id + a.id = newID(&id) a.os = os return a } diff --git a/internal/app/seal.go b/internal/app/seal.go index 12891b2..35a025c 100644 --- a/internal/app/seal.go +++ b/internal/app/seal.go @@ -9,7 +9,6 @@ import ( "io/fs" "path" "regexp" - "strconv" "git.gensokyo.uk/security/fortify/acl" "git.gensokyo.uk/security/fortify/dbus" @@ -110,14 +109,17 @@ func (a *app) Seal(config *fst.Config) error { // create seal system component seal.sys = new(appSealSys) - // mapped uid - if config.Confinement.Sandbox != nil && config.Confinement.Sandbox.MapRealUID { - seal.sys.mappedID = a.os.Geteuid() - } else { - seal.sys.mappedID = 65534 + { + // mapped uid defaults to 65534 to work around file ownership checks due to a bwrap limitation + mapuid := 65534 + if config.Confinement.Sandbox != nil && config.Confinement.Sandbox.MapRealUID { + // some programs fail to connect to dbus session running as a different uid, so a + // separate workaround is introduced to map priv-side caller uid in namespace + mapuid = a.os.Geteuid() + } + seal.sys.mapuid = newInt(mapuid) + seal.sys.runtime = path.Join("/run/user", seal.sys.mapuid.String()) } - seal.sys.mappedIDString = strconv.Itoa(seal.sys.mappedID) - seal.sys.runtime = path.Join("/run/user", seal.sys.mappedIDString) // validate uid and set user info if config.Confinement.AppID < 0 || config.Confinement.AppID > 9999 { @@ -125,8 +127,7 @@ func (a *app) Seal(config *fst.Config) error { fmt.Sprintf("aid %d out of range", config.Confinement.AppID)) } seal.sys.user = appUser{ - aid: config.Confinement.AppID, - as: strconv.Itoa(config.Confinement.AppID), + aid: newInt(config.Confinement.AppID), data: config.Confinement.Outer, home: config.Confinement.Inner, username: config.Confinement.Username, @@ -147,11 +148,10 @@ func (a *app) Seal(config *fst.Config) error { } // invoke fsu for full uid - if u, err := a.os.Uid(seal.sys.user.aid); err != nil { + if u, err := a.os.Uid(seal.sys.user.aid.unwrap()); err != nil { return err } else { - seal.sys.user.uid = u - seal.sys.user.us = strconv.Itoa(u) + seal.sys.user.uid = newInt(u) } // resolve supplementary group ids from names @@ -251,7 +251,7 @@ func (a *app) Seal(config *fst.Config) error { seal.store = state.NewMulti(seal.RunDirPath) // initialise system interface with os uid - seal.sys.I = system.New(seal.sys.user.uid) + seal.sys.I = system.New(seal.sys.user.uid.unwrap()) seal.sys.I.IsVerbose = fmsg.Load seal.sys.I.Verbose = fmsg.Verbose seal.sys.I.Verbosef = fmsg.Verbosef @@ -267,7 +267,7 @@ func (a *app) Seal(config *fst.Config) error { // verbose log seal information fmsg.Verbosef("created application seal for uid %s (%s) groups: %v, command: %s", - seal.sys.user.us, seal.sys.user.username, config.Confinement.Groups, config.Command) + seal.sys.user.uid, seal.sys.user.username, config.Confinement.Groups, config.Command) // seal app and release lock a.seal = seal diff --git a/internal/app/share.go b/internal/app/share.go index bb244c8..4fd7746 100644 --- a/internal/app/share.go +++ b/internal/app/share.go @@ -68,7 +68,7 @@ func (seal *appSeal) setupShares(bus [2]*dbus.Config, os sys.State) error { seal.sys.UpdatePermType(system.User, targetTmpdirParent, acl.Execute) // ensure child tmpdir (e.g. `/tmp/fortify.%d/tmpdir/%d`) - targetTmpdir := path.Join(targetTmpdirParent, seal.sys.user.as) + targetTmpdir := path.Join(targetTmpdirParent, seal.sys.user.aid.String()) seal.sys.Ensure(targetTmpdir, 01700) seal.sys.UpdatePermType(system.User, targetTmpdir, acl.Read, acl.Write, acl.Execute) seal.sys.bwrap.Bind(targetTmpdir, "/tmp", false, true) @@ -126,9 +126,9 @@ func (seal *appSeal) setupShares(bus [2]*dbus.Config, os sys.State) error { // generate /etc/passwd and /etc/group seal.sys.bwrap.CopyBind("/etc/passwd", - []byte(username+":x:"+seal.sys.mappedIDString+":"+seal.sys.mappedIDString+":Fortify:"+homeDir+":"+sh+"\n")) + []byte(username+":x:"+seal.sys.mapuid.String()+":"+seal.sys.mapuid.String()+":Fortify:"+homeDir+":"+sh+"\n")) seal.sys.bwrap.CopyBind("/etc/group", - []byte("fortify:x:"+seal.sys.mappedIDString+":\n")) + []byte("fortify:x:"+seal.sys.mapuid.String()+":\n")) /* Display servers @@ -181,7 +181,7 @@ func (seal *appSeal) setupShares(bus [2]*dbus.Config, os sys.State) error { return fmsg.WrapError(ErrXDisplay, "DISPLAY is not set") } else { - seal.sys.ChangeHosts("#" + seal.sys.user.us) + seal.sys.ChangeHosts("#" + seal.sys.user.uid.String()) seal.sys.bwrap.SetEnv[display] = d seal.sys.bwrap.Bind("/tmp/.X11-unix", "/tmp/.X11-unix") } diff --git a/internal/app/start.go b/internal/app/start.go index cfb8364..31e1491 100644 --- a/internal/app/start.go +++ b/internal/app/start.go @@ -57,7 +57,7 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error { a.shim = new(shim.Shim) waitErr := make(chan error, 1) if startTime, err := a.shim.Start( - a.seal.sys.user.as, + a.seal.sys.user.aid.String(), a.seal.sys.user.supp, a.seal.sys.sp, ); err != nil { @@ -90,14 +90,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, + ID: a.id.unwrap(), PID: a.shim.Unwrap().Process.Pid, Time: *startTime, } // register process state var err0 = new(StateStoreError) - err0.Inner, err0.DoErr = a.seal.store.Do(a.seal.sys.user.aid, func(c state.Cursor) { + err0.Inner, err0.DoErr = a.seal.store.Do(a.seal.sys.user.aid.unwrap(), func(c state.Cursor) { err0.InnerErr = c.Save(&sd, a.seal.ct) }) a.seal.sys.saveState = true @@ -147,11 +147,11 @@ func (a *app) Run(ctx context.Context, rs *fst.RunState) error { // update store and revert app setup transaction e := new(StateStoreError) - e.Inner, e.DoErr = a.seal.store.Do(a.seal.sys.user.aid, func(b state.Cursor) { + e.Inner, e.DoErr = a.seal.store.Do(a.seal.sys.user.aid.unwrap(), 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(*a.id); err != nil { + if err := b.Destroy(a.id.unwrap()); err != nil { return err } } diff --git a/internal/app/strings.go b/internal/app/strings.go new file mode 100644 index 0000000..19f6ea8 --- /dev/null +++ b/internal/app/strings.go @@ -0,0 +1,19 @@ +package app + +import ( + "strconv" + + "git.gensokyo.uk/security/fortify/fst" +) + +func newInt(v int) *stringPair[int] { return &stringPair[int]{v, strconv.Itoa(v)} } +func newID(id *fst.ID) *stringPair[fst.ID] { return &stringPair[fst.ID]{*id, id.String()} } + +// stringPair stores a value and its string representation. +type stringPair[T comparable] struct { + v T + s string +} + +func (s *stringPair[T]) unwrap() T { return s.v } +func (s *stringPair[T]) String() string { return s.s } diff --git a/internal/app/system.go b/internal/app/system.go index ccc5293..3a92b0f 100644 --- a/internal/app/system.go +++ b/internal/app/system.go @@ -21,9 +21,7 @@ type appSealSys struct { user appUser // mapped uid and gid in user namespace - mappedID int - // string representation of mappedID - mappedIDString string + mapuid *stringPair[int] needRevert bool saveState bool @@ -33,19 +31,14 @@ type appSealSys struct { } type appUser struct { - // full uid resolved by fsu - uid int - // string representation of uid - us string + // application id + aid *stringPair[int] + // target uid resolved by fid:aid + uid *stringPair[int] // supplementary group ids supp []string - // application id - aid int - // string representation of aid - as string - // home directory host path data string // app user home directory