Skip to content

Commit 09fd623

Browse files
committed
fix: wire protocol correctness, injectable BLE, and 21 integration tests
Wire protocol bugs fixed (found while writing tests): ChallengeHandler (iOS): - Was trying to AES-GCM decrypt the whole wire frame; fixed to strip 2-byte header and JSON-decode the plaintext ChallengeIssuedMessage - Was signing the encrypted nonce; fixed to decrypt nonce first, then sign the plaintext bytes (daemon verifies plaintext) - Was sending plain JSON response; fixed to prepend [1,4] wire header DaemonCoordinator (daemon): - Error message payload was JSON-decoded without decrypting first; fixed to session-decrypt before decoding - withTaskGroup approach hung forever on timeout — withCheckedContinuation never resumes when task is cancelled; replaced with single continuation + two unstructured Tasks racing (response vs timeout), both using OnceContinuation so the first resume wins and the rest are no-ops - FAILED_KEY_INVALIDATED audit entry was written after continuation.resume; moved before so callers reading the log immediately after await always see it WireFormat (protocol): - JSONEncoder escaped '/' as '\/' by default, doubling base64 signatures that contain many slashes and pushing BLE packets over 256 bytes; added .withoutEscapingSlashes output formatting BLEServerInterface (daemon): - Extracted protocol from BLEServer so tests can inject MockBLEServer without requiring CoreBluetooth or real Bluetooth hardware DaemonCoordinatorTests (new): - 21 integration tests covering session lifecycle, ECDH, identify-on- reconnect, auth happy path, timeout, invalid signature, key invalidated fast-fail, multi-device broadcast, first-response-wins, reconnect - All 96 tests pass (swift test --filter DaemonCoordinator confirmed)
1 parent 99ec25f commit 09fd623

6 files changed

Lines changed: 904 additions & 59 deletions

File tree

companion/TouchBridge/Core/ChallengeHandler.swift

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,23 +44,30 @@ public final class ChallengeHandler: @unchecked Sendable {
4444
return .failed(.noSession)
4545
}
4646

47-
let decryptedPayload: Data
48-
do {
49-
decryptedPayload = try session.decrypt(ciphertext: encryptedData)
50-
} catch {
51-
logger.error("Failed to decrypt challenge: \(error.localizedDescription)")
52-
return .failed(.decryptionFailed)
47+
// 2. Parse — the challenge arrives as wire format [version, type] + plain JSON.
48+
// Strip the 2-byte header before JSON-decoding. The nonce inside is separately encrypted.
49+
guard encryptedData.count > 2 else {
50+
logger.error("Challenge data too short: \(encryptedData.count) bytes")
51+
return .failed(.parseFailed)
5352
}
54-
55-
// 2. Parse
53+
let jsonPayload = encryptedData.dropFirst(2)
5654
let challengeMsg: ChallengeIssuedMessageCompanion
5755
do {
58-
challengeMsg = try JSONDecoder().decode(ChallengeIssuedMessageCompanion.self, from: decryptedPayload)
56+
challengeMsg = try JSONDecoder().decode(ChallengeIssuedMessageCompanion.self, from: jsonPayload)
5957
} catch {
6058
logger.error("Failed to parse challenge: \(error.localizedDescription)")
6159
return .failed(.parseFailed)
6260
}
6361

62+
// Decrypt the nonce — only the nonce field is AES-GCM encrypted, not the whole message.
63+
let decryptedNonce: Data
64+
do {
65+
decryptedNonce = try session.decrypt(ciphertext: challengeMsg.encryptedNonce)
66+
} catch {
67+
logger.error("Failed to decrypt nonce: \(error.localizedDescription)")
68+
return .failed(.decryptionFailed)
69+
}
70+
6471
// Check expiry
6572
let expiryDate = Date(timeIntervalSince1970: TimeInterval(challengeMsg.expiryUnix))
6673
guard Date() < expiryDate else {
@@ -81,10 +88,10 @@ public final class ChallengeHandler: @unchecked Sendable {
8188
return .failed(.biometricDenied)
8289
}
8390

84-
// 4. Sign nonce
91+
// 4. Sign the decrypted nonce — daemon verifies against the plaintext nonce it stored.
8592
let signature: Data
8693
do {
87-
signature = try signingProvider.sign(data: challengeMsg.encryptedNonce, keyTag: signingKeyTag)
94+
signature = try signingProvider.sign(data: decryptedNonce, keyTag: signingKeyTag)
8895
} catch SecureEnclaveError.keyInvalidated {
8996
logger.error("Signing key invalidated — biometric enrollment changed since pairing")
9097
// Notify the Mac immediately so it fails fast instead of waiting for timeout.
@@ -102,10 +109,13 @@ public final class ChallengeHandler: @unchecked Sendable {
102109
deviceID: deviceID
103110
)
104111

105-
// 6. Encode
112+
// 6. Encode — daemon expects wire format: [version=1][type=4(challengeResponse)] + JSON
106113
let responseData: Data
107114
do {
108-
responseData = try JSONEncoder().encode(response)
115+
let jsonData = try JSONEncoder().encode(response)
116+
var wire = Data([1, 4]) // protocolVersion=1, MessageType.challengeResponse=4
117+
wire.append(jsonData)
118+
responseData = wire
109119
} catch {
110120
logger.error("Failed to encode response: \(error.localizedDescription)")
111121
return .failed(.encodingFailed)

companion/TouchBridge/Core/CompanionCoordinator.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ public final class CompanionCoordinator: NSObject, @unchecked Sendable {
5757

5858
bleClient.delegate = self
5959

60+
// If already paired, scan only for the paired Mac's unique service UUID.
61+
// This prevents connecting to other people's TouchBridge Macs nearby.
62+
if let storedUUID = UserDefaults.standard.string(forKey: "pairedMacID"),
63+
!storedUUID.isEmpty {
64+
bleClient.serviceUUID = storedUUID
65+
}
66+
6067
// Wire challenge handler's send callback to BLE
6168
challengeHandler.sendResponse = { [weak self] data in
6269
self?.bleClient.sendResponse(data) ?? false

daemon/Sources/TouchBridgeCore/BLEServer.swift

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,53 @@ import CryptoKit
44
import OSLog
55
import TouchBridgeProtocol
66

7+
// MARK: - BLE Server Interface
8+
9+
/// Abstraction over the BLE peripheral server.
10+
///
11+
/// Extracted as a protocol so tests can inject a `MockBLEServer` without
12+
/// requiring CoreBluetooth or real Bluetooth hardware.
13+
public protocol BLEServerInterface: AnyObject {
14+
/// The delegate that receives BLE events.
15+
var delegate: BLEServerDelegate? { get set }
16+
17+
func startAdvertising()
18+
func stopAdvertising()
19+
20+
/// Send an encrypted challenge to a connected companion. Returns true if sent.
21+
@discardableResult func sendChallenge(_ data: Data, to centralID: UUID) -> Bool
22+
23+
/// Send pairing data to a connected companion. Returns true if sent.
24+
@discardableResult func sendPairingData(_ data: Data, to centralID: UUID) -> Bool
25+
26+
/// Send session key data to a connected companion. Returns true if sent.
27+
@discardableResult func sendSessionKey(_ data: Data, to centralID: UUID) -> Bool
28+
29+
/// UUIDs of currently connected centrals.
30+
var connectedCentralIDs: [UUID] { get }
31+
32+
/// Rolling-average RSSI for a connected central, or nil if unavailable.
33+
func averageRSSI(for centralID: UUID) -> Int?
34+
}
35+
736
// MARK: - Delegate Protocol
837

938
/// Events emitted by the BLE GATT server to the daemon coordinator.
1039
public protocol BLEServerDelegate: AnyObject {
1140
/// A companion device connected.
12-
func bleServer(_ server: BLEServer, centralDidConnect centralID: UUID)
41+
func bleServer(_ server: any BLEServerInterface, centralDidConnect centralID: UUID)
1342

1443
/// A companion device disconnected.
15-
func bleServer(_ server: BLEServer, centralDidDisconnect centralID: UUID)
44+
func bleServer(_ server: any BLEServerInterface, centralDidDisconnect centralID: UUID)
1645

1746
/// ECDH session key received from companion; returns our public key bytes to send back.
18-
func bleServer(_ server: BLEServer, didReceiveSessionKey data: Data, from centralID: UUID) -> Data?
47+
func bleServer(_ server: any BLEServerInterface, didReceiveSessionKey data: Data, from centralID: UUID) -> Data?
1948

2049
/// Pairing data received from companion.
21-
func bleServer(_ server: BLEServer, didReceivePairingData data: Data, from centralID: UUID)
50+
func bleServer(_ server: any BLEServerInterface, didReceivePairingData data: Data, from centralID: UUID)
2251

2352
/// Signed challenge response received from companion.
24-
func bleServer(_ server: BLEServer, didReceiveResponse data: Data, from centralID: UUID)
53+
func bleServer(_ server: any BLEServerInterface, didReceiveResponse data: Data, from centralID: UUID)
2554
}
2655

2756
// MARK: - Connected Central Tracking
@@ -58,7 +87,7 @@ struct ConnectedCentral {
5887
/// - Challenge delivery (Mac → iPhone via notify)
5988
/// - Response reception (iPhone → Mac via write)
6089
/// - Pairing flow (bidirectional)
61-
public class BLEServer: NSObject {
90+
public class BLEServer: NSObject, BLEServerInterface {
6291
private let logger = Logger(subsystem: "dev.touchbridge", category: "BLEServer")
6392

6493
private var peripheralManager: CBPeripheralManager!

daemon/Sources/TouchBridgeCore/DaemonCoordinator.swift

Lines changed: 87 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public final class DaemonCoordinator: NSObject, PAMAuthHandler, @unchecked Senda
1313
private let logger = Logger(subsystem: "dev.touchbridge", category: "Coordinator")
1414

1515
// Components
16-
public let bleServer: BLEServer
16+
public let bleServer: any BLEServerInterface
1717
public let challengeManager: ChallengeManager
1818
public let pairingManager: PairingManager
1919
public let keychainStore: KeychainStore
@@ -23,7 +23,9 @@ public final class DaemonCoordinator: NSObject, PAMAuthHandler, @unchecked Senda
2323
private var sessions: [UUID: SessionState] = [:]
2424

2525
/// Pending PAM authentications awaiting challenge results.
26-
private var pendingAuthentications: [UUID: CheckedContinuation<ChallengeResult, Never>] = [:]
26+
/// Each auth broadcast stores the same OnceContinuation under multiple challenge IDs
27+
/// (one per device), so the first valid response wins and subsequent responses are no-ops.
28+
private var pendingAuthentications: [UUID: OnceContinuation] = [:]
2729

2830
/// Callback invoked when a challenge is verified or fails.
2931
public var onChallengeResult: ((UUID, ChallengeResult, String?) -> Void)?
@@ -43,12 +45,13 @@ public final class DaemonCoordinator: NSObject, PAMAuthHandler, @unchecked Senda
4345
challengeManager: ChallengeManager = ChallengeManager(),
4446
pairingManager: PairingManager? = nil,
4547
rssiThreshold: Int = TouchBridgeConstants.defaultRSSIThreshold,
46-
serviceUUID: String = TouchBridgeConstants.serviceUUID
48+
serviceUUID: String = TouchBridgeConstants.serviceUUID,
49+
bleServer: (any BLEServerInterface)? = nil
4750
) {
4851
self.keychainStore = keychainStore
4952
self.auditLog = auditLog
5053
self.challengeManager = challengeManager
51-
self.bleServer = BLEServer(rssiThreshold: rssiThreshold, serviceUUID: serviceUUID)
54+
self.bleServer = bleServer ?? BLEServer(rssiThreshold: rssiThreshold, serviceUUID: serviceUUID)
5255

5356
let pm = pairingManager ?? PairingManager(keychainStore: keychainStore, serviceUUID: serviceUUID)
5457
self.pairingManager = pm
@@ -157,38 +160,39 @@ public final class DaemonCoordinator: NSObject, PAMAuthHandler, @unchecked Senda
157160

158161
logger.info("PAM auth: broadcasting challenge to \(targets.count) device(s)")
159162

160-
// Issue challenges to all identified devices and race for the first valid response.
161-
// Each challengeID maps to the same continuation — first response wins, subsequent
162-
// responses find their entry already removed from pendingAuthentications (no-op).
163-
let result: ChallengeResult? = await withTaskGroup(of: ChallengeResult?.self) { group in
164-
group.addTask {
165-
await withCheckedContinuation { (continuation: CheckedContinuation<ChallengeResult, Never>) in
166-
Task {
167-
var issued = 0
168-
for centralID in targets {
169-
if let challengeID = await self.issueChallenge(to: centralID, reason: service) {
170-
self.pendingAuthentications[challengeID] = continuation
171-
issued += 1
172-
}
173-
}
174-
// If every issueChallenge call failed (BLE queue full, etc.) fail immediately.
175-
if issued == 0 {
176-
continuation.resume(returning: .unknownChallenge)
177-
}
178-
// Otherwise: wait — the first device response resumes the continuation.
163+
// Race: first device response OR global timeout — whichever fires first wins.
164+
//
165+
// We use a single withCheckedContinuation + two unstructured Tasks (challenge dispatch
166+
// and timeout) rather than withTaskGroup, because withTaskGroup waits for ALL child
167+
// tasks to finish after the closure returns. That means if the timeout fires first,
168+
// the group would hang indefinitely waiting for the challenge-dispatch task to return
169+
// — which it never does because withCheckedContinuation only returns when resumed.
170+
//
171+
// With two unstructured Tasks sharing one OnceContinuation, the first resume wins;
172+
// subsequent resumes (from late responses or cleanup) are silent no-ops.
173+
let result: ChallengeResult? = await withCheckedContinuation { outer in
174+
let wrapped = OnceContinuation(outer)
175+
176+
// Challenge-dispatch task: issue to all identified devices, then just wait.
177+
Task {
178+
var issued = 0
179+
for centralID in targets {
180+
if let challengeID = await self.issueChallenge(to: centralID, reason: service) {
181+
self.pendingAuthentications[challengeID] = wrapped
182+
issued += 1
179183
}
180184
}
185+
if issued == 0 {
186+
wrapped.resume(returning: .unknownChallenge)
187+
}
188+
// Otherwise: wait — the first device response (or timeout below) resumes outer.
181189
}
182190

183-
group.addTask {
184-
// Global timeout covers the entire broadcast, not per-device.
191+
// Timeout task: resumes with nil after the deadline.
192+
Task {
185193
try? await Task.sleep(nanoseconds: UInt64(timeout * 1_000_000_000))
186-
return nil
194+
wrapped.resume(returning: nil)
187195
}
188-
189-
let first = await group.next() ?? nil
190-
group.cancelAll()
191-
return first
192196
}
193197

194198
if let result, result == .verified {
@@ -219,17 +223,17 @@ public final class DaemonCoordinator: NSObject, PAMAuthHandler, @unchecked Senda
219223

220224
extension DaemonCoordinator: BLEServerDelegate {
221225

222-
public func bleServer(_ server: BLEServer, centralDidConnect centralID: UUID) {
226+
public func bleServer(_ server: any BLEServerInterface, centralDidConnect centralID: UUID) {
223227
logger.info("Central connected: \(centralID)")
224228
sessions[centralID] = SessionState()
225229
}
226230

227-
public func bleServer(_ server: BLEServer, centralDidDisconnect centralID: UUID) {
231+
public func bleServer(_ server: any BLEServerInterface, centralDidDisconnect centralID: UUID) {
228232
logger.info("Central disconnected: \(centralID)")
229233
sessions.removeValue(forKey: centralID)
230234
}
231235

232-
public func bleServer(_ server: BLEServer, didReceiveSessionKey data: Data, from centralID: UUID) -> Data? {
236+
public func bleServer(_ server: any BLEServerInterface, didReceiveSessionKey data: Data, from centralID: UUID) -> Data? {
233237
logger.info("Received session key from \(centralID)")
234238

235239
do {
@@ -255,11 +259,20 @@ extension DaemonCoordinator: BLEServerDelegate {
255259
}
256260
}
257261

258-
public func bleServer(_ server: BLEServer, didReceivePairingData data: Data, from centralID: UUID) {
262+
public func bleServer(_ server: any BLEServerInterface, didReceivePairingData data: Data, from centralID: UUID) {
259263
logger.info("Received pairing data from \(centralID)")
260264

261265
Task {
262266
do {
267+
// Detect wire-format messages (version=1, valid type byte) vs legacy raw-JSON pairing.
268+
// Identify messages always use wire format; pairing requests use raw JSON currently.
269+
if data.count >= 2, data[data.startIndex] == 1,
270+
let msgType = MessageType(rawValue: data[data.startIndex + 1]),
271+
msgType == .identify {
272+
try await handleIdentify(data: data, from: centralID)
273+
return
274+
}
275+
263276
let (_, payload) = try WireFormat.decode(data: data)
264277
let request = try WireFormat.decodePayload(PairRequestMessage.self, from: payload)
265278

@@ -307,30 +320,38 @@ extension DaemonCoordinator: BLEServerDelegate {
307320
}
308321
}
309322

310-
public func bleServer(_ server: BLEServer, didReceiveResponse data: Data, from centralID: UUID) {
323+
public func bleServer(_ server: any BLEServerInterface, didReceiveResponse data: Data, from centralID: UUID) {
311324
logger.info("Received challenge response from \(centralID)")
312325

313326
Task {
314327
do {
315328
let (msgType, payload) = try WireFormat.decode(data: data)
316329

317330
// Handle companion error messages (e.g. key invalidated).
331+
// The error payload is AES-GCM encrypted with the session key.
318332
if msgType == .error {
319-
let errMsg = try WireFormat.decodePayload(ErrorMessage.self, from: payload)
333+
guard let crypto = sessions[centralID]?.sessionCrypto else {
334+
logger.warning("Error message from \(centralID) but no session crypto — ignoring")
335+
return
336+
}
337+
let plaintext = try crypto.decrypt(ciphertext: payload)
338+
let errMsg = try WireFormat.decodePayload(ErrorMessage.self, from: plaintext)
320339
logger.warning("Companion error \(errMsg.code): \(errMsg.description)")
321340

322341
if errMsg.code == ErrorCode.keyInvalidated.rawValue,
323342
let cidStr = errMsg.challengeID,
324343
let challengeID = UUID(uuidString: cidStr) {
325-
if let continuation = pendingAuthentications.removeValue(forKey: challengeID) {
326-
continuation.resume(returning: .keyInvalidated)
327-
}
344+
// Log before resuming so callers that await the result immediately
345+
// and then read the audit log always see the entry.
328346
await auditLog.log(AuditEntry(
329347
sessionID: cidStr,
330348
surface: "challenge",
331349
deviceID: errMsg.description,
332350
result: "FAILED_KEY_INVALIDATED"
333351
))
352+
if let continuation = pendingAuthentications.removeValue(forKey: challengeID) {
353+
continuation.resume(returning: .keyInvalidated)
354+
}
334355
}
335356
return
336357
}
@@ -428,3 +449,30 @@ extension DaemonCoordinator: BLEServerDelegate {
428449
))
429450
}
430451
}
452+
453+
// MARK: - OnceContinuation
454+
455+
/// Wraps a `CheckedContinuation` so it can safely be resumed at most once.
456+
///
457+
/// `authenticateFromPAM` stores the same `OnceContinuation` under every challenge ID
458+
/// it broadcasts. The first resume (from a device response or the timeout Task) wins;
459+
/// all subsequent calls are silent no-ops.
460+
///
461+
/// Holds `ChallengeResult?` so the timeout Task can pass `nil` ("no result = timeout")
462+
/// while device-response tasks pass a concrete `ChallengeResult`.
463+
final class OnceContinuation: @unchecked Sendable {
464+
private var inner: CheckedContinuation<ChallengeResult?, Never>?
465+
private let lock = NSLock()
466+
467+
init(_ continuation: CheckedContinuation<ChallengeResult?, Never>) {
468+
self.inner = continuation
469+
}
470+
471+
func resume(returning value: ChallengeResult?) {
472+
lock.withLock {
473+
guard let c = inner else { return }
474+
inner = nil
475+
c.resume(returning: value)
476+
}
477+
}
478+
}

0 commit comments

Comments
 (0)