helper: use generic extra files interface

This replaces the pipes object and integrates context into helper process lifecycle.

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

dbus/dbus_test.go

@@ -1,9 +1,11 @@
 package dbus_test
 
 import (
+	"context"
 	"errors"
 	"strings"
 	"testing"
+	"time"
 
 	"git.gensokyo.uk/security/fortify/dbus"
 	"git.gensokyo.uk/security/fortify/helper"
@@ -141,7 +143,7 @@ func testProxyStartWaitCloseString(t *testing.T, sandbox bool) {
 
 				t.Run("unsealed start of "+id, func(t *testing.T) {
 					want := "proxy not sealed"
-					if err := p.Start(nil, nil, sandbox, false); err == nil || err.Error() != want {
+					if err := p.Start(context.Background(), nil, sandbox); err == nil || err.Error() != want {
 						t.Errorf("Start() error = %v, wantErr %q",
 							err, errors.New(want))
 						return
@@ -149,7 +151,7 @@ func testProxyStartWaitCloseString(t *testing.T, sandbox bool) {
 				})
 
 				t.Run("unsealed wait of "+id, func(t *testing.T) {
-					wantErr := "proxy not started"
+					wantErr := "dbus: not started"
 					if err := p.Wait(); err == nil || err.Error() != wantErr {
 						t.Errorf("Wait() error = %v, wantErr %v",
 							err, wantErr)
@@ -175,7 +177,10 @@ func testProxyStartWaitCloseString(t *testing.T, sandbox bool) {
 				}
 
 				t.Run("sealed start of "+id, func(t *testing.T) {
-					if err := p.Start(nil, output, sandbox, false); err != nil {
+					ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+					defer cancel()
+
+					if err := p.Start(ctx, output, sandbox); err != nil {
 						t.Fatalf("Start(nil, nil) error = %v",
 							err)
 					}
@@ -189,22 +194,8 @@ func testProxyStartWaitCloseString(t *testing.T, sandbox bool) {
 						}
 					})
 
-					t.Run("sealed closing of "+id+" without status", func(t *testing.T) {
-						wantPanic := "attempted to close helper with no status pipe"
-						defer func() {
-							if r := recover(); r != wantPanic {
-								t.Errorf("Close() panic = %v, wantPanic %v",
-									r, wantPanic)
-							}
-						}()
-
-						if err := p.Close(); err != nil {
-							t.Errorf("Close() error = %v",
-								err)
-						}
-					})
-
 					t.Run("started wait of "+id, func(t *testing.T) {
+						p.Close()
 						if err := p.Wait(); err != nil {
 							t.Errorf("Wait() error = %v\noutput: %s",
 								err, output.String())

dbus/proxy.go

@@ -1,6 +1,7 @@
 package dbus
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -19,29 +20,21 @@ var ProxyName = "xdg-dbus-proxy"
 type Proxy struct {
 	helper helper.Helper
 	bwrap  *bwrap.Config
+	ctx    context.Context
+	cancel context.CancelCauseFunc
 
 	name    string
 	session [2]string
 	system  [2]string
+	sysP    bool
 
 	seal io.WriterTo
 	lock sync.RWMutex
 }
 
-func (p *Proxy) Session() [2]string {
-	return p.session
-}
-
-func (p *Proxy) System() [2]string {
-	return p.system
-}
-
-func (p *Proxy) Sealed() bool {
-	p.lock.RLock()
-	defer p.lock.RUnlock()
-
-	return p.seal != nil
-}
+func (p *Proxy) Session() [2]string { return p.session }
+func (p *Proxy) System() [2]string  { return p.system }
+func (p *Proxy) Sealed() bool       { p.lock.RLock(); defer p.lock.RUnlock(); return p.seal != nil }
 
 var (
 	ErrConfig = errors.New("no configuration to seal")
@@ -56,7 +49,7 @@ func (p *Proxy) String() string {
 	defer p.lock.RUnlock()
 
 	if p.helper != nil {
-		return p.helper.Unwrap().String()
+		return p.helper.String()
 	}
 
 	if p.seal != nil {
@@ -96,6 +89,7 @@ func (p *Proxy) Seal(session, system *Config) error {
 	}
 	if system != nil {
 		args = append(args, system.Args(p.system)...)
+		p.sysP = true
 	}
 	if seal, err := helper.NewCheckedArgs(args); err != nil {
 		return err

dbus/run.go

@@ -1,8 +1,10 @@
 package dbus
 
 import (
+	"context"
 	"errors"
 	"io"
+	"os"
 	"os/exec"
 	"path"
 	"path/filepath"
@@ -14,9 +16,8 @@ import (
 	"git.gensokyo.uk/security/fortify/ldd"
 )
 
-// Start launches the D-Bus proxy and sets up the Wait method.
-// ready should be buffered and must only be received from once.
-func (p *Proxy) Start(ready chan error, output io.Writer, sandbox, seccomp bool) error {
+// Start launches the D-Bus proxy.
+func (p *Proxy) Start(ctx context.Context, output io.Writer, sandbox bool) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 
@@ -25,8 +26,7 @@ func (p *Proxy) Start(ready chan error, output io.Writer, sandbox, seccomp bool)
 	}
 
 	var (
-		h   helper.Helper
-		cmd *exec.Cmd
+		h helper.Helper
 
 		argF = func(argsFD, statFD int) []string {
 			if statFD == -1 {
@@ -39,9 +39,8 @@ func (p *Proxy) Start(ready chan error, output io.Writer, sandbox, seccomp bool)
 
 	if !sandbox {
 		h = helper.New(p.seal, p.name, argF)
-		cmd = h.Unwrap()
 		// xdg-dbus-proxy does not need to inherit the environment
-		cmd.Env = []string{}
+		h.SetEnv(make([]string, 0))
 	} else {
 		// look up absolute path if name is just a file name
 		toolPath := p.name
@@ -56,7 +55,7 @@ func (p *Proxy) Start(ready chan error, output io.Writer, sandbox, seccomp bool)
 		// resolve libraries by parsing ldd output
 		var proxyDeps []*ldd.Entry
 		if toolPath != "/nonexistent-xdg-dbus-proxy" {
-			if l, err := ldd.Exec(toolPath); err != nil {
+			if l, err := ldd.Exec(ctx, toolPath); err != nil {
 				return err
 			} else {
 				proxyDeps = l
@@ -73,10 +72,6 @@ func (p *Proxy) Start(ready chan error, output io.Writer, sandbox, seccomp bool)
 			DieWithParent: true,
 		}
 
-		if !seccomp {
-			bc.Syscall = nil
-		}
-
 		// resolve proxy socket directories
 		bindTarget := make(map[string]struct{}, 2)
 		for _, ps := range []string{p.session[1], p.system[1]} {
@@ -116,35 +111,65 @@ func (p *Proxy) Start(ready chan error, output io.Writer, sandbox, seccomp bool)
 		}
 
 		h = helper.MustNewBwrap(bc, toolPath, p.seal, argF, nil, nil)
-		cmd = h.Unwrap()
 		p.bwrap = bc
 	}
 
 	if output != nil {
-		cmd.Stdout = output
-		cmd.Stderr = output
+		h.Stdout(output).Stderr(output)
 	}
-	if err := h.StartNotify(ready); err != nil {
+	c, cancel := context.WithCancelCause(ctx)
+	if err := h.Start(c, true); err != nil {
+		cancel(err)
 		return err
 	}
 
 	p.helper = h
+	p.ctx = c
+	p.cancel = cancel
 	return nil
 }
 
-// Wait waits for xdg-dbus-proxy to exit or fault.
+var proxyClosed = errors.New("proxy closed")
+
+// Wait blocks until xdg-dbus-proxy exits and releases resources.
 func (p *Proxy) Wait() error {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 
 	if p.helper == nil {
-		return errors.New("proxy not started")
+		return errors.New("dbus: not started")
 	}
 
-	return p.helper.Wait()
+	errs := make([]error, 3)
+
+	errs[0] = p.helper.Wait()
+	if p.cancel == nil &&
+		errors.Is(errs[0], context.Canceled) &&
+		errors.Is(context.Cause(p.ctx), proxyClosed) {
+		errs[0] = nil
+	}
+
+	// ensure socket removal so ephemeral directory is empty at revert
+	if err := os.Remove(p.session[1]); err != nil && !errors.Is(err, os.ErrNotExist) {
+		errs[1] = err
+	}
+	if p.sysP {
+		if err := os.Remove(p.system[1]); err != nil && !errors.Is(err, os.ErrNotExist) {
+			errs[2] = err
+		}
+	}
+
+	return errors.Join(errs...)
 }
 
-// Close closes the status file descriptor passed to xdg-dbus-proxy, causing it to stop.
-func (p *Proxy) Close() error {
-	return p.helper.Close()
+// Close cancels the context passed to the helper instance attached to xdg-dbus-proxy.
+func (p *Proxy) Close() {
+	p.lock.Lock()
+	defer p.lock.Unlock()
+
+	if p.cancel == nil {
+		panic("dbus: not started")
+	}
+	p.cancel(proxyClosed)
+	p.cancel = nil
 }