From 10a21ce3ef0566194d874b9da9b437971e968133 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Sat, 15 Mar 2025 02:21:59 +0900 Subject: [PATCH] helper: expose extra files to direct Signed-off-by: Ophestra --- dbus/run.go | 7 +++++-- helper/bwrap.go | 12 +++--------- helper/bwrap_test.go | 15 ++++++++++----- helper/cmd.go | 37 +++++++++++++++++-------------------- helper/cmd_test.go | 6 +++--- internal/app/shim/main.go | 3 ++- 6 files changed, 40 insertions(+), 40 deletions(-) diff --git a/dbus/run.go b/dbus/run.go index e1f54fb..30b06f4 100644 --- a/dbus/run.go +++ b/dbus/run.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strconv" "strings" + "syscall" "git.gensokyo.uk/security/fortify/helper" "git.gensokyo.uk/security/fortify/helper/bwrap" @@ -40,13 +41,14 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, sandbox bool) error c, cancel := context.WithCancelCause(ctx) if !sandbox { h = helper.NewDirect(c, p.name, p.seal, true, argF, func(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} if output != nil { cmd.Stdout, cmd.Stderr = output, output } // xdg-dbus-proxy does not need to inherit the environment cmd.Env = make([]string, 0) - }) + }, nil) } else { // look up absolute path if name is just a file name toolPath := p.name @@ -117,10 +119,11 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, sandbox bool) error } h = helper.MustNewBwrap(c, toolPath, p.seal, true, argF, func(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} if output != nil { cmd.Stdout, cmd.Stderr = output, output } - }, bc, true, nil, nil) + }, nil, bc, nil) p.bwrap = bc } diff --git a/helper/bwrap.go b/helper/bwrap.go index f94cf7f..97eacf8 100644 --- a/helper/bwrap.go +++ b/helper/bwrap.go @@ -7,7 +7,6 @@ import ( "os/exec" "slices" "strconv" - "syscall" "git.gensokyo.uk/security/fortify/helper/bwrap" "git.gensokyo.uk/security/fortify/helper/proc" @@ -26,12 +25,11 @@ func MustNewBwrap( stat bool, argF func(argsFd, statFd int) []string, cmdF func(cmd *exec.Cmd), - conf *bwrap.Config, - setpgid bool, extraFiles []*os.File, + conf *bwrap.Config, syncFd *os.File, ) Helper { - b, err := NewBwrap(ctx, name, wt, stat, argF, cmdF, conf, setpgid, extraFiles, syncFd) + b, err := NewBwrap(ctx, name, wt, stat, argF, cmdF, extraFiles, conf, syncFd) if err != nil { panic(err.Error()) } else { @@ -49,15 +47,11 @@ func NewBwrap( stat bool, argF func(argsFd, statFd int) []string, cmdF func(cmd *exec.Cmd), - conf *bwrap.Config, - setpgid bool, extraFiles []*os.File, + conf *bwrap.Config, syncFd *os.File, ) (Helper, error) { b, args := newHelperCmd(ctx, BubblewrapName, wt, stat, argF, extraFiles) - if setpgid { - b.Cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - } var argsFd uintptr if v, err := NewCheckedArgs(conf.Args(syncFd, b.extraFiles, &b.files)); err != nil { diff --git a/helper/bwrap_test.go b/helper/bwrap_test.go index 76f7cce..550ff34 100644 --- a/helper/bwrap_test.go +++ b/helper/bwrap_test.go @@ -36,7 +36,8 @@ func TestBwrap(t *testing.T) { "fortify", argsWt, false, argF, nil, - sc, false, nil, nil, + nil, + sc, nil, ) if err := h.Start(); !errors.Is(err, os.ErrNotExist) { @@ -51,7 +52,8 @@ func TestBwrap(t *testing.T) { "fortify", argsWt, false, argF, nil, - sc, false, nil, nil, + nil, + sc, nil, ); got == nil { t.Errorf("MustNewBwrap(%#v, %#v, %#v) got nil", sc, argsWt, "fortify") @@ -73,7 +75,8 @@ func TestBwrap(t *testing.T) { "fortify", argsWt, false, argF, nil, - &bwrap.Config{Hostname: "\x00"}, false, nil, nil, + nil, + &bwrap.Config{Hostname: "\x00"}, nil, ) }) @@ -87,7 +90,8 @@ func TestBwrap(t *testing.T) { ctx, "crash-test-dummy", nil, false, argFChecked, func(cmd *exec.Cmd) { cmd.Stdout, cmd.Stderr = stdout, stderr }, - sc, false, nil, nil, + nil, + sc, nil, ) if err := h.Start(); err != nil { @@ -108,7 +112,8 @@ func TestBwrap(t *testing.T) { ctx, "crash-test-dummy", argsWt, stat, argF, cmdF, - sc, false, nil, nil, + nil, + sc, nil, ) }) }) diff --git a/helper/cmd.go b/helper/cmd.go index 422f971..5b66afe 100644 --- a/helper/cmd.go +++ b/helper/cmd.go @@ -22,8 +22,9 @@ func NewDirect( stat bool, argF func(argsFd, statFd int) []string, cmdF func(cmd *exec.Cmd), + extraFiles []*os.File, ) Helper { - d, args := newHelperCmd(ctx, name, wt, stat, argF, nil) + d, args := newHelperCmd(ctx, name, wt, stat, argF, extraFiles) d.Args = append(d.Args, args...) if cmdF != nil { cmdF(d.Cmd) @@ -54,24 +55,6 @@ type helperCmd struct { *exec.Cmd } -// finalise sets up the underlying [exec.Cmd] object. -func (h *helperCmd) finalise() { - h.Env = slices.Grow(h.Env, 2) - if h.useArgsFd { - h.Cmd.Env = append(h.Env, FortifyHelper+"=1") - } else { - h.Cmd.Env = append(h.Env, FortifyHelper+"=0") - } - if h.useStatFd { - h.Cmd.Env = append(h.Cmd.Env, FortifyStatus+"=1") - - // stat is populated on fulfill - h.Cmd.Cancel = func() error { return h.stat.Close() } - } else { - h.Cmd.Env = append(h.Cmd.Env, FortifyStatus+"=0") - } -} - func (h *helperCmd) Start() error { h.mu.Lock() defer h.mu.Unlock() @@ -82,6 +65,20 @@ func (h *helperCmd) Start() error { return errors.New("exec: already started") } - h.finalise() + h.Env = slices.Grow(h.Env, 2) + if h.useArgsFd { + h.Env = append(h.Env, FortifyHelper+"=1") + } else { + h.Env = append(h.Env, FortifyHelper+"=0") + } + if h.useStatFd { + h.Env = append(h.Env, FortifyStatus+"=1") + + // stat is populated on fulfill + h.Cancel = func() error { return h.stat.Close() } + } else { + h.Env = append(h.Env, FortifyStatus+"=0") + } + return proc.Fulfill(h.helperFiles.ctx, &h.ExtraFiles, h.Cmd.Start, h.files, h.extraFiles) } diff --git a/helper/cmd_test.go b/helper/cmd_test.go index 6431ede..60c0caf 100644 --- a/helper/cmd_test.go +++ b/helper/cmd_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(), "/nonexistent", argsWt, false, argF, nil) + h := helper.NewDirect(context.Background(), "/nonexistent", argsWt, false, argF, nil, 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(), "fortify", argsWt, false, argF, nil); got == nil { + if got := helper.NewDirect(context.TODO(), "fortify", argsWt, false, argF, nil, 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, "crash-test-dummy", argsWt, stat, argF, cmdF) + return helper.NewDirect(ctx, "crash-test-dummy", argsWt, stat, argF, cmdF, nil) }) }) } diff --git a/internal/app/shim/main.go b/internal/app/shim/main.go index c7c853b..6cbb147 100644 --- a/internal/app/shim/main.go +++ b/internal/app/shim/main.go @@ -132,7 +132,8 @@ func Main() { 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 }, - conf, false, extraFiles, syncFd, + extraFiles, + conf, syncFd, ); err != nil { log.Fatalf("malformed sandbox config: %v", err) } else {