From ace97952ccd08afe74737659794f43233d28596e Mon Sep 17 00:00:00 2001 From: Ophestra Date: Fri, 14 Feb 2025 18:13:06 +0900 Subject: [PATCH] helper/bwrap: merge Args and FDArgs Signed-off-by: Ophestra --- helper/bwrap.go | 4 +-- helper/bwrap/arg.go | 52 +++++++++++++------------------------ helper/bwrap/config_test.go | 20 ++++++++------ helper/stub.go | 5 ++-- 4 files changed, 33 insertions(+), 48 deletions(-) diff --git a/helper/bwrap.go b/helper/bwrap.go index 8044e6a..598f526 100644 --- a/helper/bwrap.go +++ b/helper/bwrap.go @@ -75,9 +75,7 @@ func NewBwrap( b.name = name b.helperCmd = newHelperCmd(b, BubblewrapName, wt, argF, extraFiles) - args := conf.Args() - conf.FDArgs(syncFd, &args, b.extraFiles, &b.files) - if v, err := NewCheckedArgs(args); err != nil { + if v, err := NewCheckedArgs(conf.Args(syncFd, b.extraFiles, &b.files)); err != nil { return nil, err } else { f := proc.NewWriterTo(v) diff --git a/helper/bwrap/arg.go b/helper/bwrap/arg.go index 050643e..f2012cd 100644 --- a/helper/bwrap/arg.go +++ b/helper/bwrap/arg.go @@ -23,42 +23,22 @@ type FDBuilder interface { } // Args returns a slice of bwrap args corresponding to c. -func (c *Config) Args() (args []string) { +func (c *Config) Args(syncFd *os.File, extraFiles *proc.ExtraFilesPre, files *[]proc.File) (args []string) { builders := []Builder{ c.boolArgs(), c.intArgs(), c.stringArgs(), c.pairArgs(), - } - - // copy FSBuilder slice to builder slice - fb := make([]Builder, len(c.Filesystem)+1) - for i, f := range c.Filesystem { - fb[i] = f - } - fb[len(fb)-1] = c.Chmod - builders = append(builders, fb...) - - // accumulate arg count - argc := 0 - for _, b := range builders { - argc += b.Len() - } - - args = make([]string, 0, argc) - for _, b := range builders { - b.Append(&args) - } - - return -} - -func (c *Config) FDArgs(syncFd *os.File, args *[]string, extraFiles *proc.ExtraFilesPre, files *[]proc.File) { - builders := []FDBuilder{ c.seccompArgs(), newFile(positionalArgs[SyncFd], syncFd), } + builders = slices.Grow(builders, len(c.Filesystem)+1) + for _, f := range c.Filesystem { + builders = append(builders, f) + } + builders = append(builders, c.Chmod) + argc := 0 fc := 0 for _, b := range builders { @@ -67,22 +47,26 @@ func (c *Config) FDArgs(syncFd *os.File, args *[]string, extraFiles *proc.ExtraF continue } argc += l - fc++ - proc.InitFile(b, extraFiles) + if f, ok := b.(FDBuilder); ok { + fc++ + proc.InitFile(f, extraFiles) + } } - fc++ // allocate extra slot for stat fd - *args = slices.Grow(*args, argc) - *files = slices.Grow(*files, fc) + args = make([]string, 0, argc) + *files = slices.Grow(*files, fc) for _, b := range builders { if b.Len() < 1 { continue } + b.Append(&args) - b.Append(args) - *files = append(*files, b) + if f, ok := b.(FDBuilder); ok { + *files = append(*files, f) + } } + return } diff --git a/helper/bwrap/config_test.go b/helper/bwrap/config_test.go index 7cf1b79..64d783c 100644 --- a/helper/bwrap/config_test.go +++ b/helper/bwrap/config_test.go @@ -6,9 +6,15 @@ import ( "testing" "git.gensokyo.uk/security/fortify/helper/bwrap" + "git.gensokyo.uk/security/fortify/helper/proc" + "git.gensokyo.uk/security/fortify/helper/seccomp" + "git.gensokyo.uk/security/fortify/internal/fmsg" ) func TestConfig_Args(t *testing.T) { + seccomp.CPrintln = fmsg.Println + t.Cleanup(func() { seccomp.CPrintln = nil }) + testCases := []struct { name string conf *bwrap.Config @@ -137,13 +143,14 @@ func TestConfig_Args(t *testing.T) { }, }, { - name: "hostname chdir setenv unsetenv lockfile chmod", + name: "hostname chdir setenv unsetenv lockfile chmod syscall", conf: &bwrap.Config{ Hostname: "fortify", Chdir: "/.fortify", SetEnv: map[string]string{"FORTIFY_INIT": "/.fortify/sbin/init"}, UnsetEnv: []string{"HOME", "HOST"}, LockFile: []string{"/.fortify/lock"}, + Syscall: new(bwrap.SyscallPolicy), Chmod: map[string]os.FileMode{"/.fortify/sbin/init": 0755}, }, want: []string{ @@ -160,6 +167,8 @@ func TestConfig_Args(t *testing.T) { "--lock-file", "/.fortify/lock", // SetEnv: map[string]string{"FORTIFY_INIT": "/.fortify/sbin/init"} "--setenv", "FORTIFY_INIT", "/.fortify/sbin/init", + // Syscall: new(bwrap.SyscallPolicy), + "--seccomp", "3", // Chmod: map[string]os.FileMode{"/.fortify/sbin/init": 0755} "--chmod", "755", "/.fortify/sbin/init", }, @@ -167,12 +176,7 @@ func TestConfig_Args(t *testing.T) { { name: "xdg-dbus-proxy constraint sample", - conf: (&bwrap.Config{ - Unshare: nil, - UserNS: false, - Clearenv: true, - DieWithParent: true, - }). + conf: (&bwrap.Config{Clearenv: true, DieWithParent: true}). Symlink("usr/bin", "/bin"). Symlink("var/home", "/home"). Symlink("usr/lib", "/lib"). @@ -227,7 +231,7 @@ func TestConfig_Args(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - if got := tc.conf.Args(); !slices.Equal(got, tc.want) { + if got := tc.conf.Args(nil, new(proc.ExtraFilesPre), new([]proc.File)); !slices.Equal(got, tc.want) { t.Errorf("Args() = %#v, want %#v", got, tc.want) } }) diff --git a/helper/stub.go b/helper/stub.go index 830505c..5891302 100644 --- a/helper/stub.go +++ b/helper/stub.go @@ -155,9 +155,8 @@ func bwrapStub() { DieWithParent: true, AsInit: true, } - args := sc.Args() - sc.FDArgs(nil, &args, new(proc.ExtraFilesPre), new([]proc.File)) - if _, err := MustNewCheckedArgs(args).WriteTo(want); err != nil { + if _, err := MustNewCheckedArgs(sc.Args(nil, new(proc.ExtraFilesPre), new([]proc.File))). + WriteTo(want); err != nil { panic("cannot read want: " + err.Error()) }