dbus: use generalised helper.Helper for xdg-dbus-proxy

Signed-off-by: Ophestra Umiker <cat@ophivana.moe>

dbus/dbus.go

@@ -4,8 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
-	"os/exec"
 	"sync"
 
 	"git.ophivana.moe/cat/fortify/helper"
@@ -14,19 +12,12 @@ import (
 // Proxy holds references to a xdg-dbus-proxy process, and should never be copied.
 // Once sealed, configuration changes will no longer be possible and attempting to do so will result in a panic.
 type Proxy struct {
-	cmd *exec.Cmd
+	helper *helper.Helper
-
-	statP [2]*os.File
-	argsP [2]*os.File
 
 	path    string
 	session [2]string
 	system  [2]string
 
-	wait  *chan error
-	read  *chan error
-	ready *chan bool
-
 	seal io.WriterTo
 	lock sync.RWMutex
 }
@@ -39,8 +30,8 @@ func (p *Proxy) String() string {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 
-	if p.cmd != nil {
+	if p.helper != nil {
-		return p.cmd.String()
+		return p.helper.String()
 	}
 
 	if p.seal != nil {

dbus/run.go

@@ -3,12 +3,13 @@ package dbus
 import (
 	"errors"
 	"os"
-	"os/exec"
+
+	"git.ophivana.moe/cat/fortify/helper"
 )
 
 // Start launches the D-Bus proxy and sets up the Wait method.
 // ready should be buffered and should only be received from once.
-func (p *Proxy) Start(ready *chan bool) error {
+func (p *Proxy) Start(ready chan error) error {
 	p.lock.Lock()
 	defer p.lock.Unlock()
 
@@ -16,76 +17,21 @@ func (p *Proxy) Start(ready *chan bool) error {
 		return errors.New("proxy not sealed")
 	}
 
-	// acquire pipes
+	h := helper.New(p.seal, p.path,
-	if pr, pw, err := os.Pipe(); err != nil {
+		// Helper: Args is always 3 and status if set is always 4.
-		return err
+		"--args=3",
-	} else {
+		"--fd=4",
-		p.statP[0], p.statP[1] = pr, pw
-	}
-	if pr, pw, err := os.Pipe(); err != nil {
-		return err
-	} else {
-		p.argsP[0], p.argsP[1] = pr, pw
-	}
-
-	p.cmd = exec.Command(p.path,
-		// ExtraFiles: If non-nil, entry i becomes file descriptor 3+i.
-		"--fd=3",
-		"--args=4",
 	)
-	p.cmd.Env = []string{}
+	// xdg-dbus-proxy does not need to inherit the environment
-	p.cmd.ExtraFiles = []*os.File{p.statP[1], p.argsP[0]}
+	h.Env = []string{}
-	p.cmd.Stdout = os.Stdout
+
-	p.cmd.Stderr = os.Stderr
+	h.Stdout = os.Stdout
-	if err := p.cmd.Start(); err != nil {
+	h.Stderr = os.Stderr
+	if err := h.StartNotify(ready); err != nil {
 		return err
 	}
 
-	statsP, argsP := p.statP[0], p.argsP[1]
+	p.helper = h
-
-	if _, err := p.seal.WriteTo(argsP); err != nil {
-		if err1 := p.cmd.Process.Kill(); err1 != nil {
-			panic(err1)
-		}
-		return err
-	} else {
-		if err = argsP.Close(); err != nil {
-			if err1 := p.cmd.Process.Kill(); err1 != nil {
-				panic(err1)
-			}
-			return err
-		}
-	}
-
-	wait := make(chan error)
-	go func() {
-		// live out the lifespan of the process
-		wait <- p.cmd.Wait()
-	}()
-
-	read := make(chan error)
-	go func() {
-		n, err := statsP.Read(make([]byte, 1))
-		switch n {
-		case -1:
-			if err1 := p.cmd.Process.Kill(); err1 != nil {
-				panic(err1)
-			}
-			read <- err
-		case 0:
-			read <- err
-		case 1:
-			*ready <- true
-			read <- nil
-		default:
-			panic("unreachable") // unexpected read count
-		}
-	}()
-
-	p.wait = &wait
-	p.read = &read
-	p.ready = ready
-
 	return nil
 }
 
@@ -94,41 +40,14 @@ func (p *Proxy) Wait() error {
 	p.lock.RLock()
 	defer p.lock.RUnlock()
 
-	if p.wait == nil || p.read == nil {
+	if p.helper == nil {
-		return errors.New("proxy not running")
+		return errors.New("proxy not started")
 	}
 
-	defer func() {
+	return p.helper.Wait()
-		if err1 := p.statP[0].Close(); err1 != nil && !errors.Is(err1, os.ErrClosed) {
-			panic(err1)
-		}
-		if err1 := p.statP[1].Close(); err1 != nil && !errors.Is(err1, os.ErrClosed) {
-			panic(err1)
-		}
-
-		if err1 := p.argsP[0].Close(); err1 != nil && !errors.Is(err1, os.ErrClosed) {
-			panic(err1)
-		}
-		if err1 := p.argsP[1].Close(); err1 != nil && !errors.Is(err1, os.ErrClosed) {
-			panic(err1)
-		}
-
-	}()
-
-	select {
-	case err := <-*p.wait:
-		*p.ready <- false
-		return err
-	case err := <-*p.read:
-		if err != nil {
-			*p.ready <- false
-			return err
-		}
-		return <-*p.wait
-	}
 }
 
 // Close closes the status file descriptor passed to xdg-dbus-proxy, causing it to stop.
 func (p *Proxy) Close() error {
-	return p.statP[0].Close()
+	return p.helper.Close()
 }

internal/app/share.dbus.go

@@ -23,7 +23,6 @@ const (
 var (
 	ErrDBusConfig = errors.New("dbus config not supplied")
 	ErrDBusProxy  = errors.New(xdgDBusProxy + " not found")
-	ErrDBusFault  = errors.New(xdgDBusProxy + " did not start correctly")
 )
 
 type (
@@ -98,12 +97,12 @@ func (seal *appSeal) shareDBus(config [2]*dbus.Config) error {
 
 func (tx *appSealTx) startDBus() error {
 	// ready channel passed to dbus package
-	ready := make(chan bool, 1)
+	ready := make(chan error, 1)
 	// used by waiting goroutine to notify process return
 	tx.dbusWait = make(chan struct{})
 
 	// background dbus proxy start
-	if err := tx.dbus.Start(&ready); err != nil {
+	if err := tx.dbus.Start(ready); err != nil {
 		return (*StartDBusError)(wrapError(err, "cannot start message bus proxy:", err))
 	}
 	verbose.Println("starting message bus proxy:", tx.dbus)
@@ -130,9 +129,10 @@ func (tx *appSealTx) startDBus() error {
 		tx.dbusWait <- struct{}{}
 	}()
 
-	// ready is false if the proxy process faulted
+	// ready is not nil if the proxy process faulted
-	if !<-ready {
+	if err := <-ready; err != nil {
-		return (*StartDBusError)(wrapError(ErrDBusFault, "message bus proxy failed"))
+		// note that err here is either an I/O related error or a predetermined unexpected behaviour error
+		return (*StartDBusError)(wrapError(err, "message bus proxy fault after start:", err))
 	}
 	verbose.Println("message bus proxy ready")