hst/dbus: validate interface strings

This is relocated to hst to validate early.

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

7 files changed, 215 insertions, 79 deletions

diff --git a/hst/config.go b/hst/config.go
index 7c96874f..9f4ee2f7 100644
--- a/hst/config.go
+++ b/hst/config.go
@@ -140,6 +140,13 @@ func (config *Config) Validate() error {
 			Msg: "identity " + strconv.Itoa(config.Identity) + " out of range"}
 	}
 
+	if err := config.SessionBus.CheckInterfaces("session"); err != nil {
+		return err
+	}
+	if err := config.SystemBus.CheckInterfaces("system"); err != nil {
+		return err
+	}
+
 	if config.Container == nil {
 		return &AppError{Step: "validate configuration", Err: ErrConfigNull,
 			Msg: "configuration missing container state"}
diff --git a/hst/config_test.go b/hst/config_test.go
index ea50d4f1..eba2b793 100644
--- a/hst/config_test.go
+++ b/hst/config_test.go
@@ -20,6 +20,10 @@ func TestConfigValidate(t *testing.T) {
 			Msg: "identity -1 out of range"}},
 		{"identity upper", &hst.Config{Identity: 10000}, &hst.AppError{Step: "validate configuration", Err: hst.ErrIdentityBounds,
 			Msg: "identity 10000 out of range"}},
+		{"dbus session", &hst.Config{SessionBus: &hst.BusConfig{See: []string{""}}},
+			&hst.BadInterfaceError{Interface: "", Segment: "session"}},
+		{"dbus system", &hst.Config{SystemBus: &hst.BusConfig{See: []string{""}}},
+			&hst.BadInterfaceError{Interface: "", Segment: "system"}},
 		{"container", &hst.Config{}, &hst.AppError{Step: "validate configuration", Err: hst.ErrConfigNull,
 			Msg: "configuration missing container state"}},
 		{"home", &hst.Config{Container: &hst.ContainerConfig{}}, &hst.AppError{Step: "validate configuration", Err: hst.ErrConfigNull,
diff --git a/hst/dbus.go b/hst/dbus.go
index 7bad231e..6dd1a2c3 100644
--- a/hst/dbus.go
+++ b/hst/dbus.go
@@ -1,5 +1,27 @@
 package hst
 
+import (
+	"strconv"
+	"strings"
+)
+
+// BadInterfaceError is returned when Interface fails an undocumented check in xdg-dbus-proxy,
+// which would have cause a silent failure.
+type BadInterfaceError struct {
+	// Interface is the offending interface string.
+	Interface string
+	// Segment is passed through from the [BusConfig.CheckInterfaces] argument.
+	Segment string
+}
+
+func (e *BadInterfaceError) Message() string { return e.Error() }
+func (e *BadInterfaceError) Error() string {
+	if e == nil {
+		return "<nil>"
+	}
+	return "bad interface string " + strconv.Quote(e.Interface) + " in " + e.Segment + " bus configuration"
+}
+
 // BusConfig configures the xdg-dbus-proxy process.
 type BusConfig struct {
 	// See set 'see' policy for NAME (--see=NAME)
@@ -14,6 +36,74 @@ type BusConfig struct {
 	// Broadcast set RULE for broadcasts from NAME (--broadcast=NAME=RULE)
 	Broadcast map[string]string `json:"broadcast"`
 
-	Log    bool `json:"log,omitempty"`
+	// Log turn on logging (--log)
+	Log bool `json:"log,omitempty"`
+	// Filter enable filtering (--filter)
 	Filter bool `json:"filter"`
 }
+
+// Interfaces iterates over all interface strings specified in [BusConfig].
+func (c *BusConfig) Interfaces(yield func(string) bool) {
+	if c == nil {
+		return
+	}
+
+	for _, iface := range c.See {
+		if !yield(iface) {
+			return
+		}
+	}
+	for _, iface := range c.Talk {
+		if !yield(iface) {
+			return
+		}
+	}
+	for _, iface := range c.Own {
+		if !yield(iface) {
+			return
+		}
+	}
+
+	for iface := range c.Call {
+		if !yield(iface) {
+			return
+		}
+	}
+	for iface := range c.Broadcast {
+		if !yield(iface) {
+			return
+		}
+	}
+}
+
+// CheckInterfaces checks for invalid interface strings based on an undocumented check in xdg-dbus-error,
+// returning [BadInterfaceError] if one is encountered.
+func (c *BusConfig) CheckInterfaces(segment string) error {
+	if c == nil {
+		return nil
+	}
+
+	for iface := range c.Interfaces {
+		/*
+			xdg-dbus-proxy fails without output when this condition is not met:
+				char *dot = strrchr (filter->interface, '.');
+				if (dot != NULL)
+				  {
+				    *dot = 0;
+				    if (strcmp (dot + 1, "*") != 0)
+				      filter->member = g_strdup (dot + 1);
+				  }
+
+			trim ".*" since they are removed before searching for '.':
+				if (g_str_has_suffix (name, ".*"))
+				  {
+				    name[strlen (name) - 2] = 0;
+				    wildcard = TRUE;
+				  }
+		*/
+		if strings.IndexByte(strings.TrimSuffix(iface, ".*"), '.') == -1 {
+			return &BadInterfaceError{iface, segment}
+		}
+	}
+	return nil
+}
diff --git a/hst/dbus_test.go b/hst/dbus_test.go
new file mode 100644
index 00000000..8b04494b
--- /dev/null
+++ b/hst/dbus_test.go
@@ -0,0 +1,109 @@
+package hst_test
+
+import (
+	"reflect"
+	"slices"
+	"testing"
+
+	"hakurei.app/container"
+	"hakurei.app/hst"
+)
+
+func TestBadInterfaceError(t *testing.T) {
+	testCases := []struct {
+		name string
+		err  error
+		want string
+	}{
+		{"nil", (*hst.BadInterfaceError)(nil), "<nil>"},
+		{"session", &hst.BadInterfaceError{Interface: "\x00", Segment: "session"},
+			`bad interface string "\x00" in session bus configuration`},
+		{"system", &hst.BadInterfaceError{Interface: "\x01", Segment: "system"},
+			`bad interface string "\x01" in system bus configuration`},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if gotError := tc.err.Error(); gotError != tc.want {
+				t.Errorf("Error: %s, want %s", gotError, tc.want)
+			}
+			if gotMessage, ok := container.GetErrorMessage(tc.err); !ok {
+				t.Error("GetErrorMessage: ok = false")
+			} else if gotMessage != tc.want {
+				t.Errorf("GetErrorMessage: %s, want %s", gotMessage, tc.want)
+			}
+		})
+	}
+}
+
+func TestBusConfigInterfaces(t *testing.T) {
+	testCases := []struct {
+		name   string
+		c      *hst.BusConfig
+		cutoff int
+		want   []string
+	}{
+		{"nil", nil, 0, nil},
+		{"all", &hst.BusConfig{
+			See: []string{"see"}, Talk: []string{"talk"}, Own: []string{"own"},
+			Call:      map[string]string{"call": "unreachable"},
+			Broadcast: map[string]string{"broadcast": "unreachable"},
+		}, 0, []string{"see", "talk", "own", "call", "broadcast"}},
+
+		{"all cutoff", &hst.BusConfig{
+			See: []string{"see"}, Talk: []string{"talk"}, Own: []string{"own"},
+			Call:      map[string]string{"call": "unreachable"},
+			Broadcast: map[string]string{"broadcast": "unreachable"},
+		}, 3, []string{"see", "talk", "own"}},
+
+		{"cutoff see", &hst.BusConfig{See: []string{"see"}}, 1, []string{"see"}},
+		{"cutoff talk", &hst.BusConfig{Talk: []string{"talk"}}, 1, []string{"talk"}},
+		{"cutoff own", &hst.BusConfig{Own: []string{"own"}}, 1, []string{"own"}},
+		{"cutoff call", &hst.BusConfig{Call: map[string]string{"call": "unreachable"}}, 1, []string{"call"}},
+		{"cutoff broadcast", &hst.BusConfig{Broadcast: map[string]string{"broadcast": "unreachable"}}, 1, []string{"broadcast"}},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			var got []string
+			if tc.cutoff > 0 {
+				var i int
+				got = make([]string, 0, tc.cutoff)
+				for v := range tc.c.Interfaces {
+					i++
+					got = append(got, v)
+					if i == tc.cutoff {
+						break
+					}
+				}
+			} else {
+				got = slices.Collect(tc.c.Interfaces)
+			}
+
+			if !slices.Equal(got, tc.want) {
+				t.Errorf("Interfaces: %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestBusConfigCheckInterfaces(t *testing.T) {
+	testCases := []struct {
+		name string
+		c    *hst.BusConfig
+		err  error
+	}{
+		{"nil", nil, nil},
+		{"zero", &hst.BusConfig{See: []string{""}},
+			&hst.BadInterfaceError{Interface: "", Segment: "zero"}},
+		{"suffix", &hst.BusConfig{See: []string{".*"}},
+			&hst.BadInterfaceError{Interface: ".*", Segment: "suffix"}},
+		{"valid suffix", &hst.BusConfig{See: []string{"..*"}}, nil},
+		{"valid", &hst.BusConfig{See: []string{"."}}, nil},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if err := tc.c.CheckInterfaces(tc.name); !reflect.DeepEqual(err, tc.err) {
+				t.Errorf("CheckInterfaces: error = %#v, want %#v", err, tc.err)
+			}
+		})
+	}
+}
diff --git a/system/dbus/config.go b/system/dbus/config.go
index 2aeb3bb7..2a09ee90 100644
--- a/system/dbus/config.go
+++ b/system/dbus/config.go
@@ -1,74 +1,12 @@
 package dbus
 
 import (
-	"strings"
-
 	"hakurei.app/hst"
 )
 
 // ProxyPair is an upstream dbus address and a downstream socket path.
 type ProxyPair [2]string
 
-// interfacesAll returns an iterator over all interfaces specified in c.
-func interfacesAll(c *hst.BusConfig) func(yield func(string) bool) {
-	return func(yield func(string) bool) {
-		for _, iface := range c.See {
-			if !yield(iface) {
-				return
-			}
-		}
-		for _, iface := range c.Talk {
-			if !yield(iface) {
-				return
-			}
-		}
-		for _, iface := range c.Own {
-			if !yield(iface) {
-				return
-			}
-		}
-
-		for iface := range c.Call {
-			if !yield(iface) {
-				return
-			}
-		}
-		for iface := range c.Broadcast {
-			if !yield(iface) {
-				return
-			}
-		}
-	}
-}
-
-// checkInterfaces checks [hst.BusConfig] for invalid interfaces based on an undocumented check in xdg-dbus-error,
-// returning [BadInterfaceError] if one is encountered.
-func checkInterfaces(c *hst.BusConfig, segment string) error {
-	for iface := range interfacesAll(c) {
-		/*
-			xdg-dbus-proxy fails without output when this condition is not met:
-				char *dot = strrchr (filter->interface, '.');
-				if (dot != NULL)
-				  {
-				    *dot = 0;
-				    if (strcmp (dot + 1, "*") != 0)
-				      filter->member = g_strdup (dot + 1);
-				  }
-
-			trim ".*" since they are removed before searching for '.':
-				if (g_str_has_suffix (name, ".*"))
-				  {
-				    name[strlen (name) - 2] = 0;
-				    wildcard = TRUE;
-				  }
-		*/
-		if strings.IndexByte(strings.TrimSuffix(iface, ".*"), '.') == -1 {
-			return &BadInterfaceError{iface, segment}
-		}
-	}
-	return nil
-}
-
 // Args returns the xdg-dbus-proxy arguments equivalent of [hst.BusConfig].
 func Args(c *hst.BusConfig, bus ProxyPair) (args []string) {
 	argc := 2 + len(c.See) + len(c.Talk) + len(c.Own) + len(c.Call) + len(c.Broadcast)
diff --git a/system/dbus/config_test.go b/system/dbus/config_test.go
index 9a0e570e..84477e40 100644
--- a/system/dbus/config_test.go
+++ b/system/dbus/config_test.go
@@ -10,7 +10,7 @@ import (
 	"hakurei.app/system/dbus"
 )
 
-func TestConfig_Args(t *testing.T) {
+func TestConfigArgs(t *testing.T) {
 	for _, tc := range testCasesExt {
 		if tc.wantErr {
 			// args does not check for nulls
@@ -19,9 +19,7 @@ func TestConfig_Args(t *testing.T) {
 
 		t.Run("build arguments for "+tc.id, func(t *testing.T) {
 			if got := dbus.Args(tc.c, tc.bus); !slices.Equal(got, tc.want) {
-				t.Errorf("Args(%q) = %v, want %v",
-					tc.bus,
-					got, tc.want)
+				t.Errorf("Args: %v, want %v", got, tc.want)
 			}
 		})
 	}
diff --git a/system/dbus/proxy.go b/system/dbus/proxy.go
index 59a1d7d4..535f233d 100644
--- a/system/dbus/proxy.go
+++ b/system/dbus/proxy.go
@@ -2,7 +2,6 @@ package dbus
 
 import (
 	"context"
-	"fmt"
 	"io"
 	"sync"
 	"syscall"
@@ -16,15 +15,6 @@ import (
 // Overriding ProxyName will only affect Proxy instance created after the change.
 var ProxyName = "xdg-dbus-proxy"
 
-type BadInterfaceError struct {
-	Interface string
-	Segment   string
-}
-
-func (e *BadInterfaceError) Error() string {
-	return fmt.Sprintf("bad interface string %q in %s bus configuration", e.Interface, e.Segment)
-}
-
 // Proxy holds the state of a xdg-dbus-proxy process, and should never be copied.
 type Proxy struct {
 	helper helper.Helper
@@ -74,13 +64,13 @@ func Finalise(sessionBus, systemBus ProxyPair, session, system *hst.BusConfig) (
 
 	var args []string
 	if session != nil {
-		if err = checkInterfaces(session, "session"); err != nil {
+		if err = session.CheckInterfaces("session"); err != nil {
 			return
 		}
 		args = append(args, Args(session, sessionBus)...)
 	}
 	if system != nil {
-		if err = checkInterfaces(system, "system"); err != nil {
+		if err = system.CheckInterfaces("system"); err != nil {
 			return
 		}
 		args = append(args, Args(system, systemBus)...)