diff --git a/cmd/root.go b/cmd/root.go index 90ff5cf9f..670985ad9 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -391,6 +391,9 @@ func runRoot(cmd *cobra.Command, args []string) { } } + // all device skis are registered, unknown ones may now be denied + eebus.ConfigComplete() + // setup MCP if err == nil && isMcp() { router := httpd.Router() diff --git a/server/eebus/eebus.go b/server/eebus/eebus.go index 742671c5e..24f2e9ba7 100644 --- a/server/eebus/eebus.go +++ b/server/eebus/eebus.go @@ -83,6 +83,10 @@ type EEBus struct { mux sync.Mutex log *util.Logger + // configured is set once device configuration has finished. Until then, an + // unknown ski may still belong to a device that is not configured yet. + configured bool + ski string paired []shipapi.ServiceIdentity // devices paired via SHIP Pairing Service @@ -108,6 +112,20 @@ func Instance() (*EEBus, error) { return instance, nil } +// ConfigComplete marks device configuration as finished. Pairing requests from +// unknown skis are left pending until then- the ski of a device that is still +// being configured is not yet registered, and denying it would lock the remote +// service out until the next restart. +func ConfigComplete() { + if instance == nil { + return + } + + instance.mux.Lock() + defer instance.mux.Unlock() + instance.configured = true +} + func GetStatus() any { var ski string if instance != nil { @@ -565,6 +583,13 @@ func (c *EEBus) ServicePairingDetailUpdate(identity shipapi.ServiceIdentity, det defer c.mux.Unlock() if clients, ok := c.clients[identity.SKI]; !ok || len(clients) == 0 { + if !c.configured { + // device configuration is still running- leave the request pending + // instead of denying a ski that is about to be registered + c.log.DEBUG.Printf("pairing request from %s while configuring, left pending", identity.SKI) + return + } + // this is an unknown SKI, so deny pairing c.service.CancelPairing(identity) } diff --git a/server/eebus/eebus_test.go b/server/eebus/eebus_test.go index 25412737c..2e7958ee3 100644 --- a/server/eebus/eebus_test.go +++ b/server/eebus/eebus_test.go @@ -76,3 +76,25 @@ func TestUnregisterDevice_MutexNotHeldDuringShipCall(t *testing.T) { c.UnregisterDevice("aabbcc", dev) } + +// TestPairingDeniedOnlyWhenConfigured guards that an unknown ski is not denied +// while device configuration is still running- its device may register the ski +// moments later, and a denial locks the remote service out until the next restart. +func TestPairingDeniedOnlyWhenConfigured(t *testing.T) { + identity := shipapi.NewServiceIdentity("aabbcc", "", "") + detail := shipapi.NewConnectionStateDetail(shipapi.ConnectionStateReceivedPairingRequest, nil) + + service := eebusmocks.NewServiceInterface(t) + c := &EEBus{ + log: util.NewLogger("test"), + clients: make(map[string][]Device), + service: service, + } + + // still configuring - no CancelPairing expected + c.ServicePairingDetailUpdate(identity, detail) + + service.EXPECT().CancelPairing(identity).Once() + c.configured = true + c.ServicePairingDetailUpdate(identity, detail) +}