feat(bluetooth): Add trusted device pairing and encrypted sync#3223
feat(bluetooth): Add trusted device pairing and encrypted sync#3223blacklight wants to merge 6 commits into
Conversation
- Pairing now required before a watch can sync loyalty cards over Bluetooth. - Adds encryption, trusted/blocked device management, and user-facing notifications for allow/block decisions.
TheLastProject
left a comment
There was a problem hiding this comment.
I haven't tested this code yet, just looking through it. I'm definitely struggling at some parts. I understand encrypting adds complexity, but the general communication flow has become hard to understand so I do think we'll probably have to change this a bit.
Perhaps a flow more like this would be easier to maintain?
Each onResume:
- Watch calls /VERSIONS (always cleartext) to see which API version is supported
- Watch call /AUTH with a public key, if the MAC + pubkey combination is known by the Android app, return OK with a pubkey the watch should use. Otherwise return UNAUTHORIZED (on Unauthorized, show an alert on the watch)
- If OK, call the card sync, etc.
So, basically, the old flow with just 1 additional step. The decryption is just hooked up in the regular flow so the regular loop doesn't have to encrypt/decrypt, it just sees the decrypted data.
But maybe I'm missing something :)
| val adapter = context.getSystemService(BLUETOOTH_SERVICE) as? BluetoothManager | ||
| val bondedDevices = try { | ||
| adapter?.adapter?.bondedDevices ?: emptySet() | ||
| } catch (_: SecurityException) { | ||
| emptySet<BluetoothDevice>() | ||
| } | ||
| val deviceMap = bondedDevices.associateBy { it.address } | ||
|
|
||
| val entries = knownAddresses.map { address -> | ||
| val name = deviceName(deviceMap[address], address) | ||
| getString(R.string.settings_wear_sync_device_name, name, address) | ||
| }.toTypedArray() |
There was a problem hiding this comment.
Just so I get this right: you're using a local list of trusted devices, but you only store the mac address, and you get the device name from the Bluetooth adapter? Or am I misunderstanding?
There was a problem hiding this comment.
The display name is fetched from BluetoothAdapter.getBondedDevices() at runtime, and falls back to the raw MAC address if the device isn't bonded or the name can't be read. The stored "trusted device" data itself is just the MAC + key.
| fun isPostNotificationsGranted(context: Context): Boolean = | ||
| Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU || | ||
| ContextCompat.checkSelfPermission( | ||
| context, | ||
| Manifest.permission.POST_NOTIFICATIONS | ||
| ) == PackageManager.PERMISSION_GRANTED | ||
|
|
There was a problem hiding this comment.
It feels kinda inconsistent to me that for Bluetooth this is split into 2 functions (required + granted), but for notifications it's not.
| val completed = AtomicBoolean(false) | ||
| val handler = Handler(Looper.getMainLooper()) | ||
| val timeoutRunnable = Runnable { | ||
| if (completed.compareAndSet(false, true)) { | ||
| Log.w(TAG, "Bluetooth card sync timed out") | ||
| onResult(null, SyncStatus.PHONE_NOT_REACHABLE) | ||
| } | ||
| } | ||
| handler.postDelayed(timeoutRunnable, 20000) | ||
|
|
There was a problem hiding this comment.
The 20 seconds timeout caps the entire Bluetooth card sync operation. If the background sync doesn't finish within 20 seconds, the Runnable fires PHONE_NOT_REACHABLE.
| } finally { | ||
| handler.removeCallbacks(timeoutRunnable) | ||
| if (completed.compareAndSet(false, true)) { | ||
| onResult(result, status) |
There was a problem hiding this comment.
Also don't really understand this change here
There was a problem hiding this comment.
This thread runs the actual Bluetooth work. If it finishes first, it removes the timeout and calls onResult with the real result.
The timeoutRunnable is the 20 s safety net. If the thread is stuck connecting or waiting on I/O, the runnable fires and reports PHONE_NOT_REACHABLE.
| WearBluetoothProtocol.BT_CMD_AUTH, | ||
| WearBluetoothProtocol.BT_CMD_AUTH_RESET -> { | ||
| // Re-exchange the key on every auth so the watch can never use a stale key | ||
| // after the phone has reset/trusted it again. | ||
| handleAuthenticatedSession(reader, writer, address, deviceName) | ||
| } |
There was a problem hiding this comment.
I'm a bit confused what's happening here.
Is WearBluetoothProtocol.BT_CMD_AUTH sent in cleartext and then all the sync commands are sent in non-cleartext? I find that the flow has gotten quite hard to understand (and maintain/extend) here.
There was a problem hiding this comment.
This is a common pattern in simple symmetric key encryption. The key is exchanged in cleartext and then all the following communication will use that key. See #3223 (comment) for my comment on the current encryption protocol.
No official release has been done yet -> no need to bump the protocol version. Addresses: CatimaLoyalty#3223 (comment)
The Bluetooth pairing authorization notification was using `NotificationCompat.CATEGORY_CALL`, which is semantically meant for incoming phone/video calls. `CATEGORY_STATUS` is a better fit for a device connection request since no call is involved - even though it's still not the semantically perferct choice.
…IBLE The watch cannot know which side of a protocol mismatch is older, so PHONE_OUTDATED and WATCH_OUTDATED are replaced by a neutral VERSION_INCOMPATIBLE status. The UI now tells the user to update both apps, and the watch stops retrying sync when versions are incompatible. Addresses: CatimaLoyalty#3223 (comment)
Bluetooth Wear Sync Security Implementation
Current State
protect.card_locker.wearos.BluetoothServerServiceis a Classic Bluetooth RFCOMM server (not BLE). It advertises the UUIDe5b4f020-3a7e-4b6d-9f2c-1a8c5d3e7f90with the nameCatimaWear.me.hackerchick.catima.wear.BluetoothCardClient) scans the phone's bonded devices and tries to connect to each one using the same UUID.listenUsingRfcommWithServiceRecord/createRfcommSocketToServiceRecord), which means the Bluetooth link is encrypted and authenticated by the OS pairing/link key.Threat Model
/V1/CARDS_REQUEST_PAGE/<n>) until the full set is exfiltrated.Proposed Mitigations
1. App-level pairing confirmation
AUTH_REQUIREDand show a notification/activity on the phone:<device name>to access your cards?"2. Paired devices list in Settings
3. Application-layer encryption
/V2/AUTHhandshake. The key exchange happens over the already authenticated and encrypted RFCOMM link.<base64(nonce)>:<base64(ciphertext+tag)>\n.Limitations and Caveats