From 0f1f0e43643d302494c0dac8bbc3eda7813f06bb Mon Sep 17 00:00:00 2001
From: Ophestra <cat@gensokyo.uk>
Date: Sat, 15 Mar 2025 02:10:22 +0900
Subject: [PATCH] helper: combine helper ipc setup
The two-step args call is no longer necessary since stat is passed on initialisation.
Signed-off-by: Ophestra <cat@gensokyo.uk>
---
helper/bwrap.go | 47 ++-------
helper/bwrap_test.go | 2 +-
helper/cmd.go | 87 ++++++++++++++++
helper/{direct_test.go => cmd_test.go} | 0
helper/direct.go | 132 -------------------------
helper/helper.go | 58 +++++++++++
6 files changed, 155 insertions(+), 171 deletions(-)
create mode 100644 helper/cmd.go
rename helper/{direct_test.go => cmd_test.go} (100%)
delete mode 100644 helper/direct.go
diff --git a/helper/bwrap.go b/helper/bwrap.go
index 7e9e9bc..f94cf7f 100644
--- a/helper/bwrap.go
+++ b/helper/bwrap.go
@@ -2,13 +2,11 @@ package helper
import (
"context"
- "errors"
"io"
"os"
"os/exec"
"slices"
"strconv"
- "sync"
"syscall"
"git.gensokyo.uk/security/fortify/helper/bwrap"
@@ -18,34 +16,6 @@ import (
// BubblewrapName is the file name or path to bubblewrap.
var BubblewrapName = "bwrap"
-type bubblewrap struct {
- // final args fd of bwrap process
- argsFd uintptr
-
- // name of the command to run in bwrap
- name string
-
- lock sync.RWMutex
- *helperCmd
-}
-
-func (b *bubblewrap) Start() error {
- b.lock.Lock()
- defer b.lock.Unlock()
-
- // Check for doubled Start calls before we defer failure cleanup. If the prior
- // call to Start succeeded, we don't want to spuriously close its pipes.
- if b.Cmd != nil && b.Cmd.Process != nil {
- return errors.New("exec: already started")
- }
-
- args := b.finalise()
- b.Cmd.Args = slices.Grow(b.Cmd.Args, 4+len(args))
- b.Cmd.Args = append(b.Cmd.Args, "--args", strconv.Itoa(int(b.argsFd)), "--", b.name)
- b.Cmd.Args = append(b.Cmd.Args, args...)
- return proc.Fulfill(b.ctx, &b.ExtraFiles, b.Cmd.Start, b.files, b.extraFiles)
-}
-
// 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.
@@ -84,24 +54,25 @@ func NewBwrap(
extraFiles []*os.File,
syncFd *os.File,
) (Helper, error) {
- b := new(bubblewrap)
-
- b.name = name
- b.helperCmd = newHelperCmd(ctx, BubblewrapName, wt, argF, stat, extraFiles)
+ b, args := newHelperCmd(ctx, BubblewrapName, wt, stat, argF, extraFiles)
if setpgid {
b.Cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
}
- if cmdF != nil {
- cmdF(b.helperCmd.Cmd)
- }
+ var argsFd uintptr
if v, err := NewCheckedArgs(conf.Args(syncFd, b.extraFiles, &b.files)); err != nil {
return nil, err
} else {
f := proc.NewWriterTo(v)
- b.argsFd = proc.InitFile(f, b.extraFiles)
+ argsFd = proc.InitFile(f, b.extraFiles)
b.files = append(b.files, f)
}
+ b.Args = slices.Grow(b.Args, 4+len(args))
+ b.Args = append(b.Args, "--args", strconv.Itoa(int(argsFd)), "--", name)
+ b.Args = append(b.Args, args...)
+ if cmdF != nil {
+ cmdF(b.Cmd)
+ }
return b, nil
}
diff --git a/helper/bwrap_test.go b/helper/bwrap_test.go
index 08b383e..76f7cce 100644
--- a/helper/bwrap_test.go
+++ b/helper/bwrap_test.go
@@ -71,7 +71,7 @@ func TestBwrap(t *testing.T) {
helper.MustNewBwrap(
context.TODO(),
"fortify",
- nil, false,
+ argsWt, false,
argF, nil,
&bwrap.Config{Hostname: "\x00"}, false, nil, nil,
)
diff --git a/helper/cmd.go b/helper/cmd.go
new file mode 100644
index 0000000..422f971
--- /dev/null
+++ b/helper/cmd.go
@@ -0,0 +1,87 @@
+package helper
+
+import (
+ "context"
+ "errors"
+ "io"
+ "os"
+ "os/exec"
+ "slices"
+ "sync"
+ "syscall"
+
+ "git.gensokyo.uk/security/fortify/helper/proc"
+)
+
+// NewDirect initialises a new direct Helper instance with wt as the null-terminated argument writer.
+// Function argF returns an array of arguments passed directly to the child process.
+func NewDirect(
+ ctx context.Context,
+ name string,
+ wt io.WriterTo,
+ stat bool,
+ argF func(argsFd, statFd int) []string,
+ cmdF func(cmd *exec.Cmd),
+) Helper {
+ d, args := newHelperCmd(ctx, name, wt, stat, argF, nil)
+ d.Args = append(d.Args, args...)
+ if cmdF != nil {
+ cmdF(d.Cmd)
+ }
+ return d
+}
+
+func newHelperCmd(
+ ctx context.Context,
+ name string,
+ wt io.WriterTo,
+ stat bool,
+ argF func(argsFd, statFd int) []string,
+ extraFiles []*os.File,
+) (cmd *helperCmd, args []string) {
+ cmd = new(helperCmd)
+ cmd.helperFiles, args = newHelperFiles(ctx, wt, stat, argF, extraFiles)
+ cmd.Cmd = commandContext(ctx, name)
+ cmd.Cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) }
+ cmd.WaitDelay = WaitDelay
+ return
+}
+
+// helperCmd provides a [exec.Cmd] wrapper around helper ipc.
+type helperCmd struct {
+ mu sync.RWMutex
+ *helperFiles
+ *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()
+
+ // Check for doubled Start calls before we defer failure cleanup. If the prior
+ // call to Start succeeded, we don't want to spuriously close its pipes.
+ if h.Cmd != nil && h.Cmd.Process != nil {
+ return errors.New("exec: already started")
+ }
+
+ h.finalise()
+ return proc.Fulfill(h.helperFiles.ctx, &h.ExtraFiles, h.Cmd.Start, h.files, h.extraFiles)
+}
diff --git a/helper/direct_test.go b/helper/cmd_test.go
similarity index 100%
rename from helper/direct_test.go
rename to helper/cmd_test.go
diff --git a/helper/direct.go b/helper/direct.go
deleted file mode 100644
index 74dd46e..0000000
--- a/helper/direct.go
+++ /dev/null
@@ -1,132 +0,0 @@
-package helper
-
-import (
- "context"
- "errors"
- "io"
- "os"
- "os/exec"
- "slices"
- "sync"
- "syscall"
-
- "git.gensokyo.uk/security/fortify/helper/proc"
-)
-
-// NewDirect initialises a new direct Helper instance with wt as the null-terminated argument writer.
-// Function argF returns an array of arguments passed directly to the child process.
-func NewDirect(
- ctx context.Context,
- name string,
- wt io.WriterTo,
- stat bool,
- argF func(argsFd, statFd int) []string,
- cmdF func(cmd *exec.Cmd),
-) Helper {
- d := new(direct)
- d.helperCmd = newHelperCmd(ctx, name, wt, argF, stat, nil)
- if cmdF != nil {
- cmdF(d.helperCmd.Cmd)
- }
- return d
-}
-
-// direct starts the helper directly and manages status and args fd.
-type direct struct {
- lock sync.RWMutex
- *helperCmd
-}
-
-func (h *direct) Start() error {
- h.lock.Lock()
- defer h.lock.Unlock()
-
- // Check for doubled Start calls before we defer failure cleanup. If the prior
- // call to Start succeeded, we don't want to spuriously close its pipes.
- if h.Cmd != nil && h.Cmd.Process != nil {
- return errors.New("exec: already started")
- }
-
- args := h.finalise()
- h.Cmd.Args = append(h.Cmd.Args, args...)
- return proc.Fulfill(h.ctx, &h.ExtraFiles, h.Cmd.Start, h.files, h.extraFiles)
-}
-
-func newHelperCmd(
- 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
- cmd.hasStatFd = stat
-
- cmd.Cmd = commandContext(ctx, name)
- cmd.Cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) }
- cmd.WaitDelay = WaitDelay
-
- cmd.extraFiles = new(proc.ExtraFilesPre)
- for _, f := range extraFiles {
- _, v := cmd.extraFiles.Append()
- *v = f
- }
-
- argsFd := -1
- if wt != nil {
- f := proc.NewWriterTo(wt)
- argsFd = int(proc.InitFile(f, cmd.extraFiles))
- cmd.files = append(cmd.files, f)
- cmd.hasArgsFd = true
- }
- cmd.argF = func(statFd int) []string { return argF(argsFd, statFd) }
-
- return
-}
-
-// helperCmd wraps Cmd and implements methods shared across all Helper implementations.
-type helperCmd struct {
- // returns an array of arguments passed directly
- // to the helper process
- argF func(statFd int) []string
- // whether argsFd is present
- hasArgsFd bool
- // whether statFd is present
- hasStatFd bool
-
- // closes statFd
- stat io.Closer
- // deferred extraFiles fulfillment
- files []proc.File
- // passed through to [proc.Fulfill] and [proc.InitFile]
- extraFiles *proc.ExtraFilesPre
-
- ctx context.Context
- *exec.Cmd
-}
-
-// finalise sets up the underlying [exec.Cmd] object.
-func (h *helperCmd) finalise() (args []string) {
- h.Env = slices.Grow(h.Env, 2)
- if h.hasArgsFd {
- h.Cmd.Env = append(h.Env, FortifyHelper+"=1")
- } else {
- h.Cmd.Env = append(h.Env, FortifyHelper+"=0")
- }
-
- statFd := -1
- if h.hasStatFd {
- f := proc.NewStat(&h.stat)
- statFd = int(proc.InitFile(f, h.extraFiles))
- h.files = append(h.files, f)
- 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")
- }
- return h.argF(statFd)
-}
diff --git a/helper/helper.go b/helper/helper.go
index ab27644..ac45ac7 100644
--- a/helper/helper.go
+++ b/helper/helper.go
@@ -2,9 +2,14 @@
package helper
import (
+ "context"
"fmt"
+ "io"
+ "os"
"os/exec"
"time"
+
+ "git.gensokyo.uk/security/fortify/helper/proc"
)
var (
@@ -28,3 +33,56 @@ type Helper interface {
fmt.Stringer
}
+
+func newHelperFiles(
+ ctx context.Context,
+ wt io.WriterTo,
+ stat bool,
+ argF func(argsFd, statFd int) []string,
+ extraFiles []*os.File,
+) (hl *helperFiles, args []string) {
+ hl = new(helperFiles)
+ hl.ctx = ctx
+ hl.useArgsFd = wt != nil
+ hl.useStatFd = stat
+
+ hl.extraFiles = new(proc.ExtraFilesPre)
+ for _, f := range extraFiles {
+ _, v := hl.extraFiles.Append()
+ *v = f
+ }
+
+ argsFd := -1
+ if hl.useArgsFd {
+ f := proc.NewWriterTo(wt)
+ argsFd = int(proc.InitFile(f, hl.extraFiles))
+ hl.files = append(hl.files, f)
+ }
+
+ statFd := -1
+ if hl.useStatFd {
+ f := proc.NewStat(&hl.stat)
+ statFd = int(proc.InitFile(f, hl.extraFiles))
+ hl.files = append(hl.files, f)
+ }
+
+ args = argF(argsFd, statFd)
+ return
+}
+
+// helperFiles provides a generic wrapper around helper ipc.
+type helperFiles struct {
+ // whether argsFd is present
+ useArgsFd bool
+ // whether statFd is present
+ useStatFd bool
+
+ // closes statFd
+ stat io.Closer
+ // deferred extraFiles fulfillment
+ files []proc.File
+ // passed through to [proc.Fulfill] and [proc.InitFile]
+ extraFiles *proc.ExtraFilesPre
+
+ ctx context.Context
+}