mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-09-19 04:59:59 +00:00
fix(ble): defer the broadcast feed until a verified ANNOUNCE, don't reject the CCCD
Android clients write the CCCD during discovery, before they announce, and never retry. Rejecting that write left a legitimate peer unsubscribed even after the announce landed.
This commit is contained in:
parent
6e53bef71e
commit
78615b7951
@ -30,6 +30,9 @@ class BluetoothConnectionTracker(
|
||||
val addressPeerMap = ConcurrentHashMap<String, String>()
|
||||
// Track whether we have seen the first ANNOUNCE on a given device connection
|
||||
private val firstAnnounceSeen = ConcurrentHashMap<String, Boolean>()
|
||||
// CCCD writes that arrived before a verified ANNOUNCE. Grant the feed later
|
||||
// rather than rejecting the descriptor — Android clients do not retry it.
|
||||
private val pendingBroadcastSubscriptions = ConcurrentHashMap<String, BluetoothDevice>()
|
||||
// RSSI tracking from scan results (for devices we discover but may connect as servers)
|
||||
private val scanRSSI = ConcurrentHashMap<String, Int>()
|
||||
private val connectionStateLock = Any()
|
||||
@ -85,6 +88,7 @@ class BluetoothConnectionTracker(
|
||||
removePendingConnection(deviceAddress)
|
||||
// Mark as awaiting first ANNOUNCE on this connection
|
||||
firstAnnounceSeen[deviceAddress] = false
|
||||
pendingBroadcastSubscriptions.remove(deviceAddress)
|
||||
}
|
||||
|
||||
/**
|
||||
@ -276,6 +280,7 @@ class BluetoothConnectionTracker(
|
||||
subscribedDevices.removeAll { it.address == deviceAddress }
|
||||
addressPeerMap.remove(deviceAddress)
|
||||
firstAnnounceSeen.remove(deviceAddress)
|
||||
pendingBroadcastSubscriptions.remove(deviceAddress)
|
||||
}
|
||||
Log.d(TAG, "Cleaned up device connection for $deviceAddress")
|
||||
}
|
||||
@ -292,6 +297,7 @@ class BluetoothConnectionTracker(
|
||||
subscribedDevices.removeAll { it.address == deviceAddress }
|
||||
addressPeerMap.remove(deviceAddress)
|
||||
firstAnnounceSeen.remove(deviceAddress)
|
||||
pendingBroadcastSubscriptions.remove(deviceAddress)
|
||||
Log.d(TAG, "Cleaned up device connection for $deviceAddress")
|
||||
true
|
||||
} else {
|
||||
@ -330,13 +336,47 @@ class BluetoothConnectionTracker(
|
||||
pendingConnections.clear()
|
||||
scanRSSI.clear()
|
||||
firstAnnounceSeen.clear()
|
||||
pendingBroadcastSubscriptions.clear()
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark that we have received the first ANNOUNCE over this device connection.
|
||||
* Record a CCCD enable. Grants the broadcast feed immediately when a verified
|
||||
* ANNOUNCE already arrived on this connection; otherwise defers until it does.
|
||||
*
|
||||
* @return true if the feed was granted now
|
||||
*/
|
||||
fun requestBroadcastSubscription(device: BluetoothDevice): Boolean {
|
||||
synchronized(connectionStateLock) {
|
||||
val action = GattNotificationEligibility.action(
|
||||
firstAnnounceSeen[device.address] == true
|
||||
)
|
||||
if (action == GattSubscriptionAction.GRANT) {
|
||||
pendingBroadcastSubscriptions.remove(device.address)
|
||||
addSubscribedDeviceIfAbsent(device)
|
||||
return true
|
||||
}
|
||||
pendingBroadcastSubscriptions[device.address] = device
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark that we have received the first ANNOUNCE over this device connection
|
||||
* and grant any CCCD write that was waiting on it.
|
||||
*/
|
||||
fun noteAnnounceReceived(deviceAddress: String) {
|
||||
firstAnnounceSeen[deviceAddress] = true
|
||||
synchronized(connectionStateLock) {
|
||||
firstAnnounceSeen[deviceAddress] = true
|
||||
pendingBroadcastSubscriptions.remove(deviceAddress)?.let { device ->
|
||||
addSubscribedDeviceIfAbsent(device)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun addSubscribedDeviceIfAbsent(device: BluetoothDevice) {
|
||||
if (subscribedDevices.none { it.address == device.address }) {
|
||||
subscribedDevices.add(device)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@ -283,28 +283,14 @@ class BluetoothGattServerManager(
|
||||
}
|
||||
|
||||
if (BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE.contentEquals(value)) {
|
||||
if (!GattNotificationEligibility.maySubscribe(
|
||||
connectionTracker.hasSeenFirstAnnounce(device.address)
|
||||
)
|
||||
) {
|
||||
Log.w(
|
||||
val granted = connectionTracker.requestBroadcastSubscription(device)
|
||||
if (!granted) {
|
||||
Log.i(
|
||||
TAG,
|
||||
"Rejecting broadcast subscription from ${device.address} before verified ANNOUNCE"
|
||||
"Deferring broadcast feed for ${device.address} until verified ANNOUNCE"
|
||||
)
|
||||
if (responseNeeded) {
|
||||
gattServer?.sendResponse(
|
||||
device,
|
||||
requestId,
|
||||
BluetoothGatt.GATT_INSUFFICIENT_AUTHORIZATION,
|
||||
0,
|
||||
null
|
||||
)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
connectionTracker.addSubscribedDevice(device)
|
||||
|
||||
connectionScope.launch {
|
||||
delay(100)
|
||||
if (isActive) { // Check if still active
|
||||
|
||||
@ -1,11 +1,21 @@
|
||||
package com.bitchat.android.mesh
|
||||
|
||||
/**
|
||||
* Whether a BLE central may subscribe to the mesh broadcast notification feed.
|
||||
* Whether a BLE central may receive the mesh broadcast notification feed.
|
||||
*
|
||||
* Granting notifications before the peer has sent a verified ANNOUNCE would let
|
||||
* a passive listener harvest the full mesh feed with no participation (#901).
|
||||
* Granting the feed before the peer has sent a verified ANNOUNCE would let a
|
||||
* passive listener harvest it with no participation (#901). Android clients
|
||||
* write the CCCD during service discovery, before they send that ANNOUNCE, and
|
||||
* they do not retry the descriptor write — so a GATT reject here would leave a
|
||||
* legitimate peer unsubscribed even after they announce. Defer instead: accept
|
||||
* the CCCD write, withhold packets, then grant once the ANNOUNCE verifies.
|
||||
*/
|
||||
internal object GattNotificationEligibility {
|
||||
fun maySubscribe(hasVerifiedAnnounce: Boolean): Boolean = hasVerifiedAnnounce
|
||||
internal enum class GattSubscriptionAction {
|
||||
GRANT,
|
||||
DEFER,
|
||||
}
|
||||
|
||||
internal object GattNotificationEligibility {
|
||||
fun action(hasVerifiedAnnounce: Boolean): GattSubscriptionAction =
|
||||
if (hasVerifiedAnnounce) GattSubscriptionAction.GRANT else GattSubscriptionAction.DEFER
|
||||
}
|
||||
|
||||
@ -79,6 +79,58 @@ class BluetoothConnectionTrackerLinkObservationTest {
|
||||
assertTrue(tracker.addressPeerMap.containsValue(PEER_ID))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `cccd before announce withholds the feed then grants it`() {
|
||||
val address = "AA:BB:CC:DD:EE:FF"
|
||||
val device = mock<BluetoothDevice>()
|
||||
whenever(device.address).thenReturn(address)
|
||||
tracker.addDeviceConnection(
|
||||
address,
|
||||
BluetoothConnectionTracker.DeviceConnection(device = device, linkID = "link-a")
|
||||
)
|
||||
|
||||
assertFalse(tracker.requestBroadcastSubscription(device))
|
||||
assertTrue(tracker.getSubscribedDevices().none { it.address == address })
|
||||
|
||||
tracker.noteAnnounceReceived(address)
|
||||
assertTrue(tracker.getSubscribedDevices().any { it.address == address })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `announce before cccd grants the feed on the descriptor write`() {
|
||||
val address = "AA:BB:CC:DD:EE:00"
|
||||
val device = mock<BluetoothDevice>()
|
||||
whenever(device.address).thenReturn(address)
|
||||
tracker.addDeviceConnection(
|
||||
address,
|
||||
BluetoothConnectionTracker.DeviceConnection(device = device, linkID = "link-b")
|
||||
)
|
||||
|
||||
tracker.noteAnnounceReceived(address)
|
||||
assertTrue(tracker.requestBroadcastSubscription(device))
|
||||
assertTrue(tracker.getSubscribedDevices().any { it.address == address })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `disconnect drops a deferred broadcast subscription`() {
|
||||
val address = "AA:BB:CC:DD:EE:01"
|
||||
val device = mock<BluetoothDevice>()
|
||||
whenever(device.address).thenReturn(address)
|
||||
tracker.addDeviceConnection(
|
||||
address,
|
||||
BluetoothConnectionTracker.DeviceConnection(device = device, linkID = "link-c")
|
||||
)
|
||||
tracker.requestBroadcastSubscription(device)
|
||||
tracker.cleanupDeviceConnection(address)
|
||||
|
||||
tracker.addDeviceConnection(
|
||||
address,
|
||||
BluetoothConnectionTracker.DeviceConnection(device = device, linkID = "link-d")
|
||||
)
|
||||
tracker.noteAnnounceReceived(address)
|
||||
assertTrue(tracker.getSubscribedDevices().none { it.address == address })
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val PEER_ID = "0011223344556677"
|
||||
}
|
||||
|
||||
@ -1,17 +1,22 @@
|
||||
package com.bitchat.android.mesh
|
||||
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Test
|
||||
|
||||
class GattNotificationEligibilityTest {
|
||||
@Test
|
||||
fun `subscription denied before a verified announce`() {
|
||||
assertFalse(GattNotificationEligibility.maySubscribe(hasVerifiedAnnounce = false))
|
||||
fun `subscription is deferred before a verified announce`() {
|
||||
assertEquals(
|
||||
GattSubscriptionAction.DEFER,
|
||||
GattNotificationEligibility.action(hasVerifiedAnnounce = false)
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `subscription allowed after a verified announce`() {
|
||||
assertTrue(GattNotificationEligibility.maySubscribe(hasVerifiedAnnounce = true))
|
||||
fun `subscription is granted after a verified announce`() {
|
||||
assertEquals(
|
||||
GattSubscriptionAction.GRANT,
|
||||
GattNotificationEligibility.action(hasVerifiedAnnounce = true)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user