From 5d95f1332e9b28a1a9ca9ebb73afe2a3e2db6047 Mon Sep 17 00:00:00 2001 From: Joshua Rogers Date: Wed, 12 Aug 2026 00:17:56 +0200 Subject: [PATCH] fix: prove a moved peer endpoint before relocating routing A Bonjour resolve echoing a peer's cleartext `fp` could re-point a pinned peer's host/port at an attacker-chosen machine, since `update(with:)` treats fp equality as sufficient. Gate same-id endpoint moves from unproven (Bonjour) resolves behind a PSK-proof ping, mirroring `migrateRenamedPeerIfNeeded`; INTRODUCE-sourced updates stay immediate. --- .../Model/Store/NetworkDeviceStore.swift | 54 +++++++++++++++++-- 1 file changed, 50 insertions(+), 4 deletions(-) diff --git a/Magic Switch/Model/Store/NetworkDeviceStore.swift b/Magic Switch/Model/Store/NetworkDeviceStore.swift index df20a46..4102281 100644 --- a/Magic Switch/Model/Store/NetworkDeviceStore.swift +++ b/Magic Switch/Model/Store/NetworkDeviceStore.swift @@ -84,9 +84,10 @@ final class NetworkDeviceStore: ObservableObject, NetworkDeviceManageable { /// together) don't stack up multiple rechecks. Main-only. private var pendingFastRecheck: Set = [] - /// Advertised names with a rename-migration proof ping already in flight, so - /// a burst of resolves for the same service doesn't fan out into a burst of - /// pings. Main-only. See `migrateRenamedPeerIfNeeded`. + /// Advertised names with a rename-migration or endpoint-move proof ping + /// already in flight, so a burst of resolves for the same service doesn't fan + /// out into a burst of pings. Main-only. See `migrateRenamedPeerIfNeeded` and + /// `verifyMovedEndpointIfNeeded`. private var pendingMigrationProofs: Set = [] /// TTL for discovered entries with no live Bonjour presence (INTRODUCE- @@ -162,6 +163,15 @@ final class NetworkDeviceStore: ObservableObject, NetworkDeviceManageable { } func updateNetworkDevice(_ device: NetworkDevice) { + updateNetworkDevice(device, proven: false) + } + + /// `proven` means the peer just demonstrated this endpoint over the secure + /// channel (an INTRODUCE handshake), so it may relocate a pinned record's + /// routing at once. An unproven Bonjour resolve may not: its `fp` is + /// cleartext multicast any LAN listener can echo, so a matching fingerprint + /// alone can't authorise pointing `host`/`port` at an attacker-chosen host. + private func updateNetworkDevice(_ device: NetworkDevice, proven: Bool) { // Never let an advertisement re-point a registered record at this Mac. // Our own service carries the *same* `fp` as the peer — it's a hash of the // shared PSK — so `update(with:)` would take it as a trusted routing @@ -187,6 +197,18 @@ final class NetworkDeviceStore: ObservableObject, NetworkDeviceManageable { } return } + // An unproven resolve carrying the pinned fingerprint but a *new* + // endpoint isn't trusted to relocate routing — the fp is cleartext and + // any LAN listener can echo it. Prove the endpoint over the secure + // channel first (as `migrateRenamedPeerIfNeeded` does for a rename), + // leaving the verified routing untouched until it does. + if !proven, let stored = prior.fingerprint, stored == device.fingerprint, + prior.pendingFingerprint == nil, + prior.host != device.host || prior.port != device.port + { + verifyMovedEndpointIfNeeded(for: device) + return + } let priorFingerprint = prior.fingerprint networkDevices[index].update(with: device) saveNetworkDevices() @@ -229,7 +251,7 @@ final class NetworkDeviceStore: ObservableObject, NetworkDeviceManageable { fingerprint: provedFingerprint ) addDiscoveredNetworkDevice(device) - updateNetworkDevice(device) + updateNetworkDevice(device, proven: true) // After the update: if it just parked this record pending exactly the // proved key, the proof supersedes the warning in the same tick (the // handshake-time resolve hooks fired before these frames existed). @@ -352,6 +374,30 @@ final class NetworkDeviceStore: ObservableObject, NetworkDeviceManageable { ) } + /// A pinned registered peer advertising a *new* endpoint under its own name + /// may be a real IP/port change — or a LAN bystander echoing the cleartext + /// `fp` to redirect us at a host it controls. Probe the advertised endpoint + /// and relocate the record only if whoever answers there proves the pairing + /// key. Same proof and in-flight de-dup as `migrateRenamedPeerIfNeeded`. + private func verifyMovedEndpointIfNeeded(for discovered: NetworkDevice) { + guard !pendingMigrationProofs.contains(discovered.id) else { return } + pendingMigrationProofs.insert(discovered.id) + executeCommand(.ping, on: discovered, countsTowardRateLimit: false) { [weak self] result in + DispatchQueue.main.async { + guard let self = self else { return } + self.pendingMigrationProofs.remove(discovered.id) + guard case .success = result, + let index = self.networkDevices.firstIndex(where: { $0.id == discovered.id }), + self.networkDevices[index].fingerprint == discovered.fingerprint, + self.networkDevices[index].pendingFingerprint == nil + else { return } + self.networkDevices[index].update(with: discovered) + self.saveNetworkDevices() + self.deviceReachability[discovered.id] = self.networkDevices[index].isActive + } + } + } + /// Re-attempt migration against services already resolved. A resolve happens /// once per Bonjour `didFind` and nothing re-runs it, so when the renamed /// service resolves *before* the old name's goodbye arrives — a coin flip,