helper/bwrap: move sync to helper state

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

dbus/run.go

@@ -110,7 +110,7 @@ func (p *Proxy) Start(ready chan error, output io.Writer, sandbox bool) error {
 			bc.Bind(k, k)
 		}
 
-		h = helper.MustNewBwrap(bc, p.seal, toolPath, argF)
+		h = helper.MustNewBwrap(bc, toolPath, p.seal, argF, nil)
 		cmd = h.Unwrap()
 		p.bwrap = bc
 	}

helper/bwrap.go

@@ -21,7 +21,8 @@ type bubblewrap struct {
 
 	// bwrap pipes
 	control *pipes
-	// sync pipe
+	// keep this fd open while sandbox is running
+	// (--sync-fd FD)
 	sync *os.File
 	// returns an array of arguments passed directly
 	// to the child process spawned by bwrap
@@ -119,8 +120,12 @@ func (b *bubblewrap) Unwrap() *exec.Cmd {
 // MustNewBwrap initialises a new Bwrap instance with wt as the null-terminated argument writer.
 // If wt is nil, the child process spawned by bwrap will not get an argument pipe.
 // Function argF returns an array of arguments passed directly to the child process.
-func MustNewBwrap(conf *bwrap.Config, wt io.WriterTo, name string, argF func(argsFD, statFD int) []string) Helper {
-	b, err := NewBwrap(conf, wt, name, argF)
+func MustNewBwrap(
+	conf *bwrap.Config, name string,
+	wt io.WriterTo, argF func(argsFD, statFD int) []string,
+	syncFd *os.File,
+) Helper {
+	b, err := NewBwrap(conf, name, wt, argF, syncFd)
 	if err != nil {
 		panic(err.Error())
 	} else {
@@ -131,7 +136,11 @@ func MustNewBwrap(conf *bwrap.Config, wt io.WriterTo, name string, argF func(arg
 // NewBwrap initialises a new Bwrap instance with wt as the null-terminated argument writer.
 // If wt is nil, the child process spawned by bwrap will not get an argument pipe.
 // Function argF returns an array of arguments passed directly to the child process.
-func NewBwrap(conf *bwrap.Config, wt io.WriterTo, name string, argF func(argsFD, statFD int) []string) (Helper, error) {
+func NewBwrap(
+	conf *bwrap.Config, name string,
+	wt io.WriterTo, argF func(argsFD, statFD int) []string,
+	syncFd *os.File,
+) (Helper, error) {
 	b := new(bubblewrap)
 
 	if args, err := NewCheckedArgs(conf.Args()); err != nil {
@@ -140,7 +149,7 @@ func NewBwrap(conf *bwrap.Config, wt io.WriterTo, name string, argF func(argsFD,
 		b.control = &pipes{args: args}
 	}
 
-	b.sync = conf.Sync()
+	b.sync = syncFd
 	b.argF = argF
 	b.name = name
 	if wt != nil {

helper/bwrap/builder.go

@@ -161,10 +161,3 @@ func (c *Config) SetGID(gid int) *Config {
 	}
 	return c
 }
-
-// SetSync sets the sync pipe kept open while sandbox is running
-// (--sync-fd FD)
-func (c *Config) SetSync(s *os.File) *Config {
-	c.sync = s
-	return c
-}

helper/bwrap/config.go

@@ -1,9 +1,5 @@
 package bwrap
 
-import (
-	"os"
-)
-
 type Config struct {
 	// unshare every namespace we support by default if nil
 	// (--unshare-all)
@@ -61,10 +57,6 @@ type Config struct {
 	// (--as-pid-1)
 	AsInit bool `json:"as_init"`
 
-	// keep this fd open while sandbox is running
-	// (--sync-fd FD)
-	sync *os.File
-
 	/* unmapped options include:
 	    --unshare-user-try           Create new user namespace if possible else continue by skipping it
 	    --unshare-cgroup-try         Create new cgroup namespace if possible else continue by skipping it
@@ -90,12 +82,6 @@ type Config struct {
 	among which --args is used internally for passing arguments */
 }
 
-// Sync keep this fd open while sandbox is running
-// (--sync-fd FD)
-func (c *Config) Sync() *os.File {
-	return c.sync
-}
-
 type UnshareConfig struct {
 	// (--unshare-user)
 	// create new user namespace

helper/bwrap/config_test.go

@@ -126,8 +126,7 @@ func TestConfig_Args(t *testing.T) {
 			name: "uid gid sync",
 			conf: (new(bwrap.Config)).
 				SetUID(1971).
-				SetGID(100).
-				SetSync(os.Stdin),
+				SetGID(100),
 			want: []string{
 				"--unshare-all", "--unshare-user",
 				"--disable-userns", "--assert-userns-disabled",
@@ -135,8 +134,6 @@ func TestConfig_Args(t *testing.T) {
 				"--uid", "1971",
 				// SetGID(100)
 				"--gid", "100",
-				// SetSync(os.Stdin)
-				// this is set when the process is created
 			},
 		},
 		{
@@ -246,10 +243,4 @@ func TestConfig_Args(t *testing.T) {
 		}()
 		(new(bwrap.Config)).Persist("/run", "", "")
 	})
-
-	t.Run("sync file", func(t *testing.T) {
-		if s := (new(bwrap.Config)).SetSync(os.Stdout).Sync(); s != os.Stdout {
-			t.Errorf("Sync() = %v", s)
-		}
-	})
 }

helper/bwrap_test.go

@@ -31,7 +31,11 @@ func TestBwrap(t *testing.T) {
 			helper.BubblewrapName = bubblewrapName
 		})
 
-		h := helper.MustNewBwrap(sc, argsWt, "fortify", argF)
+		h := helper.MustNewBwrap(
+			sc, "fortify",
+			argsWt, argF,
+			nil,
+		)
 
 		if err := h.Start(); !errors.Is(err, os.ErrNotExist) {
 			t.Errorf("Start() error = %v, wantErr %v",
@@ -40,7 +44,11 @@ func TestBwrap(t *testing.T) {
 	})
 
 	t.Run("valid new helper nil check", func(t *testing.T) {
-		if got := helper.MustNewBwrap(sc, argsWt, "fortify", argF); got == nil {
+		if got := helper.MustNewBwrap(
+			sc, "fortify",
+			argsWt, argF,
+			nil,
+		); got == nil {
 			t.Errorf("MustNewBwrap(%#v, %#v, %#v) got nil",
 				sc, argsWt, "fortify")
 			return
@@ -56,7 +64,11 @@ func TestBwrap(t *testing.T) {
 			}
 		}()
 
-		helper.MustNewBwrap(&bwrap.Config{Hostname: "\x00"}, nil, "fortify", argF)
+		helper.MustNewBwrap(
+			&bwrap.Config{Hostname: "\x00"}, "fortify",
+			nil, argF,
+			nil,
+		)
 	})
 
 	t.Run("start notify without pipes panic", func(t *testing.T) {
@@ -69,13 +81,21 @@ func TestBwrap(t *testing.T) {
 		}()
 
 		panic(fmt.Sprintf("unreachable: %v",
-			helper.MustNewBwrap(sc, nil, "fortify", argF).StartNotify(make(chan error))))
+			helper.MustNewBwrap(
+				sc, "fortify",
+				nil, argF,
+				nil,
+			).StartNotify(make(chan error))))
 	})
 
 	t.Run("start without pipes", func(t *testing.T) {
 		helper.InternalReplaceExecCommand(t)
 
-		h := helper.MustNewBwrap(sc, nil, "crash-test-dummy", argFChecked)
+		h := helper.MustNewBwrap(
+			sc, "crash-test-dummy",
+			nil, argFChecked,
+			nil,
+		)
 		cmd := h.Unwrap()
 
 		stdout, stderr := new(strings.Builder), new(strings.Builder)
@@ -107,6 +127,6 @@ func TestBwrap(t *testing.T) {
 	})
 
 	t.Run("implementation compliance", func(t *testing.T) {
-		testHelper(t, func() helper.Helper { return helper.MustNewBwrap(sc, argsWt, "crash-test-dummy", argF) })
+		testHelper(t, func() helper.Helper { return helper.MustNewBwrap(sc, "crash-test-dummy", argsWt, argF, nil) })
 	})
 }

internal/app/start.go

@@ -45,32 +45,19 @@ func (a *app) Run(ctx context.Context, rs *RunState) error {
 		}
 	}
 
-	a.shim = new(shim.Shim)
-	// keep a reference to shim payload for sync fd
-	payload := &shim.Payload{
-		Argv:  a.seal.command,
-		Exec:  shimExec,
-		Bwrap: a.seal.sys.bwrap,
-		Home:  a.seal.sys.user.data,
-
-		Verbose: fmsg.Verbose(),
-	}
-
 	// startup will go ahead, commit system setup
 	if err := a.seal.sys.Commit(); err != nil {
 		return err
 	}
 	a.seal.sys.needRevert = true
 
-	// export sync pipe from sys
-	a.seal.sys.bwrap.SetSync(a.seal.sys.Sync())
-
 	// start shim via manager
+	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.supp,
-		payload,
+		a.seal.sys.Sync(),
 	); err != nil {
 		return err
 	} else {
@@ -88,7 +75,14 @@ func (a *app) Run(ctx context.Context, rs *RunState) error {
 		}()
 
 		// send payload
-		if err = a.shim.Serve(shimSetupCtx, payload); err != nil {
+		if err = a.shim.Serve(shimSetupCtx, &shim.Payload{
+			Argv:  a.seal.command,
+			Exec:  shimExec,
+			Bwrap: a.seal.sys.bwrap,
+			Home:  a.seal.sys.user.data,
+
+			Verbose: fmsg.Verbose(),
+		}); err != nil {
 			return err
 		}
 

internal/proc/priv/shim/main.go

@@ -62,8 +62,9 @@ func Main() {
 	}
 
 	// restore bwrap sync fd
+	var syncFd *os.File
 	if payload.Sync != nil {
-		payload.Bwrap.SetSync(os.NewFile(*payload.Sync, "sync"))
+		syncFd = os.NewFile(*payload.Sync, "sync")
 	}
 
 	// close setup socket
@@ -134,8 +135,11 @@ func Main() {
 	conf.Symlink("fortify", innerInit)
 
 	helper.BubblewrapName = payload.Exec[0] // resolved bwrap path by parent
-	if b, err := helper.NewBwrap(conf, nil, innerInit,
-		func(int, int) []string { return make([]string, 0) }); err != nil {
+	if b, err := helper.NewBwrap(
+		conf, innerInit,
+		nil, func(int, int) []string { return make([]string, 0) },
+		syncFd,
+	); err != nil {
 		fmsg.Fatalf("malformed sandbox config: %v", err)
 	} else {
 		cmd := b.Unwrap()

internal/proc/priv/shim/manager.go

@@ -24,6 +24,8 @@ type Shim struct {
 	killFallback chan error
 	// monitor to shim encoder
 	encoder *gob.Encoder
+	// bwrap --sync-fd value
+	sync *uintptr
 }
 
 func (s *Shim) String() string {
@@ -46,8 +48,8 @@ func (s *Shim) Start(
 	aid string,
 	// string representation of supplementary group ids
 	supp []string,
-	// shim setup payload
-	payload *Payload,
+	// bwrap --sync-fd
+	syncFd *os.File,
 ) (*time.Time, error) {
 	// prepare user switcher invocation
 	var fsu string
@@ -80,9 +82,9 @@ func (s *Shim) Start(
 	s.cmd.Dir = "/"
 
 	// pass sync fd if set
-	if payload.Bwrap.Sync() != nil {
-		fd := proc.ExtraFile(s.cmd, payload.Bwrap.Sync())
-		payload.Sync = &fd
+	if syncFd != nil {
+		fd := proc.ExtraFile(s.cmd, syncFd)
+		s.sync = &fd
 	}
 
 	fmsg.VPrintln("starting shim via fsu:", s.cmd)
@@ -106,6 +108,7 @@ func (s *Shim) Serve(ctx context.Context, payload *Payload) error {
 	}
 	defer func() { killShim() }()
 
+	payload.Sync = s.sync
 	encodeErr := make(chan error)
 	go func() { encodeErr <- s.encoder.Encode(payload) }()
 

ldd/exec.go

@@ -16,13 +16,15 @@ func Exec(p string) ([]*Entry, error) {
 		cmd *exec.Cmd
 	)
 
-	if b, err := helper.NewBwrap((&bwrap.Config{
+	if b, err := helper.NewBwrap(
+		(&bwrap.Config{
 			Hostname:      "fortify-ldd",
 			Chdir:         "/",
 			NewSession:    true,
 			DieWithParent: true,
-	}).Bind("/", "/").DevTmpfs("/dev"),
-		nil, "ldd", func(_, _ int) []string { return []string{p} }); err != nil {
+		}).Bind("/", "/").DevTmpfs("/dev"), "ldd",
+		nil, func(_, _ int) []string { return []string{p} }, nil,
+	); err != nil {
 		return nil, err
 	} else {
 		cmd = b.Unwrap()