From f9bf20a3c75d01cf597477231f9bbb61f15cd4fd Mon Sep 17 00:00:00 2001 From: Ophestra Date: Sat, 15 Mar 2025 01:02:18 +0900 Subject: [PATCH] helper: rearrange initialisation args This improves consistency across two different helper implementations. Signed-off-by: Ophestra --- dbus/run.go | 16 ++++++++-------- helper/args_test.go | 2 +- helper/bwrap.go | 18 +++++++++--------- helper/bwrap_test.go | 36 +++++++++++++++++++++--------------- helper/direct.go | 15 +++++++++------ helper/direct_test.go | 6 +++--- internal/app/shim/main.go | 10 ++++------ 7 files changed, 55 insertions(+), 48 deletions(-) diff --git a/dbus/run.go b/dbus/run.go index ae2d9e0..e1f54fb 100644 --- a/dbus/run.go +++ b/dbus/run.go @@ -28,25 +28,25 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, sandbox bool) error var ( h helper.Helper - argF = func(argsFD, statFD int) []string { - if statFD == -1 { - return []string{"--args=" + strconv.Itoa(argsFD)} + argF = func(argsFd, statFd int) []string { + if statFd == -1 { + return []string{"--args=" + strconv.Itoa(argsFd)} } else { - return []string{"--args=" + strconv.Itoa(argsFD), "--fd=" + strconv.Itoa(statFD)} + return []string{"--args=" + strconv.Itoa(argsFd), "--fd=" + strconv.Itoa(statFd)} } } ) c, cancel := context.WithCancelCause(ctx) if !sandbox { - h = helper.NewDirect(c, p.seal, p.name, argF, func(cmd *exec.Cmd) { + h = helper.NewDirect(c, p.name, p.seal, true, argF, func(cmd *exec.Cmd) { if output != nil { cmd.Stdout, cmd.Stderr = output, output } // xdg-dbus-proxy does not need to inherit the environment cmd.Env = make([]string, 0) - }, true) + }) } else { // look up absolute path if name is just a file name toolPath := p.name @@ -116,11 +116,11 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, sandbox bool) error bc.Bind(k, k) } - h = helper.MustNewBwrap(c, bc, toolPath, true, p.seal, argF, func(cmd *exec.Cmd) { + h = helper.MustNewBwrap(c, toolPath, p.seal, true, argF, func(cmd *exec.Cmd) { if output != nil { cmd.Stdout, cmd.Stderr = output, output } - }, nil, nil, true) + }, bc, true, nil, nil) p.bwrap = bc } diff --git a/helper/args_test.go b/helper/args_test.go index a23b315..9014041 100644 --- a/helper/args_test.go +++ b/helper/args_test.go @@ -9,7 +9,7 @@ import ( "git.gensokyo.uk/security/fortify/helper" ) -func Test_argsFD_String(t *testing.T) { +func Test_argsFd_String(t *testing.T) { wantString := strings.Join(wantArgs, " ") if got := argsWt.(fmt.Stringer).String(); got != wantString { t.Errorf("String(): got %v; want %v", diff --git a/helper/bwrap.go b/helper/bwrap.go index 7ced938..7e9e9bc 100644 --- a/helper/bwrap.go +++ b/helper/bwrap.go @@ -51,17 +51,17 @@ func (b *bubblewrap) Start() error { // Function argF returns an array of arguments passed directly to the child process. func MustNewBwrap( ctx context.Context, - conf *bwrap.Config, name string, - setpgid bool, wt io.WriterTo, - argF func(argsFD, statFD int) []string, + stat bool, + argF func(argsFd, statFd int) []string, cmdF func(cmd *exec.Cmd), + conf *bwrap.Config, + setpgid bool, extraFiles []*os.File, syncFd *os.File, - stat bool, ) Helper { - b, err := NewBwrap(ctx, conf, name, setpgid, wt, argF, cmdF, extraFiles, syncFd, stat) + b, err := NewBwrap(ctx, name, wt, stat, argF, cmdF, conf, setpgid, extraFiles, syncFd) if err != nil { panic(err.Error()) } else { @@ -74,20 +74,20 @@ func MustNewBwrap( // Function argF returns an array of arguments passed directly to the child process. func NewBwrap( ctx context.Context, - conf *bwrap.Config, name string, - setpgid bool, wt io.WriterTo, + stat bool, argF func(argsFd, statFd int) []string, cmdF func(cmd *exec.Cmd), + conf *bwrap.Config, + setpgid bool, extraFiles []*os.File, syncFd *os.File, - stat bool, ) (Helper, error) { b := new(bubblewrap) b.name = name - b.helperCmd = newHelperCmd(ctx, BubblewrapName, wt, argF, extraFiles, stat) + b.helperCmd = newHelperCmd(ctx, BubblewrapName, wt, argF, stat, extraFiles) if setpgid { b.Cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} } diff --git a/helper/bwrap_test.go b/helper/bwrap_test.go index 4b42356..08b383e 100644 --- a/helper/bwrap_test.go +++ b/helper/bwrap_test.go @@ -33,9 +33,10 @@ func TestBwrap(t *testing.T) { h := helper.MustNewBwrap( context.Background(), - sc, "fortify", false, - argsWt, argF, nil, - nil, nil, false, + "fortify", + argsWt, false, + argF, nil, + sc, false, nil, nil, ) if err := h.Start(); !errors.Is(err, os.ErrNotExist) { @@ -47,9 +48,10 @@ func TestBwrap(t *testing.T) { t.Run("valid new helper nil check", func(t *testing.T) { if got := helper.MustNewBwrap( context.TODO(), - sc, "fortify", false, - argsWt, argF, nil, - nil, nil, false, + "fortify", + argsWt, false, + argF, nil, + sc, false, nil, nil, ); got == nil { t.Errorf("MustNewBwrap(%#v, %#v, %#v) got nil", sc, argsWt, "fortify") @@ -68,22 +70,24 @@ func TestBwrap(t *testing.T) { helper.MustNewBwrap( context.TODO(), - &bwrap.Config{Hostname: "\x00"}, "fortify", false, - nil, argF, nil, - nil, nil, false, + "fortify", + nil, false, + argF, nil, + &bwrap.Config{Hostname: "\x00"}, false, nil, nil, ) }) t.Run("start without pipes", func(t *testing.T) { helper.InternalReplaceExecCommand(t) - c, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() stdout, stderr := new(strings.Builder), new(strings.Builder) h := helper.MustNewBwrap( - c, sc, "crash-test-dummy", false, - nil, argFChecked, func(cmd *exec.Cmd) { cmd.Stdout, cmd.Stderr = stdout, stderr }, - nil, nil, false, + ctx, "crash-test-dummy", + nil, false, + argFChecked, func(cmd *exec.Cmd) { cmd.Stdout, cmd.Stderr = stdout, stderr }, + sc, false, nil, nil, ) if err := h.Start(); err != nil { @@ -101,8 +105,10 @@ func TestBwrap(t *testing.T) { t.Run("implementation compliance", func(t *testing.T) { testHelper(t, func(ctx context.Context, cmdF func(cmd *exec.Cmd), stat bool) helper.Helper { return helper.MustNewBwrap( - ctx, sc, "crash-test-dummy", false, - argsWt, argF, cmdF, nil, nil, stat, + ctx, "crash-test-dummy", + argsWt, stat, + argF, cmdF, + sc, false, nil, nil, ) }) }) diff --git a/helper/direct.go b/helper/direct.go index 16aede8..74dd46e 100644 --- a/helper/direct.go +++ b/helper/direct.go @@ -17,14 +17,14 @@ import ( // Function argF returns an array of arguments passed directly to the child process. func NewDirect( ctx context.Context, - wt io.WriterTo, name string, + wt io.WriterTo, + stat bool, argF func(argsFd, statFd int) []string, cmdF func(cmd *exec.Cmd), - stat bool, ) Helper { d := new(direct) - d.helperCmd = newHelperCmd(ctx, name, wt, argF, nil, stat) + d.helperCmd = newHelperCmd(ctx, name, wt, argF, stat, nil) if cmdF != nil { cmdF(d.helperCmd.Cmd) } @@ -53,9 +53,12 @@ func (h *direct) Start() error { } func newHelperCmd( - ctx context.Context, name string, - wt io.WriterTo, argF func(argsFd, statFd int) []string, - extraFiles []*os.File, stat bool, + ctx context.Context, + name string, + wt io.WriterTo, + argF func(argsFd, statFd int) []string, + stat bool, + extraFiles []*os.File, ) (cmd *helperCmd) { cmd = new(helperCmd) cmd.ctx = ctx diff --git a/helper/direct_test.go b/helper/direct_test.go index 7af0e3a..6431ede 100644 --- a/helper/direct_test.go +++ b/helper/direct_test.go @@ -12,7 +12,7 @@ import ( func TestDirect(t *testing.T) { t.Run("start non-existent helper path", func(t *testing.T) { - h := helper.NewDirect(context.Background(), argsWt, "/nonexistent", argF, nil, false) + h := helper.NewDirect(context.Background(), "/nonexistent", argsWt, false, argF, nil) if err := h.Start(); !errors.Is(err, os.ErrNotExist) { t.Errorf("Start: error = %v, wantErr %v", @@ -21,7 +21,7 @@ func TestDirect(t *testing.T) { }) t.Run("valid new helper nil check", func(t *testing.T) { - if got := helper.NewDirect(context.TODO(), argsWt, "fortify", argF, nil, false); got == nil { + if got := helper.NewDirect(context.TODO(), "fortify", argsWt, false, argF, nil); got == nil { t.Errorf("New(%q, %q) got nil", argsWt, "fortify") return @@ -30,7 +30,7 @@ func TestDirect(t *testing.T) { t.Run("implementation compliance", func(t *testing.T) { testHelper(t, func(ctx context.Context, cmdF func(cmd *exec.Cmd), stat bool) helper.Helper { - return helper.NewDirect(ctx, argsWt, "crash-test-dummy", argF, cmdF, stat) + return helper.NewDirect(ctx, "crash-test-dummy", argsWt, stat, argF, cmdF) }) }) } diff --git a/internal/app/shim/main.go b/internal/app/shim/main.go index 5690f8f..c7c853b 100644 --- a/internal/app/shim/main.go +++ b/internal/app/shim/main.go @@ -128,13 +128,11 @@ func Main() { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() // unreachable if b, err := helper.NewBwrap( - ctx, - conf, path.Join(fst.Tmp, "sbin/init0"), false, - nil, func(int, int) []string { return make([]string, 0) }, + ctx, path.Join(fst.Tmp, "sbin/init0"), + nil, false, + func(int, int) []string { return make([]string, 0) }, func(cmd *exec.Cmd) { cmd.Stdin, cmd.Stdout, cmd.Stderr = os.Stdin, os.Stdout, os.Stderr }, - extraFiles, - syncFd, - false, + conf, false, extraFiles, syncFd, ); err != nil { log.Fatalf("malformed sandbox config: %v", err) } else {