diff options
| author | Ophestra <cat@gensokyo.uk> | 2025-04-16 23:10:04 +0900 |
|---|---|---|
| committer | Ophestra <cat@gensokyo.uk> | 2025-04-16 23:35:17 +0900 |
| commit | 5979d8b1e0fc4249fb73de9b0ba099bae7707214 (patch) | |
| tree | ec3e9c859007005889cfad99158d3fd401587bf1 /dbus | |
| parent | e587112e6382aae80d46097c2e7636e22abdf917 (diff) | |
dbus: clean up wrapper implementation
The dbus proxy wrapper haven't been updated much ever since the helper interface was introduced.
Signed-off-by: Ophestra <cat@gensokyo.uk>
Diffstat (limited to 'dbus')
| -rw-r--r-- | dbus/config.go | 5 | ||||
| -rw-r--r-- | dbus/dbus_test.go | 222 | ||||
| -rw-r--r-- | dbus/export_test.go | 18 | ||||
| -rw-r--r-- | dbus/proc.go | 92 | ||||
| -rw-r--r-- | dbus/proxy.go | 95 |
5 files changed, 205 insertions, 227 deletions
diff --git a/dbus/config.go b/dbus/config.go index 160a7ca2..26fce463 100644 --- a/dbus/config.go +++ b/dbus/config.go @@ -7,6 +7,9 @@ import ( "os" ) +// ProxyPair is an upstream dbus address and a downstream socket path. +type ProxyPair [2]string + type Config struct { // See set 'see' policy for NAME (--see=NAME) See []string `json:"see"` @@ -24,7 +27,7 @@ type Config struct { Filter bool `json:"filter"` } -func (c *Config) Args(bus [2]string) (args []string) { +func (c *Config) Args(bus ProxyPair) (args []string) { argc := 2 + len(c.See) + len(c.Talk) + len(c.Own) + len(c.Call) + len(c.Broadcast) if c.Log { argc++ diff --git a/dbus/dbus_test.go b/dbus/dbus_test.go index 0917d9d1..206692be 100644 --- a/dbus/dbus_test.go +++ b/dbus/dbus_test.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "io" "os" "os/exec" "strings" @@ -19,64 +20,21 @@ import ( "git.gensokyo.uk/security/fortify/sandbox" ) -func TestNew(t *testing.T) { - for _, tc := range [][2][2]string{ - { - {"unix:path=/run/user/1971/bus", "/tmp/fortify.1971/1ca5d183ef4c99e74c3e544715f32702/bus"}, - {"unix:path=/run/dbus/system_bus_socket", "/tmp/fortify.1971/1ca5d183ef4c99e74c3e544715f32702/system_bus_socket"}, - }, - { - {"unix:path=/run/user/1971/bus", "/tmp/fortify.1971/881ac3796ff3f3bf0a773824383187a0/bus"}, - {"unix:path=/run/dbus/system_bus_socket", "/tmp/fortify.1971/881ac3796ff3f3bf0a773824383187a0/system_bus_socket"}, - }, - { - {"unix:path=/run/user/1971/bus", "/tmp/fortify.1971/3d1a5084520ef79c0c6a49a675bac701/bus"}, - {"unix:path=/run/dbus/system_bus_socket", "/tmp/fortify.1971/3d1a5084520ef79c0c6a49a675bac701/system_bus_socket"}, - }, - { - {"unix:path=/run/user/1971/bus", "/tmp/fortify.1971/2a1639bab712799788ea0ff7aa280c35/bus"}, - {"unix:path=/run/dbus/system_bus_socket", "/tmp/fortify.1971/2a1639bab712799788ea0ff7aa280c35/system_bus_socket"}, - }, - } { - t.Run("create instance for "+tc[0][0]+" and "+tc[1][0], func(t *testing.T) { - if got := dbus.New(tc[0], tc[1]); !got.CompareTestNew(tc[0], tc[1]) { - t.Errorf("New(%q, %q) = %v", - tc[0], tc[1], - got) - } - }) - } -} - -func TestProxy_Seal(t *testing.T) { - t.Run("double seal panic", func(t *testing.T) { - defer func() { - want := "dbus proxy sealed twice" - if r := recover(); r != want { - t.Errorf("Seal: panic = %q, want %q", - r, want) - } - }() - - p := dbus.New([2]string{}, [2]string{}) - _ = p.Seal(dbus.NewConfig("", true, false), nil) - _ = p.Seal(dbus.NewConfig("", true, false), nil) - }) - - ep := dbus.New([2]string{}, [2]string{}) - if err := ep.Seal(nil, nil); !errors.Is(err, dbus.ErrConfig) { - t.Errorf("Seal(nil, nil) error = %v, want %v", - err, dbus.ErrConfig) +func TestFinalise(t *testing.T) { + if _, err := dbus.Finalise(dbus.ProxyPair{}, dbus.ProxyPair{}, nil, nil); !errors.Is(err, syscall.EBADE) { + t.Errorf("Finalise: error = %v, want %v", + err, syscall.EBADE) } for id, tc := range testCasePairs() { - t.Run("create seal for "+id, func(t *testing.T) { - p := dbus.New(tc[0].bus, tc[1].bus) - if err := p.Seal(tc[0].c, tc[1].c); (errors.Is(err, syscall.EINVAL)) != tc[0].wantErr { - t.Errorf("Seal(%p, %p) error = %v, wantErr %v", - tc[0].c, tc[1].c, + t.Run("create final for "+id, func(t *testing.T) { + var wt io.WriterTo + if v, err := dbus.Finalise(tc[0].bus, tc[1].bus, tc[0].c, tc[1].c); (errors.Is(err, syscall.EINVAL)) != tc[0].wantErr { + t.Errorf("Finalise: error = %v, wantErr %v", err, tc[0].wantErr) return + } else { + wt = v } // rest of the tests happen for sealed instances @@ -89,25 +47,23 @@ func TestProxy_Seal(t *testing.T) { args := append(tc[0].want, tc[1].want...) for _, arg := range args { want.WriteString(arg) - want.WriteByte('\x00') + want.WriteByte(0) } - wt := p.AccessTestProxySeal() got := new(strings.Builder) if _, err := wt.WriteTo(got); err != nil { - t.Errorf("p.seal.WriteTo(): %v", err) + t.Errorf("WriteTo: error = %v", err) } if want.String() != got.String() { - t.Errorf("Seal(%p, %p) seal = %v, want %v", - tc[0].c, tc[1].c, + t.Errorf("Seal: %q, want %q", got.String(), want.String()) } }) } } -func TestProxy_Start_Wait_Close_String(t *testing.T) { +func TestProxyStartWaitCloseString(t *testing.T) { oldWaitDelay := helper.WaitDelay helper.WaitDelay = 16 * time.Second t.Cleanup(func() { helper.WaitDelay = oldWaitDelay }) @@ -116,29 +72,62 @@ func TestProxy_Start_Wait_Close_String(t *testing.T) { proxyName := dbus.ProxyName dbus.ProxyName = os.Args[0] t.Cleanup(func() { dbus.ProxyName = proxyName }) - testProxyStartWaitCloseString(t, true) + testProxyFinaliseStartWaitCloseString(t, true) }) - t.Run("direct", func(t *testing.T) { testProxyStartWaitCloseString(t, false) }) + t.Run("direct", func(t *testing.T) { testProxyFinaliseStartWaitCloseString(t, false) }) } -func testProxyStartWaitCloseString(t *testing.T, useSandbox bool) { +func testProxyFinaliseStartWaitCloseString(t *testing.T, useSandbox bool) { + var p *dbus.Proxy + + t.Run("string for nil proxy", func(t *testing.T) { + want := "(invalid dbus proxy)" + if got := p.String(); got != want { + t.Errorf("String: %q, want %q", + got, want) + } + }) + + t.Run("invalid start", func(t *testing.T) { + if !useSandbox { + p = dbus.NewDirect(context.TODO(), nil, nil) + } else { + p = dbus.New(context.TODO(), nil, nil) + } + + if err := p.Start(); !errors.Is(err, syscall.ENOTRECOVERABLE) { + t.Errorf("Start: error = %q, wantErr %q", + err, syscall.ENOTRECOVERABLE) + return + } + }) + for id, tc := range testCasePairs() { // this test does not test errors if tc[0].wantErr { continue } - t.Run("string for nil proxy", func(t *testing.T) { - var p *dbus.Proxy - want := "(invalid dbus proxy)" - if got := p.String(); got != want { - t.Errorf("String() = %v, want %v", - got, want) + t.Run("proxy for "+id, func(t *testing.T) { + var final *dbus.Final + t.Run("finalise", func(t *testing.T) { + if v, err := dbus.Finalise(tc[0].bus, tc[1].bus, tc[0].c, tc[1].c); err != nil { + t.Errorf("Finalise: error = %v, wantErr %v", + err, tc[0].wantErr) + return + } else { + final = v + } + }) + + ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Second) + defer cancel() + if !useSandbox { + p = dbus.NewDirect(ctx, final, nil) + } else { + p = dbus.New(ctx, final, nil) } - }) - t.Run("proxy for "+id, func(t *testing.T) { - p := dbus.New(tc[0].bus, tc[1].bus) p.CommandContext = func(ctx context.Context) (cmd *exec.Cmd) { return exec.CommandContext(ctx, os.Args[0], "-test.v", "-test.run=TestHelperInit", "--", "init") @@ -163,79 +152,52 @@ func testProxyStartWaitCloseString(t *testing.T, useSandbox bool) { p.FilterF = func(v []byte) []byte { return bytes.SplitN(v, []byte("TestHelperInit\n"), 2)[1] } output := new(strings.Builder) - t.Run("unsealed", func(t *testing.T) { - t.Run("string", func(t *testing.T) { - want := "(unsealed dbus proxy)" - if got := p.String(); got != want { - t.Errorf("String() = %v, want %v", - got, want) - return - } - }) - - t.Run("start", func(t *testing.T) { - want := "proxy not sealed" - if err := p.Start(context.Background(), nil, useSandbox); err == nil || err.Error() != want { - t.Errorf("Start() error = %v, wantErr %q", - err, errors.New(want)) - return - } - }) - - t.Run("wait", func(t *testing.T) { - wantErr := "dbus: not started" - if err := p.Wait(); err == nil || err.Error() != wantErr { - t.Errorf("Wait() error = %v, wantErr %v", - err, wantErr) - } - }) - }) - - t.Run("seal with "+id, func(t *testing.T) { - if err := p.Seal(tc[0].c, tc[1].c); err != nil { - t.Errorf("Seal(%p, %p) error = %v, wantErr %v", - tc[0].c, tc[1].c, - err, tc[0].wantErr) - return + t.Run("invalid wait", func(t *testing.T) { + wantErr := "dbus: not started" + if err := p.Wait(); err == nil || err.Error() != wantErr { + t.Errorf("Wait: error = %v, wantErr %v", + err, wantErr) } }) - t.Run("sealed", func(t *testing.T) { - want := strings.Join(append(tc[0].want, tc[1].want...), " ") + t.Run("string", func(t *testing.T) { + want := "(unused dbus proxy)" if got := p.String(); got != want { - t.Errorf("String() = %v, want %v", + t.Errorf("String: %q, want %q", got, want) return } + }) - t.Run("start", func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() + t.Run("start", func(t *testing.T) { + if err := p.Start(); err != nil { + t.Fatalf("Start: error = %v", + err) + } - if err := p.Start(ctx, output, useSandbox); err != nil { - t.Fatalf("Start(nil, nil) error = %v", - err) + t.Run("string", func(t *testing.T) { + wantSubstr := fmt.Sprintf("%s -test.run=TestHelperStub -- --args=3 --fd=4", os.Args[0]) + if useSandbox { + wantSubstr = fmt.Sprintf(`argv: ["%s" "-test.run=TestHelperStub" "--" "--args=3" "--fd=4"], flags: 0x0, seccomp: 0x3e`, os.Args[0]) } + if got := p.String(); !strings.Contains(got, wantSubstr) { + t.Errorf("String: %q, want %q", + got, wantSubstr) + return + } + }) - t.Run("string", func(t *testing.T) { - wantSubstr := fmt.Sprintf("%s -test.run=TestHelperStub -- --args=3 --fd=4", os.Args[0]) - if useSandbox { - wantSubstr = fmt.Sprintf(`argv: ["%s" "-test.run=TestHelperStub" "--" "--args=3" "--fd=4"], flags: 0x0, seccomp: 0x3e`, os.Args[0]) - } - if got := p.String(); !strings.Contains(got, wantSubstr) { - t.Errorf("String() = %v, want %v", - p.String(), wantSubstr) - return - } - }) - - t.Run("wait", func(t *testing.T) { - p.Close() + t.Run("wait", func(t *testing.T) { + done := make(chan struct{}) + go func() { if err := p.Wait(); err != nil { - t.Errorf("Wait() error = %v\noutput: %s", + t.Errorf("Wait: error = %v\noutput: %s", err, output.String()) } - }) + close(done) + }() + p.Close() + <-done }) }) }) diff --git a/dbus/export_test.go b/dbus/export_test.go index d42fde51..574a4a73 100644 --- a/dbus/export_test.go +++ b/dbus/export_test.go @@ -1,13 +1,13 @@ package dbus -import "io" +import ( + "context" + "io" +) -// CompareTestNew provides TestNew with comparison access to unexported Proxy fields. -func (p *Proxy) CompareTestNew(session, system [2]string) bool { - return session == p.session && system == p.system -} - -// AccessTestProxySeal provides TestProxy_Seal with access to unexported Proxy seal field. -func (p *Proxy) AccessTestProxySeal() io.WriterTo { - return p.seal +// NewDirect returns a new instance of [Proxy] with its sandbox disabled. +func NewDirect(ctx context.Context, final *Final, output io.Writer) *Proxy { + p := New(ctx, final, output) + p.useSandbox = false + return p } diff --git a/dbus/proc.go b/dbus/proc.go index cd087d5f..d6fe0b01 100644 --- a/dbus/proc.go +++ b/dbus/proc.go @@ -3,14 +3,12 @@ package dbus import ( "context" "errors" - "io" "os" "os/exec" "path" "path/filepath" "slices" "strconv" - "strings" "syscall" "git.gensokyo.uk/security/fortify/helper" @@ -19,25 +17,30 @@ import ( "git.gensokyo.uk/security/fortify/sandbox/seccomp" ) -// Start launches the D-Bus proxy. -func (p *Proxy) Start(ctx context.Context, output io.Writer, useSandbox bool) error { - p.lock.Lock() - defer p.lock.Unlock() +// Start starts and configures a D-Bus proxy process. +func (p *Proxy) Start() error { + if p.final == nil || p.final.WriterTo == nil { + return syscall.ENOTRECOVERABLE + } + + p.mu.Lock() + defer p.mu.Unlock() + p.pmu.Lock() + defer p.pmu.Unlock() - if p.seal == nil { - return errors.New("proxy not sealed") + if p.cancel != nil || p.cause != nil { + return errors.New("dbus: already started") } - var h helper.Helper + ctx, cancel := context.WithCancelCause(p.ctx) - c, cancel := context.WithCancelCause(ctx) - if !useSandbox { - h = helper.NewDirect(c, p.name, p.seal, true, argF, func(cmd *exec.Cmd) { + if !p.useSandbox { + p.helper = helper.NewDirect(ctx, p.name, p.final, true, argF, func(cmd *exec.Cmd) { if p.CmdF != nil { p.CmdF(cmd) } - if output != nil { - cmd.Stdout, cmd.Stderr = output, output + if p.output != nil { + cmd.Stdout, cmd.Stderr = p.output, p.output } cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} cmd.Env = make([]string, 0) @@ -59,15 +62,15 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, useSandbox bool) er libPaths = ldd.Path(entries) } - h = helper.New( - c, toolPath, - p.seal, true, + p.helper = helper.New( + ctx, toolPath, + p.final, true, argF, func(container *sandbox.Container) { container.Seccomp |= seccomp.FilterMultiarch container.Hostname = "fortify-dbus" container.CommandContext = p.CommandContext - if output != nil { - container.Stdout, container.Stderr = output, output + if p.output != nil { + container.Stdout, container.Stderr = p.output, p.output } if p.CmdF != nil { @@ -81,10 +84,17 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, useSandbox bool) er // upstream bus directories upstreamPaths := make([]string, 0, 2) - for _, as := range []string{p.session[0], p.system[0]} { - if len(as) > 0 && strings.HasPrefix(as, "unix:path=/") { - // leave / intact - upstreamPaths = append(upstreamPaths, path.Dir(as[10:])) + for _, addr := range [][]AddrEntry{p.final.SessionUpstream, p.final.SystemUpstream} { + for _, ent := range addr { + if ent.Method != "unix" { + continue + } + for _, pair := range ent.Values { + if pair[0] != "path" || !path.IsAbs(pair[1]) { + continue + } + upstreamPaths = append(upstreamPaths, path.Dir(pair[1])) + } } } slices.Sort(upstreamPaths) @@ -95,10 +105,10 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, useSandbox bool) er // parent directories of bind paths sockDirPaths := make([]string, 0, 2) - if d := path.Dir(p.session[1]); path.IsAbs(d) { + if d := path.Dir(p.final.Session[1]); path.IsAbs(d) { sockDirPaths = append(sockDirPaths, d) } - if d := path.Dir(p.system[1]); path.IsAbs(d) { + if d := path.Dir(p.final.System[1]); path.IsAbs(d) { sockDirPaths = append(sockDirPaths, d) } slices.Sort(sockDirPaths) @@ -113,14 +123,13 @@ func (p *Proxy) Start(ctx context.Context, output io.Writer, useSandbox bool) er }, nil) } - if err := h.Start(); err != nil { + if err := p.helper.Start(); err != nil { cancel(err) + p.helper = nil return err } - p.helper = h - p.ctx = c - p.cancel = cancel + p.cancel, p.cause = cancel, func() error { return context.Cause(ctx) } return nil } @@ -128,28 +137,30 @@ 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() + p.mu.RLock() + defer p.mu.RUnlock() - if p.helper == nil { + p.pmu.RLock() + if p.helper == nil || p.cancel == nil || p.cause == nil { + p.pmu.RUnlock() return errors.New("dbus: not started") } 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) { + if errors.Is(errs[0], context.Canceled) && + errors.Is(p.cause(), proxyClosed) { errs[0] = nil } + p.pmu.RUnlock() // ensure socket removal so ephemeral directory is empty at revert - if err := os.Remove(p.session[1]); err != nil && !errors.Is(err, os.ErrNotExist) { + if err := os.Remove(p.final.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) { + if p.final.System[1] != "" { + if err := os.Remove(p.final.System[1]); err != nil && !errors.Is(err, os.ErrNotExist) { errs[2] = err } } @@ -159,14 +170,13 @@ func (p *Proxy) Wait() error { // 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() + p.pmu.Lock() + defer p.pmu.Unlock() if p.cancel == nil { panic("dbus: not started") } p.cancel(proxyClosed) - p.cancel = nil } func argF(argsFd, statFd int) []string { diff --git a/dbus/proxy.go b/dbus/proxy.go index 209ae573..a18bf753 100644 --- a/dbus/proxy.go +++ b/dbus/proxy.go @@ -2,11 +2,10 @@ package dbus import ( "context" - "errors" - "fmt" "io" "os/exec" "sync" + "syscall" "git.gensokyo.uk/security/fortify/helper" ) @@ -15,84 +14,88 @@ import ( // Overriding ProxyName will only affect Proxy instance created after the change. var ProxyName = "xdg-dbus-proxy" -// 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. +// Proxy holds the state of a xdg-dbus-proxy process, and should never be copied. type Proxy struct { helper helper.Helper ctx context.Context + cancel context.CancelCauseFunc + cause func() error + + final *Final + output io.Writer + useSandbox bool - name string - session [2]string - system [2]string - CmdF func(any) - sysP bool + name string + CmdF func(any) CommandContext func(ctx context.Context) (cmd *exec.Cmd) FilterF func([]byte) []byte - seal io.WriterTo - lock sync.RWMutex + mu, pmu 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 } - -var ( - ErrConfig = errors.New("no configuration to seal") -) - func (p *Proxy) String() string { if p == nil { return "(invalid dbus proxy)" } - p.lock.RLock() - defer p.lock.RUnlock() + p.mu.RLock() + defer p.mu.RUnlock() if p.helper != nil { return p.helper.String() } - if p.seal != nil { - return p.seal.(fmt.Stringer).String() - } - - return "(unsealed dbus proxy)" + return "(unused dbus proxy)" } -// Seal seals the Proxy instance. -func (p *Proxy) Seal(session, system *Config) error { - p.lock.Lock() - defer p.lock.Unlock() - - if p.seal != nil { - panic("dbus proxy sealed twice") - } +// Final describes the outcome of a proxy configuration. +type Final struct { + Session, System ProxyPair + // parsed upstream address + SessionUpstream, SystemUpstream []AddrEntry + io.WriterTo +} +// Finalise creates a checked argument writer for [Proxy]. +func Finalise(sessionBus, systemBus ProxyPair, session, system *Config) (final *Final, err error) { if session == nil && system == nil { - return ErrConfig + return nil, syscall.EBADE } var args []string if session != nil { - args = append(args, session.Args(p.session)...) + args = append(args, session.Args(sessionBus)...) } if system != nil { - args = append(args, system.Args(p.system)...) - p.sysP = true + args = append(args, system.Args(systemBus)...) + } + + final = &Final{Session: sessionBus, System: systemBus} + + final.WriterTo, err = helper.NewCheckedArgs(args) + if err != nil { + return } - if seal, err := helper.NewCheckedArgs(args); err != nil { - return err - } else { - p.seal = seal + + if session != nil { + final.SessionUpstream, err = Parse([]byte(final.Session[0])) + if err != nil { + return + } + } + if system != nil { + final.SystemUpstream, err = Parse([]byte(final.System[0])) + if err != nil { + return + } } - return nil + return } -// New returns a reference to a new unsealed Proxy. -func New(session, system [2]string) *Proxy { - return &Proxy{name: ProxyName, session: session, system: system} +// New returns a new instance of [Proxy]. +func New(ctx context.Context, final *Final, output io.Writer) *Proxy { + return &Proxy{name: ProxyName, ctx: ctx, final: final, output: output, useSandbox: true} } |
