fix(pairing): keep the addCu listener deactivated whenever a trusted addCu device exists#92
Open
kirollosnct wants to merge 3 commits into
Open
fix(pairing): keep the addCu listener deactivated whenever a trusted addCu device exists#92kirollosnct wants to merge 3 commits into
kirollosnct wants to merge 3 commits into
Conversation
…ice is trusted startPairingService enabled the pairing listener unconditionally in listener mode. Per pairing spec §4.3, processing addCu-requests SHALL stay deactivated as long as the pairing with the trusted devZ is intended — and a restart does not change that intent. With a trusted addCu device restored from persistence, the listener therefore started while it had to be off. Beyond the §4.3 violation this made the 1.b.i trust transfer unreachable after a reboot: a new request arriving during the first 15 minutes was evaluated by the active listener, deferred by the replacement-window check — but only after its digest was recorded and the listener stopped itself. The post-window re-evaluation then rejected the request as a replay, locking the new devZ out. Guard the startup activation on GetTrustedAddCuDevice(): with a trusted addCu device present, the listener stays off and the §4.3 1.a replacement timer armed by startAddCuReplacementTimersForOfflineDevices owns the reactivation. A request announced during the window now stays un-consumed in the mDNS cache and gets a clean first evaluation after the timer expires. Reconnection of the trusted device keeps processing off (§4.3 1.b.ii), and manual removal still reactivates immediately (§4.3 2.b).
…evice is trusted via RegisterRemoteService RegisterRemoteService marked the service trusted and triggered a connection attempt, but never touched the pairing listener. The listener was only disarmed indirectly once the SHIP handshake with the device reached ConnectionStateCompleted — leaving it armed for the seconds until an online devZ connects, and indefinitely for an offline one (e.g. a persisted pairing restored after Hub start). Per pairing spec §4.3, registering trust in an addCu device expresses the pairing intent, so processing of addCu-requests must deactivate at registration. The window is not just non-compliant but exploitable: with the listener wrongly armed, a third devZ announcing a valid addCu-request lands in OnPairingSuccess, where the just-registered device is found as the trusted addCu device with a differing fingerprint — and since RegisterRemoteService arms no replacement window, the §4.3 1.b.i transfer logic immediately untrusts the device that was just registered and trusts the announcer instead. Stop the pairing listener in RegisterRemoteService when the registered service is a trusted addCu device and the Hub has started. This mirrors UnregisterRemoteService's addCu path (which reactivates) and completes the invariant of the startup guard: the listener is off whenever a trusted addCu device exists, regardless of how or when the trust was established. Registration before Start is already covered by the startup guard in startPairingService. Stopping the listener alone, however, would strand an offline devZ: nothing on this path arms the §4.3 1.a replacement timer, so the listener would stay deactivated indefinitely with manual removal (UnregisterRemoteService) the only recovery — inconsistent with the startup path, where startAddCuReplacementTimersForOfflineDevices arms the timer for exactly this kind of trusted-offline device. So when the registered addCu device has no live connection, arm the replacement timer too; a completed connection cancels it via StopAddCuReplacementTimer. This also gives defense in depth: with the window armed, OnPairingSuccess defers a competing announcement (via IsInReplacementWindow) instead of transferring trust immediately.
Hub.Shutdown stopped the announcement lifetime timers but never touched the AddCu replacement tracker. Whenever a trusted addCu device is offline at shutdown — the normal state, armed at startup by startAddCuReplacementTimersForOfflineDevices or by RegisterRemoteService — the 15-minute replacement timer stayed live across teardown. Two consequences: - Post-shutdown resurrection: up to 15 minutes after Shutdown, the time.AfterFunc fires handleAddCuReplacementTimeout, which calls reactivatePairingListener -> enablePairingListener (re-creating a listener on an already-shut-down pairing service) and RequestPairingEntries (calling into already-shut-down mDNS). - Memory retention: the pending timer keeps a reference to the Hub, so the whole object graph cannot be collected until the window elapses. Add AddCuReplacementTracker.StopAll — mirroring AnnouncementLifetimeTracker.StopAll — which unconditionally stops the active timer and clears tracking state, and call it from Hub.Shutdown next to the existing lifetime-tracker teardown.
kirollosnct
force-pushed
the
fix/pairing-startup-listener-guard
branch
from
July 14, 2026 09:24
f247969 to
f5a50f8
Compare
|
coverage: 93.096% (+0.1%) from 92.972% — fix/pairing-startup-listener-guard into dev |
kirollosnct
marked this pull request as ready for review
July 19, 2026 11:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Per pairing spec §4.3, processing of addCu-requests SHALL stay deactivated as long as the pairing with the trusted devZ is intended. Two entry points violated this:
Fix
Together: the listener is off whenever a trusted addCu device exists, regardless of how or when the trust was established. Reconnection keeps processing off (§4.3 1.b.ii) and manual removal still reactivates immediately (§4.3 2.b), both unchanged.