helper/bwrap: ordered filesystem args

The argument builder was written based on the incorrect assumption that bwrap arguments are unordered. The argument builder is replaced in this commit to correct that mistake.

Signed-off-by: Ophestra Umiker <cat@ophivana.moe>

10 files changed, 75 insertions, 98 deletions

diff --git a/internal/app/config.go b/internal/app/config.go
index 7950c59d..3f6d1c0f 100644
--- a/internal/app/config.go
+++ b/internal/app/config.go
@@ -1,11 +1,18 @@
 package app
 
 import (
+	"encoding/gob"
+	"os"
+
 	"git.ophivana.moe/cat/fortify/dbus"
 	"git.ophivana.moe/cat/fortify/helper/bwrap"
 	"git.ophivana.moe/cat/fortify/internal/state"
 )
 
+func init() {
+	gob.Register(new(bwrap.PermConfig[*bwrap.TmpfsConfig]))
+}
+
 // Config is used to seal an *App
 type Config struct {
 	// D-Bus application ID
@@ -55,7 +62,7 @@ type SandboxConfig struct {
 	// sandbox host filesystem access
 	Filesystem []*FilesystemConfig `json:"filesystem"`
 	// tmpfs mount points to mount last
-	Tmpfs []bwrap.TmpfsConfig `json:"tmpfs"`
+	Tmpfs []string `json:"tmpfs"`
 }
 
 type FilesystemConfig struct {
@@ -71,61 +78,39 @@ type FilesystemConfig struct {
 	Must bool `json:"require,omitempty"`
 }
 
+// Bwrap returns the address of the corresponding bwrap.Config to s.
+// Note that remaining tmpfs entries must be queued by the caller prior to launch.
 func (s *SandboxConfig) Bwrap() *bwrap.Config {
 	if s == nil {
 		return nil
 	}
 
-	nobody := 65534
-	conf := &bwrap.Config{
+	conf := (&bwrap.Config{
 		Net:           s.Net,
 		UserNS:        s.UserNS,
-		UID:           &nobody,
-		GID:           &nobody,
 		Hostname:      s.Hostname,
 		Clearenv:      true,
 		SetEnv:        s.Env,
-		Procfs:        []string{"/proc"},
-		DevTmpfs:      []string{"/dev"},
-		Mqueue:        []string{"/dev/mqueue"},
 		NewSession:    !s.NoNewSession,
 		DieWithParent: true,
 		AsInit:        true,
-	}
+
+		// initialise map
+		Chmod: make(map[string]os.FileMode),
+	}).
+		SetUID(65534).SetGID(65534).
+		Procfs("/proc").DevTmpfs("/dev").Mqueue("/dev/mqueue")
 
 	for _, c := range s.Filesystem {
 		if c == nil {
 			continue
 		}
-		p := [2]string{c.Src, c.Dst}
+		src := c.Src
+		dest := c.Dst
 		if c.Dst == "" {
-			p[1] = c.Src
-		}
-
-		switch {
-		case c.Device:
-			if c.Must {
-				conf.DevBind = append(conf.DevBind, p)
-			} else {
-				conf.DevBindTry = append(conf.DevBindTry, p)
-			}
-		case c.Write:
-			if c.Must {
-				conf.Bind = append(conf.Bind, p)
-			} else {
-				conf.BindTry = append(conf.BindTry, p)
-			}
-		default:
-			if c.Must {
-				conf.ROBind = append(conf.ROBind, p)
-			} else {
-				conf.ROBindTry = append(conf.ROBindTry, p)
-			}
+			dest = c.Src
 		}
-	}
-
-	for _, tmpfs := range s.Tmpfs {
-		conf.Tmpfs = append(conf.Tmpfs, bwrap.PermConfig[bwrap.TmpfsConfig]{Path: tmpfs, Last: true})
+		conf.Bind(src, dest, !c.Must, c.Write, c.Device)
 	}
 
 	return conf
@@ -164,9 +149,7 @@ func Template() *Config {
 					{Src: "/data/user/0", Dst: "/data/data", Write: true, Must: true},
 					{Src: "/var/tmp", Write: true},
 				},
-				Tmpfs: []bwrap.TmpfsConfig{
-					{Size: 8 * 1024, Dir: "/var/run/nscd"},
-				},
+				Tmpfs: []string{"/var/run/nscd"},
 			},
 			SystemBus: &dbus.Config{
 				See:       nil,
diff --git a/internal/app/seal.go b/internal/app/seal.go
index 9199f5b0..146bcc52 100644
--- a/internal/app/seal.go
+++ b/internal/app/seal.go
@@ -9,7 +9,6 @@ import (
 	"strconv"
 
 	"git.ophivana.moe/cat/fortify/dbus"
-	"git.ophivana.moe/cat/fortify/helper/bwrap"
 	"git.ophivana.moe/cat/fortify/internal"
 	"git.ophivana.moe/cat/fortify/internal/state"
 	"git.ophivana.moe/cat/fortify/internal/verbose"
@@ -163,7 +162,7 @@ func (a *app) Seal(config *Config) error {
 		// hide nscd from sandbox if present
 		nscd := "/var/run/nscd"
 		if _, err := os.Stat(nscd); !errors.Is(err, os.ErrNotExist) {
-			conf.Tmpfs = append(conf.Tmpfs, bwrap.TmpfsConfig{Size: 8 * 1024, Dir: nscd})
+			conf.Tmpfs = append(conf.Tmpfs, nscd)
 		}
 		// bind GPU stuff
 		if config.Confinement.Enablements.Has(state.EnableX) || config.Confinement.Enablements.Has(state.EnableWayland) {
@@ -172,6 +171,7 @@ func (a *app) Seal(config *Config) error {
 		config.Confinement.Sandbox = conf
 	}
 	seal.sys.bwrap = config.Confinement.Sandbox.Bwrap()
+	seal.sys.tmpfs = config.Confinement.Sandbox.Tmpfs
 	if seal.sys.bwrap.SetEnv == nil {
 		seal.sys.bwrap.SetEnv = make(map[string]string)
 	}
diff --git a/internal/app/share.dbus.go b/internal/app/share.dbus.go
index 804dbd3f..912cd97b 100644
--- a/internal/app/share.dbus.go
+++ b/internal/app/share.dbus.go
@@ -65,12 +65,12 @@ func (seal *appSeal) shareDBus(config [2]*dbus.Config) error {
 	// share proxy sockets
 	sessionInner := path.Join(seal.sys.runtime, "bus")
 	seal.sys.setEnv(dbusSessionBusAddress, "unix:path="+sessionInner)
-	seal.sys.bind(sessionBus[1], sessionInner, true)
+	seal.sys.bwrap.Bind(sessionBus[1], sessionInner)
 	seal.sys.updatePerm(sessionBus[1], acl.Read, acl.Write)
 	if seal.sys.dbusSystem {
 		systemInner := "/run/dbus/system_bus_socket"
 		seal.sys.setEnv(dbusSystemBusAddress, "unix:path="+systemInner)
-		seal.sys.bind(systemBus[1], systemInner, true)
+		seal.sys.bwrap.Bind(systemBus[1], systemInner)
 		seal.sys.updatePerm(systemBus[1], acl.Read, acl.Write)
 	}
 
diff --git a/internal/app/share.display.go b/internal/app/share.display.go
index da9f6eff..86343d8b 100644
--- a/internal/app/share.display.go
+++ b/internal/app/share.display.go
@@ -41,7 +41,7 @@ func (seal *appSeal) shareDisplay() error {
 			w := path.Join(seal.sys.runtime, "wayland-0")
 			seal.sys.link(wp, wpi)
 			seal.sys.setEnv(waylandDisplay, w)
-			seal.sys.bind(wpi, w, true)
+			seal.sys.bwrap.Bind(wpi, w)
 
 			// ensure Wayland socket ACL (e.g. `/run/user/%d/wayland-%d`)
 			seal.sys.updatePermTag(state.EnableWayland, wp, acl.Read, acl.Write, acl.Execute)
@@ -59,7 +59,7 @@ func (seal *appSeal) shareDisplay() error {
 		} else {
 			seal.sys.changeHosts(seal.sys.Username)
 			seal.sys.setEnv(display, d)
-			seal.sys.bind("/tmp/.X11-unix", "/tmp/.X11-unix", true)
+			seal.sys.bwrap.Bind("/tmp/.X11-unix", "/tmp/.X11-unix")
 		}
 	}
 
diff --git a/internal/app/share.pulse.go b/internal/app/share.pulse.go
index 39e6fc82..df26b16c 100644
--- a/internal/app/share.pulse.go
+++ b/internal/app/share.pulse.go
@@ -65,7 +65,7 @@ func (seal *appSeal) sharePulse() error {
 	psi := path.Join(seal.shareLocal, "pulse")
 	p := path.Join(seal.sys.runtime, "pulse", "native")
 	seal.sys.link(ps, psi)
-	seal.sys.bind(psi, p, true)
+	seal.sys.bwrap.Bind(psi, p)
 	seal.sys.setEnv(pulseServer, "unix:"+p)
 
 	// publish current user's pulse cookie for target user
diff --git a/internal/app/share.runtime.go b/internal/app/share.runtime.go
index ac4135f4..ebb13aa6 100644
--- a/internal/app/share.runtime.go
+++ b/internal/app/share.runtime.go
@@ -4,7 +4,6 @@ import (
 	"path"
 
 	"git.ophivana.moe/cat/fortify/acl"
-	"git.ophivana.moe/cat/fortify/helper/bwrap"
 	"git.ophivana.moe/cat/fortify/internal/state"
 )
 
@@ -17,20 +16,13 @@ const (
 // shareRuntime queues actions for sharing/ensuring the runtime and share directories
 func (seal *appSeal) shareRuntime() {
 	// mount tmpfs on inner runtime (e.g. `/run/user/%d`)
-	seal.sys.bwrap.Tmpfs = append(seal.sys.bwrap.Tmpfs,
-		bwrap.PermConfig[bwrap.TmpfsConfig]{
-			Path: bwrap.TmpfsConfig{
-				Size: 1 * 1024 * 1024,
-				Dir:  "/run/user",
-			},
-		},
-		bwrap.PermConfig[bwrap.TmpfsConfig]{
-			Path: bwrap.TmpfsConfig{
-				Size: 8 * 1024 * 1024,
-				Dir:  seal.sys.runtime,
-			},
-		},
-	)
+	seal.sys.bwrap.Tmpfs("/run/user", 1*1024*1024)
+	seal.sys.bwrap.Tmpfs(seal.sys.runtime, 8*1024*1024)
+
+	// point to inner runtime path `/run/user/%d`
+	seal.sys.setEnv(xdgRuntimeDir, seal.sys.runtime)
+	seal.sys.setEnv(xdgSessionClass, "user")
+	seal.sys.setEnv(xdgSessionType, "tty")
 
 	// ensure RunDir (e.g. `/run/user/%d/fortify`)
 	seal.sys.ensure(seal.RunDirPath, 0700)
@@ -53,22 +45,3 @@ func (seal *appSeal) shareRuntime() {
 	seal.sys.ensureEphemeral(seal.shareLocal, 0700)
 	seal.sys.updatePerm(seal.shareLocal, acl.Execute)
 }
-
-func (seal *appSeal) shareRuntimeChild() string {
-	// ensure child runtime parent directory (e.g. `/tmp/fortify.%d/runtime`)
-	targetRuntimeParent := path.Join(seal.SharePath, "runtime")
-	seal.sys.ensure(targetRuntimeParent, 0700)
-	seal.sys.updatePermTag(state.EnableLength, targetRuntimeParent, acl.Execute)
-
-	// ensure child runtime directory (e.g. `/tmp/fortify.%d/runtime/%d`)
-	targetRuntime := path.Join(targetRuntimeParent, seal.sys.Uid)
-	seal.sys.ensure(targetRuntime, 0700)
-	seal.sys.updatePermTag(state.EnableLength, targetRuntime, acl.Read, acl.Write, acl.Execute)
-
-	// point to ensured runtime path
-	seal.sys.setEnv(xdgRuntimeDir, targetRuntime)
-	seal.sys.setEnv(xdgSessionClass, "user")
-	seal.sys.setEnv(xdgSessionType, "tty")
-
-	return targetRuntime
-}
diff --git a/internal/app/share.system.go b/internal/app/share.system.go
index 1b17ca49..e1a3e1de 100644
--- a/internal/app/share.system.go
+++ b/internal/app/share.system.go
@@ -3,6 +3,9 @@ package app
 import (
 	"os"
 	"path"
+
+	"git.ophivana.moe/cat/fortify/acl"
+	"git.ophivana.moe/cat/fortify/internal/state"
 )
 
 const (
@@ -38,6 +41,24 @@ func (seal *appSeal) shareSystem() {
 	seal.sys.writeFile(groupPath, []byte("fortify:x:65534:\n"))
 
 	// bind /etc/passwd and /etc/group
-	seal.sys.bind(passwdPath, "/etc/passwd", true)
-	seal.sys.bind(groupPath, "/etc/group", true)
+	seal.sys.bwrap.Bind(passwdPath, "/etc/passwd")
+	seal.sys.bwrap.Bind(groupPath, "/etc/group")
+}
+
+func (seal *appSeal) shareTmpdirChild() string {
+	// ensure child tmpdir parent directory (e.g. `/tmp/fortify.%d/tmpdir`)
+	targetTmpdirParent := path.Join(seal.SharePath, "tmpdir")
+	seal.sys.ensure(targetTmpdirParent, 0700)
+	seal.sys.updatePermTag(state.EnableLength, targetTmpdirParent, acl.Execute)
+
+	// ensure child tmpdir (e.g. `/tmp/fortify.%d/tmpdir/%d`)
+	targetTmpdir := path.Join(targetTmpdirParent, seal.sys.Uid)
+	seal.sys.ensure(targetTmpdir, 01700)
+	seal.sys.updatePermTag(state.EnableLength, targetTmpdir, acl.Read, acl.Write, acl.Execute)
+	seal.sys.bwrap.Bind(targetTmpdir, "/tmp", false, true)
+
+	// mount tmpfs on inner shared directory (e.g. `/tmp/fortify.%d`)
+	seal.sys.bwrap.Tmpfs(seal.SharePath, 1*1024*1024)
+
+	return targetTmpdir
 }
diff --git a/internal/app/start.go b/internal/app/start.go
index 725d69dd..c496561c 100644
--- a/internal/app/start.go
+++ b/internal/app/start.go
@@ -70,7 +70,7 @@ func (a *app) Start() error {
 	a.cmd.Stderr = os.Stderr
 	a.cmd.Dir = a.seal.RunDirPath
 
-	if wls, err := shim.ServeConfig(confSockPath, &shim.Payload{
+	if wls, err := shim.ServeConfig(confSockPath, a.seal.sys.uid, &shim.Payload{
 		Argv:  a.seal.command,
 		Exec:  shimExec,
 		Bwrap: a.seal.sys.bwrap,
diff --git a/internal/app/system.go b/internal/app/system.go
index 4f3b6730..a0162939 100644
--- a/internal/app/system.go
+++ b/internal/app/system.go
@@ -59,6 +59,7 @@ type appSeal struct {
 // appSealTx contains the system-level component of the app seal
 type appSealTx struct {
 	bwrap *bwrap.Config
+	tmpfs []string
 
 	// reference to D-Bus proxy instance, nil if disabled
 	dbus *dbus.Proxy
@@ -110,15 +111,6 @@ func (tx *appSealTx) setEnv(k, v string) {
 	tx.bwrap.SetEnv[k] = v
 }
 
-// bind mounts a directory within the sandbox
-func (tx *appSealTx) bind(src, dest string, ro bool) {
-	if !ro {
-		tx.bwrap.Bind = append(tx.bwrap.Bind, [2]string{src, dest})
-	} else {
-		tx.bwrap.ROBind = append(tx.bwrap.ROBind, [2]string{src, dest})
-	}
-}
-
 // ensure appends a directory ensure action
 func (tx *appSealTx) ensure(path string, perm os.FileMode) {
 	tx.mkdir = append(tx.mkdir, appEnsureEntry{path, perm, false})
@@ -183,14 +175,14 @@ func (tx *appSealTx) changeHosts(username string) {
 func (tx *appSealTx) writeFile(dst string, data []byte) {
 	tx.files = append(tx.files, [2]string{dst, string(data)})
 	tx.updatePerm(dst, acl.Read)
-	tx.bind(dst, dst, true)
+	tx.bwrap.Bind(dst, dst)
 }
 
 // copyFile appends a tmpfiles action
 func (tx *appSealTx) copyFile(dst, src string) {
 	tx.tmpfiles = append(tx.tmpfiles, [2]string{dst, src})
 	tx.updatePerm(dst, acl.Read)
-	tx.bind(dst, dst, true)
+	tx.bwrap.Bind(dst, dst)
 }
 
 // link appends a hardlink action
@@ -324,6 +316,12 @@ func (tx *appSealTx) commit() error {
 
 	// disarm partial commit rollback
 	txp = nil
+
+	// queue tmpfs at the end of tx.bwrap.Filesystem
+	for _, dest := range tx.tmpfs {
+		tx.bwrap.Tmpfs(dest, 8*1024)
+	}
+
 	return nil
 }
 
@@ -416,10 +414,10 @@ func (seal *appSeal) shareAll(bus [2]*dbus.Config) error {
 	}
 	seal.shared = true
 
+	targetTmpdir := seal.shareTmpdirChild()
+	verbose.Printf("child tmpdir %q configured\n", targetTmpdir)
 	seal.shareRuntime()
 	seal.shareSystem()
-	targetRuntime := seal.shareRuntimeChild()
-	verbose.Printf("child runtime data dir '%s' configured\n", targetRuntime)
 	if err := seal.shareDisplay(); err != nil {
 		return err
 	}
diff --git a/internal/shim/parent.go b/internal/shim/parent.go
index 98856555..59be5885 100644
--- a/internal/shim/parent.go
+++ b/internal/shim/parent.go
@@ -8,12 +8,13 @@ import (
 	"os"
 	"syscall"
 
+	"git.ophivana.moe/cat/fortify/acl"
 	"git.ophivana.moe/cat/fortify/internal/verbose"
 )
 
 // called in the parent process
 
-func ServeConfig(socket string, payload *Payload, wl string, done chan struct{}) (*net.UnixConn, error) {
+func ServeConfig(socket string, uid int, payload *Payload, wl string, done chan struct{}) (*net.UnixConn, error) {
 	var ws *net.UnixConn
 	if payload.WL {
 		if f, err := net.DialUnix("unix", nil, &net.UnixAddr{Name: wl, Net: "unix"}); err != nil {
@@ -28,7 +29,7 @@ func ServeConfig(socket string, payload *Payload, wl string, done chan struct{})
 		return nil, err
 	} else {
 		verbose.Println("configuring shim on socket", socket)
-		if err = os.Chmod(socket, 0777); err != nil {
+		if err = acl.UpdatePerm(socket, uid, acl.Read, acl.Write, acl.Execute); err != nil {
 			fmt.Println("fortify: cannot change permissions of shim setup socket:", err)
 		}
 
@@ -39,6 +40,7 @@ func ServeConfig(socket string, payload *Payload, wl string, done chan struct{})
 			} else {
 				if err = gob.NewEncoder(conn).Encode(*payload); err != nil {
 					fmt.Println("fortify: cannot stream shim payload:", err)
+					_ = os.Remove(socket)
 					return
 				}