system: optimise string formatting

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

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"`

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,
 		},
 	}
 }

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{

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{

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()})

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)

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")
 	}
 

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

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 {

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 "<invalid enablement>"
-	}
-	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(), ", ")
 }

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)
+			}
+		})
+	}
+}

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())
 	}

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) {

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)
 			}
 		}

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) }

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)
 			}
 		})