fix: harden peer pinning, same-name discovery, and full-set switch guard#53
Merged
Merged
Conversation
Once a fingerprint is pinned (TOFU), update(with:) only treated a *different* incoming fingerprint as a mismatch. An advertisement carrying no fingerprint at all fell through and overwrote host/port/isActive, so a LAN attacker advertising the peer's Bonjour name with no `fp` TXT record could silently re-point routing at a machine they control. Now a pinned device only accepts a routing update from an advertisement carrying the same fingerprint; a missing one is ignored entirely.
- Identify "self" by interface address (getifaddrs), not by Host.current().localizedName. When two Macs share a device name mDNS renames one service, so the name-based check made the renamed Mac hide its real peer (same name) and list itself instead. - Prefer an IPv4 address when resolving a peer. A link-local IPv6 address (fe80::...%enX) often won't round-trip through NWEndpoint.Host, so taking whichever address resolved first could yield an unusable host.
A full-set switch fired while a per-peripheral pair/handoff was in flight would issue a re-entrant connect/release on a peripheral that's already transitioning. The dropdown Mac row now greys out while any peripheral is .connecting/.releasing (with an explanatory tooltip), and performSwitch guards against the brief click-race before the row re-renders.
|
🎉 This PR is included in version 2.16.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
1 participant
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.
Summary
A round of correctness/security hardening from a code review of the discovery, pairing, and switch paths, plus a small UX guard. Three concern-grouped commits.
Changes
Security — peer identity pinning (
NetworkDevice.update(with:))Once a fingerprint is pinned (TOFU), only an advertisement carrying that same fingerprint updates routing. Previously a mismatch was caught only when the incoming advertisement had a different fingerprint — one with no
fpfell through and overwrotehost/port/isActive, so a LAN attacker advertising the peer's Bonjour name with no fingerprint could silently re-point routing at their machine. A missing fingerprint is now ignored entirely.Discovery — same-named Macs (
NetworkDeviceStore/ServiceBrowser)getifaddrs) instead ofHost.current().localizedName. When two Macs share a device name, mDNS renames one service, so the old name check made the renamed Mac hide its real peer (same name) and list itself.fe80::…%enX) often won't round-trip throughNWEndpoint.Host.UX — block "send all" while a peripheral is switching (
DropdownContentView/AppDelegate/BluetoothPeripheralStore)The full-set switch (clicking a Mac row) greys out while any peripheral is
.connecting/.releasing, with an explanatory tooltip, andperformSwitchguards the click-race — preventing a re-entrant connect/release on a peripheral already mid pair/handoff.Testing
xcodebuild … -configuration Debug buildsucceeds; sources formatted withswift format.Note
The fingerprint is advertised in cleartext in the Bonjour TXT record, so a LAN attacker who reads it can replay the matching value to re-point routing — but that only yields a DoS (the reachability ping fails and the row greys out); it can't steal peripherals, since the command channel still requires completing the PSK handshake. Fully hardening the advertisement would be a larger change.