From 9b507715d42f74c3f18fe817c2464abfca1b0182 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Wed, 8 Oct 2025 04:57:22 +0900 Subject: [PATCH] hst/dbus: validate interface strings This is relocated to hst to validate early. Signed-off-by: Ophestra --- hst/config.go | 7 +++ hst/config_test.go | 4 ++ hst/dbus.go | 92 ++++++++++++++++++++++++++++++- hst/dbus_test.go | 109 +++++++++++++++++++++++++++++++++++++ system/dbus/config.go | 62 --------------------- system/dbus/config_test.go | 6 +- system/dbus/proxy.go | 14 +---- 7 files changed, 215 insertions(+), 79 deletions(-) create mode 100644 hst/dbus_test.go diff --git a/hst/config.go b/hst/config.go index 7c96874..9f4ee2f 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 ea50d4f..eba2b79 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 7bad231..6dd1a2c 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 "" + } + 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 0000000..8b04494 --- /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), ""}, + {"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 2aeb3bb..2a09ee9 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 9a0e570..84477e4 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 59a1d7d..535f233 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)...)