From 116d8af91fc0a9f2cfb908e370d9bbe5b6913265 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Tue, 25 Mar 2025 04:42:30 +0900 Subject: [PATCH] system: optimise string formatting Signed-off-by: Ophestra --- cmd/fpkg/app.go | 2 +- fst/config.go | 4 +- internal/app/app_nixos_test.go | 2 +- internal/app/app_pd_test.go | 2 +- internal/app/process.go | 31 ++++---------- internal/app/seal.go | 10 ++--- main.go | 39 ++++++++++------- print_test.go | 18 ++++---- system/acl_test.go | 18 ++++---- system/enablement.go | 77 +++++++++++++--------------------- system/enablement_test.go | 43 +++++++++++++++++++ system/mkdir.go | 2 +- system/mkdir_test.go | 16 +++---- system/op.go | 34 ++++++++++----- system/op_internal_test.go | 19 ++------- system/op_test.go | 10 +++-- 16 files changed, 173 insertions(+), 154 deletions(-) create mode 100644 system/enablement_test.go diff --git a/cmd/fpkg/app.go b/cmd/fpkg/app.go index a1e5de9..cf92dad 100644 --- a/cmd/fpkg/app.go +++ b/cmd/fpkg/app.go @@ -41,7 +41,7 @@ type appInfo struct { // passed through to [fst.Config] SessionBus *dbus.Config `json:"session_bus,omitempty"` // passed through to [fst.Config] - Enablements system.Enablements `json:"enablements"` + Enablements system.Enablement `json:"enablements"` // passed through to [fst.Config] Multiarch bool `json:"multiarch,omitempty"` diff --git a/fst/config.go b/fst/config.go index 7fc3af6..866f730 100644 --- a/fst/config.go +++ b/fst/config.go @@ -48,7 +48,7 @@ type ConfinementConfig struct { SessionBus *dbus.Config `json:"session_bus,omitempty"` // system resources to expose to the container - Enablements system.Enablements `json:"enablements"` + Enablements system.Enablement `json:"enablements"` } type ExtraPermConfig struct { @@ -153,7 +153,7 @@ func Template() *Config { Log: false, Filter: true, }, - Enablements: system.EWayland.Mask() | system.EDBus.Mask() | system.EPulse.Mask(), + Enablements: system.EWayland | system.EDBus | system.EPulse, }, } } diff --git a/internal/app/app_nixos_test.go b/internal/app/app_nixos_test.go index ca9fde0..d89d099 100644 --- a/internal/app/app_nixos_test.go +++ b/internal/app/app_nixos_test.go @@ -45,7 +45,7 @@ var testCasesNixos = []sealTestCase{ Call: map[string]string{}, Broadcast: map[string]string{}, Filter: true, }, - Enablements: system.EWayland.Mask() | system.EDBus.Mask() | system.EPulse.Mask(), + Enablements: system.EWayland | system.EDBus | system.EPulse, }, }, fst.ID{ diff --git a/internal/app/app_pd_test.go b/internal/app/app_pd_test.go index 1535579..bde436f 100644 --- a/internal/app/app_pd_test.go +++ b/internal/app/app_pd_test.go @@ -196,7 +196,7 @@ var testCasesPd = []sealTestCase{ }, Filter: true, }, - Enablements: system.EWayland.Mask() | system.EDBus.Mask() | system.EPulse.Mask(), + Enablements: system.EWayland | system.EDBus | system.EPulse, }, }, fst.ID{ diff --git a/internal/app/process.go b/internal/app/process.go index 6c33714..00f6643 100644 --- a/internal/app/process.go +++ b/internal/app/process.go @@ -5,7 +5,6 @@ import ( "errors" "log" "os/exec" - "strings" "time" "git.gensokyo.uk/security/fortify/fst" @@ -54,17 +53,16 @@ func (seal *outcome) Run(rs *fst.RunState) error { revert app setup transaction */ - rt, ec := new(system.Enablements), new(system.Criteria) - ec.Enablements = new(system.Enablements) - ec.Set(system.Process) + var rt system.Enablement + ec := system.Process if states, err := c.Load(); err != nil { // revert per-process state here to limit damage storeErr.OpErr = err - return seal.sys.Revert(ec) + return seal.sys.Revert((*system.Criteria)(&ec)) } else { if l := len(states); l == 0 { fmsg.Verbose("no other launchers active, will clean up globals") - ec.Set(system.User) + ec |= system.User } else { fmsg.Verbosef("found %d active launchers, cleaning up without globals", l) } @@ -72,31 +70,20 @@ func (seal *outcome) Run(rs *fst.RunState) error { // accumulate enablements of remaining launchers for i, s := range states { if s.Config != nil { - *rt |= s.Config.Confinement.Enablements + rt |= s.Config.Confinement.Enablements } else { log.Printf("state entry %d does not contain config", i) } } } - // invert accumulated enablements for cleanup - for i := system.Enablement(0); i < system.Enablement(system.ELen); i++ { - if !rt.Has(i) { - ec.Set(i) - } - } + ec |= rt ^ (system.EWayland | system.EX11 | system.EDBus | system.EPulse) if fmsg.Load() { - labels := make([]string, 0, system.ELen+1) - for i := system.Enablement(0); i < system.Enablement(system.ELen+2); i++ { - if ec.Has(i) { - labels = append(labels, system.TypeString(i)) - } - } - if len(labels) > 0 { - fmsg.Verbose("reverting operations type", strings.Join(labels, ", ")) + if ec > 0 { + fmsg.Verbose("reverting operations type", system.TypeString(ec)) } } - return seal.sys.Revert(ec) + return seal.sys.Revert((*system.Criteria)(&ec)) }() }) storeErr.save([]error{revertErr, store.Close()}) diff --git a/internal/app/seal.go b/internal/app/seal.go index a25897d..549de16 100644 --- a/internal/app/seal.go +++ b/internal/app/seal.go @@ -224,7 +224,7 @@ func (seal *outcome) finalise(ctx context.Context, sys sys.State, config *fst.Co conf.Cover = append(conf.Cover, nscd) } // bind GPU stuff - if config.Confinement.Enablements.Has(system.EX11) || config.Confinement.Enablements.Has(system.EWayland) { + if config.Confinement.Enablements&(system.EX11|system.EWayland) != 0 { conf.Filesystem = append(conf.Filesystem, &fst.FilesystemConfig{Src: "/dev/dri", Device: true}) } // opportunistically bind kvm @@ -338,7 +338,7 @@ func (seal *outcome) finalise(ctx context.Context, sys sys.State, config *fst.Co seal.env[term] = t } - if config.Confinement.Enablements.Has(system.EWayland) { + if config.Confinement.Enablements&system.EWayland != 0 { // outer wayland socket (usually `/run/user/%d/wayland-%d`) var socketPath string if name, ok := sys.LookupEnv(wl.WaylandDisplay); !ok { @@ -371,7 +371,7 @@ func (seal *outcome) finalise(ctx context.Context, sys sys.State, config *fst.Co } } - if config.Confinement.Enablements.Has(system.EX11) { + if config.Confinement.Enablements&system.EX11 != 0 { if d, ok := sys.LookupEnv(display); !ok { return fmsg.WrapError(ErrXDisplay, "DISPLAY is not set") @@ -386,7 +386,7 @@ func (seal *outcome) finalise(ctx context.Context, sys sys.State, config *fst.Co PulseAudio server and authentication */ - if config.Confinement.Enablements.Has(system.EPulse) { + if config.Confinement.Enablements&system.EPulse != 0 { // PulseAudio runtime directory (usually `/run/user/%d/pulse`) pulseRuntimeDir := path.Join(sc.RuntimePath, "pulse") // PulseAudio socket (usually `/run/user/%d/pulse/native`) @@ -439,7 +439,7 @@ func (seal *outcome) finalise(ctx context.Context, sys sys.State, config *fst.Co D-Bus proxy */ - if config.Confinement.Enablements.Has(system.EDBus) { + if config.Confinement.Enablements&system.EDBus != 0 { // ensure dbus session bus defaults if config.Confinement.SessionBus == nil { config.Confinement.SessionBus = dbus.NewConfig(config.ID, true, true) diff --git a/main.go b/main.go index cd9489a..9890b52 100644 --- a/main.go +++ b/main.go @@ -96,12 +96,13 @@ func buildCommand(out io.Writer) command.Command { mpris bool dbusVerbose bool - fid string - aid int - groups command.RepeatableFlag - homeDir string - userName string - enablements [system.ELen]bool + fid string + aid int + groups command.RepeatableFlag + homeDir string + userName string + + wayland, x11, dBus, pulse bool ) c.NewCommand("run", "Configure and start a permissive default sandbox", func(args []string) error { @@ -158,15 +159,21 @@ func buildCommand(out io.Writer) command.Command { config.Confinement.Outer = homeDir config.Confinement.Username = userName - // enablements from flags - for i := system.Enablement(0); i < system.Enablement(system.ELen); i++ { - if enablements[i] { - config.Confinement.Enablements.Set(i) - } + if wayland { + config.Confinement.Enablements |= system.EWayland + } + if x11 { + config.Confinement.Enablements |= system.EX11 + } + if dBus { + config.Confinement.Enablements |= system.EDBus + } + if pulse { + config.Confinement.Enablements |= system.EPulse } // parse D-Bus config file from flags if applicable - if enablements[system.EDBus] { + if dBus { if dbusConfigSession == "builtin" { config.Confinement.SessionBus = dbus.NewConfig(fid, true, mpris) } else { @@ -215,13 +222,13 @@ func buildCommand(out io.Writer) command.Command { "Application home directory"). Flag(&userName, "u", command.StringFlag("chronos"), "Passwd name within sandbox"). - Flag(&enablements[system.EWayland], "wayland", command.BoolFlag(false), + Flag(&wayland, "wayland", command.BoolFlag(false), "Allow Wayland connections"). - Flag(&enablements[system.EX11], "X", command.BoolFlag(false), + Flag(&x11, "X", command.BoolFlag(false), "Share X11 socket and allow connection"). - Flag(&enablements[system.EDBus], "dbus", command.BoolFlag(false), + Flag(&dBus, "dbus", command.BoolFlag(false), "Proxy D-Bus connection"). - Flag(&enablements[system.EPulse], "pulse", command.BoolFlag(false), + Flag(&pulse, "pulse", command.BoolFlag(false), "Share PulseAudio socket and cookie") } diff --git a/print_test.go b/print_test.go index 3c70899..addeb60 100644 --- a/print_test.go +++ b/print_test.go @@ -37,7 +37,7 @@ func Test_printShowInstance(t *testing.T) { }{ {"config", nil, fst.Template(), false, false, `App ID: 9 (org.chromium.Chromium) - Enablements: Wayland, D-Bus, PulseAudio + Enablements: wayland, dbus, pulseaudio Groups: ["video"] Directory: /var/lib/persist/home/org.chromium.Chromium Hostname: "localhost" @@ -74,14 +74,14 @@ System bus App ID: 0 - Enablements: (No enablements) + Enablements: (no enablements) Directory: Command: `}, {"config flag none", nil, &fst.Config{Confinement: fst.ConfinementConfig{Sandbox: new(fst.SandboxConfig)}}, false, false, `App ID: 0 - Enablements: (No enablements) + Enablements: (no enablements) Directory: Flags: none Etc: /etc @@ -90,7 +90,7 @@ App `}, {"config nil entries", nil, &fst.Config{Confinement: fst.ConfinementConfig{Sandbox: &fst.SandboxConfig{Filesystem: make([]*fst.FilesystemConfig, 1)}, ExtraPerms: make([]*fst.ExtraPermConfig, 1)}}, false, false, `App ID: 0 - Enablements: (No enablements) + Enablements: (no enablements) Directory: Flags: none Etc: /etc @@ -105,7 +105,7 @@ Extra ACL App ID: 0 - Enablements: (No enablements) + Enablements: (no enablements) Directory: Command: @@ -121,7 +121,7 @@ Session bus App ID: 9 (org.chromium.Chromium) - Enablements: Wayland, D-Bus, PulseAudio + Enablements: wayland, dbus, pulseaudio Groups: ["video"] Directory: /var/lib/persist/home/org.chromium.Chromium Hostname: "localhost" @@ -162,7 +162,7 @@ State App ID: 0 - Enablements: (No enablements) + Enablements: (no enablements) Directory: Command: @@ -469,8 +469,8 @@ func Test_printPs(t *testing.T) { `}, - {"valid", state.Entries{testID: testState}, false, false, ` Instance PID App Uptime Enablements Command - 8e2c76b0 3735928559 9 1h2m32s Wayland, D-Bus, PulseAudio ["chromium" "--ignore-gpu-blocklist" "--disable-smooth-scrolling" "--enable-features=UseOzonePlatform" "--ozone-platform=wayland"] + {"valid", state.Entries{testID: testState}, false, false, ` Instance PID App Uptime Enablements Command + 8e2c76b0 3735928559 9 1h2m32s wayland, dbus, pulseaudio ["chromium" "--ignore-gpu-blocklist" "--disable-smooth-scrolling" "--enable-features=UseOzonePlatform" "--ozone-platform=wayland"] `}, {"valid short", state.Entries{testID: testState}, true, false, `8e2c76b0 diff --git a/system/acl_test.go b/system/acl_test.go index ed154ad..22a33c6 100644 --- a/system/acl_test.go +++ b/system/acl_test.go @@ -46,20 +46,20 @@ func TestUpdatePermType(t *testing.T) { } } -func TestACL_String(t *testing.T) { +func TestACLString(t *testing.T) { testCases := []struct { want string et Enablement perms []acl.Perm }{ - {`--- type: Process path: "/nonexistent"`, Process, []acl.Perm{}}, - {`r-- type: User path: "/nonexistent"`, User, []acl.Perm{acl.Read}}, - {`-w- type: Wayland path: "/nonexistent"`, EWayland, []acl.Perm{acl.Write}}, - {`--x type: X11 path: "/nonexistent"`, EX11, []acl.Perm{acl.Execute}}, - {`rw- type: D-Bus path: "/nonexistent"`, EDBus, []acl.Perm{acl.Read, acl.Write}}, - {`r-x type: PulseAudio path: "/nonexistent"`, EPulse, []acl.Perm{acl.Read, acl.Execute}}, - {`rwx type: User path: "/nonexistent"`, User, []acl.Perm{acl.Read, acl.Write, acl.Execute}}, - {`rwx type: Process path: "/nonexistent"`, Process, []acl.Perm{acl.Read, acl.Write, acl.Write, acl.Execute}}, + {`--- type: process path: "/nonexistent"`, Process, []acl.Perm{}}, + {`r-- type: user path: "/nonexistent"`, User, []acl.Perm{acl.Read}}, + {`-w- type: wayland path: "/nonexistent"`, EWayland, []acl.Perm{acl.Write}}, + {`--x type: x11 path: "/nonexistent"`, EX11, []acl.Perm{acl.Execute}}, + {`rw- type: dbus path: "/nonexistent"`, EDBus, []acl.Perm{acl.Read, acl.Write}}, + {`r-x type: pulseaudio path: "/nonexistent"`, EPulse, []acl.Perm{acl.Read, acl.Execute}}, + {`rwx type: user path: "/nonexistent"`, User, []acl.Perm{acl.Read, acl.Write, acl.Execute}}, + {`rwx type: process path: "/nonexistent"`, Process, []acl.Perm{acl.Read, acl.Write, acl.Write, acl.Execute}}, } for _, tc := range testCases { diff --git a/system/enablement.go b/system/enablement.go index f0911d5..f3e74e0 100644 --- a/system/enablement.go +++ b/system/enablement.go @@ -1,68 +1,47 @@ package system import ( + "fmt" "strings" ) -type ( - // Enablement represents an optional system resource - Enablement uint8 - // Enablements represents optional system resources to share - Enablements uint64 -) +// Enablement represents optional system resources. +type Enablement byte const ( - EWayland Enablement = iota + EWayland Enablement = 1 << iota EX11 EDBus EPulse + + EM ) -var enablementString = [...]string{ - EWayland: "Wayland", - EX11: "X11", - EDBus: "D-Bus", - EPulse: "PulseAudio", -} - -const ELen = len(enablementString) - func (e Enablement) String() string { - if int(e) >= ELen { - return "" - } - return enablementString[e] -} + switch e { + case 0: + return "(no enablements)" + case EWayland: + return "wayland" + case EX11: + return "x11" + case EDBus: + return "dbus" + case EPulse: + return "pulseaudio" + default: + buf := new(strings.Builder) + buf.Grow(32) -func (e Enablement) Mask() Enablements { - return 1 << e -} - -// Has returns whether a feature is enabled -func (es *Enablements) Has(e Enablement) bool { - return *es&e.Mask() != 0 -} - -// Set enables a feature -func (es *Enablements) Set(e Enablement) { - if es.Has(e) { - panic("enablement " + e.String() + " set twice") - } - - *es |= e.Mask() -} - -func (es *Enablements) String() string { - buf := new(strings.Builder) - for i := Enablement(0); i < Enablement(ELen); i++ { - if es.Has(i) { - buf.WriteString(", " + i.String()) + for i := Enablement(1); i < EM; i <<= 1 { + if e&i != 0 { + buf.WriteString(", " + i.String()) + } } - } - if buf.Len() == 0 { - buf.WriteString("(No enablements)") + if buf.Len() == 0 { + return fmt.Sprintf("e%x", byte(e)) + } + return strings.TrimPrefix(buf.String(), ", ") } - - return strings.TrimPrefix(buf.String(), ", ") } diff --git a/system/enablement_test.go b/system/enablement_test.go new file mode 100644 index 0000000..f4f362d --- /dev/null +++ b/system/enablement_test.go @@ -0,0 +1,43 @@ +package system_test + +import ( + "testing" + + "git.gensokyo.uk/security/fortify/system" +) + +func TestEnablementString(t *testing.T) { + testCases := []struct { + flags system.Enablement + want string + }{ + {0, "(no enablements)"}, + {system.EWayland, "wayland"}, + {system.EX11, "x11"}, + {system.EDBus, "dbus"}, + {system.EPulse, "pulseaudio"}, + {system.EWayland | system.EX11, "wayland, x11"}, + {system.EWayland | system.EDBus, "wayland, dbus"}, + {system.EWayland | system.EPulse, "wayland, pulseaudio"}, + {system.EX11 | system.EDBus, "x11, dbus"}, + {system.EX11 | system.EPulse, "x11, pulseaudio"}, + {system.EDBus | system.EPulse, "dbus, pulseaudio"}, + {system.EWayland | system.EX11 | system.EDBus, "wayland, x11, dbus"}, + {system.EWayland | system.EX11 | system.EPulse, "wayland, x11, pulseaudio"}, + {system.EWayland | system.EDBus | system.EPulse, "wayland, dbus, pulseaudio"}, + {system.EX11 | system.EDBus | system.EPulse, "x11, dbus, pulseaudio"}, + {system.EWayland | system.EX11 | system.EDBus | system.EPulse, "wayland, x11, dbus, pulseaudio"}, + + {1 << 5, "e20"}, + {1 << 6, "e40"}, + {1 << 7, "e80"}, + } + + for _, tc := range testCases { + t.Run(tc.want, func(t *testing.T) { + if got := tc.flags.String(); got != tc.want { + t.Errorf("String: %q, want %q", got, tc.want) + } + }) + } +} diff --git a/system/mkdir.go b/system/mkdir.go index bc67997..6e7a55f 100644 --- a/system/mkdir.go +++ b/system/mkdir.go @@ -78,7 +78,7 @@ func (m *Mkdir) Path() string { } func (m *Mkdir) String() string { - t := "Ensure" + t := "ensure" if m.ephemeral { t = TypeString(m.Type()) } diff --git a/system/mkdir_test.go b/system/mkdir_test.go index 2082694..14a47b0 100644 --- a/system/mkdir_test.go +++ b/system/mkdir_test.go @@ -41,20 +41,20 @@ func TestEphemeral(t *testing.T) { } } -func TestMkdir_String(t *testing.T) { +func TestMkdirString(t *testing.T) { testCases := []struct { want string ephemeral bool et Enablement }{ - {"Ensure", false, User}, - {"Ensure", false, Process}, - {"Ensure", false, EWayland}, + {"ensure", false, User}, + {"ensure", false, Process}, + {"ensure", false, EWayland}, - {"Wayland", true, EWayland}, - {"X11", true, EX11}, - {"D-Bus", true, EDBus}, - {"PulseAudio", true, EPulse}, + {"wayland", true, EWayland}, + {"x11", true, EX11}, + {"dbus", true, EDBus}, + {"pulseaudio", true, EPulse}, } for _, tc := range testCases { t.Run(tc.want, func(t *testing.T) { diff --git a/system/op.go b/system/op.go index 3c334b0..10e95c1 100644 --- a/system/op.go +++ b/system/op.go @@ -5,28 +5,29 @@ import ( "context" "errors" "log" + "strings" "sync" ) const ( // User type is reverted at final launcher exit. - User = Enablement(ELen) + User = EM << iota // Process type is unconditionally reverted on exit. - Process = Enablement(ELen + 1) + Process + + CM ) // Criteria specifies types of Op to revert. -type Criteria struct { - *Enablements -} +type Criteria Enablement func (ec *Criteria) hasType(o Op) bool { // nil criteria: revert everything except User - if ec.Enablements == nil { + if ec == nil { return o.Type() != User } - return ec.Has(o.Type()) + return Enablement(*ec)&o.Type() != 0 } // Op is a reversible system operation. @@ -48,11 +49,22 @@ type Op interface { func TypeString(e Enablement) string { switch e { case User: - return "User" + return "user" case Process: - return "Process" + return "process" default: - return e.String() + buf := new(strings.Builder) + buf.Grow(48) + if v := e &^ User &^ Process; v != 0 { + buf.WriteString(v.String()) + } + + for i := User; i < CM; i <<= 1 { + if e&i != 0 { + buf.WriteString(", " + TypeString(i)) + } + } + return strings.TrimPrefix(buf.String(), ", ") } } @@ -110,7 +122,7 @@ func (sys *I) Commit(ctx context.Context) error { if sp != nil { // rollback partial commit msg.Verbosef("commit faulted after %d ops, rolling back partial commit", len(sp.ops)) - if err := sp.Revert(&Criteria{nil}); err != nil { + if err := sp.Revert(nil); err != nil { log.Println("errors returned reverting partial commit:", err) } } diff --git a/system/op_internal_test.go b/system/op_internal_test.go index 7ec4f17..4d43309 100644 --- a/system/op_internal_test.go +++ b/system/op_internal_test.go @@ -47,10 +47,10 @@ func (ptc tcOp) test(t *testing.T, gotOps []Op, wantOps []Op, fn string) { ec *Criteria want bool }{ - {"nil", newCriteria(), ptc.et != User}, + {"nil", nil, ptc.et != User}, {"self", newCriteria(ptc.et), true}, - {"all", newCriteria(EWayland, EX11, EDBus, EPulse, User, Process), true}, - {"enablements", newCriteria(EWayland, EX11, EDBus, EPulse), ptc.et != User && ptc.et != Process}, + {"all", newCriteria(EWayland | EX11 | EDBus | EPulse | User | Process), true}, + {"enablements", newCriteria(EWayland | EX11 | EDBus | EPulse), ptc.et != User && ptc.et != Process}, } for _, tc := range testCases { @@ -65,15 +65,4 @@ func (ptc tcOp) test(t *testing.T, gotOps []Op, wantOps []Op, fn string) { } } -func newCriteria(labels ...Enablement) *Criteria { - ec := new(Criteria) - if len(labels) == 0 { - return ec - } - - ec.Enablements = new(Enablements) - for _, e := range labels { - ec.Set(e) - } - return ec -} +func newCriteria(e Enablement) *Criteria { return (*Criteria)(&e) } diff --git a/system/op_test.go b/system/op_test.go index 9d17969..5fb1beb 100644 --- a/system/op_test.go +++ b/system/op_test.go @@ -37,15 +37,17 @@ func TestTypeString(t *testing.T) { {system.EX11, system.EX11.String()}, {system.EDBus, system.EDBus.String()}, {system.EPulse, system.EPulse.String()}, - {system.User, "User"}, - {system.Process, "Process"}, + {system.User, "user"}, + {system.Process, "process"}, + {system.User | system.Process, "user, process"}, + {system.EWayland | system.User | system.Process, "wayland, user, process"}, + {system.EX11 | system.Process, "x11, process"}, } for _, tc := range testCases { t.Run("label type string "+tc.want, func(t *testing.T) { if got := system.TypeString(tc.e); got != tc.want { - t.Errorf("TypeString(%d) = %v, want %v", - tc.e, + t.Errorf("TypeString: %q, want %q", got, tc.want) } })