diff --git a/app/src/main/java/com/bitchat/android/mesh/BluetoothMeshService.kt b/app/src/main/java/com/bitchat/android/mesh/BluetoothMeshService.kt index 22f17867..b71b4a15 100644 --- a/app/src/main/java/com/bitchat/android/mesh/BluetoothMeshService.kt +++ b/app/src/main/java/com/bitchat/android/mesh/BluetoothMeshService.kt @@ -654,7 +654,7 @@ class BluetoothMeshService(private val context: Context) : TransportBridgeServic // Decode request and respond with missing packets val fromPeer = routed.peerID ?: return val req = RequestSyncPacket.decode(routed.packet.payload) ?: return - gossipSyncManager.handleRequestSync(fromPeer, req) + gossipSyncManager.handleRequestSync(fromPeer, req, routed.ingressLinkID) } } diff --git a/app/src/main/java/com/bitchat/android/mesh/MeshCore.kt b/app/src/main/java/com/bitchat/android/mesh/MeshCore.kt index 28cd3d02..4e3993f2 100644 --- a/app/src/main/java/com/bitchat/android/mesh/MeshCore.kt +++ b/app/src/main/java/com/bitchat/android/mesh/MeshCore.kt @@ -525,7 +525,7 @@ class MeshCore( override fun handleRequestSync(routed: RoutedPacket) { val fromPeer = routed.peerID ?: return val req = RequestSyncPacket.decode(routed.packet.payload) ?: return - gossipSyncManager.handleRequestSync(fromPeer, req) + gossipSyncManager.handleRequestSync(fromPeer, req, routed.ingressLinkID) } } } diff --git a/app/src/main/java/com/bitchat/android/sync/GossipSyncManager.kt b/app/src/main/java/com/bitchat/android/sync/GossipSyncManager.kt index bc2465d9..71fd19c2 100644 --- a/app/src/main/java/com/bitchat/android/sync/GossipSyncManager.kt +++ b/app/src/main/java/com/bitchat/android/sync/GossipSyncManager.kt @@ -200,11 +200,11 @@ class GossipSyncManager( } /** - * Charges [fromPeerID] for one request and returns how many packets it may be sent in - * response, or 0 when the request must be dropped. Called before any decoding or hashing so - * a flood costs us the admission check and nothing else. + * Charges [budgetKey] for one request and returns how many packets may be sent in response, + * or 0 when the request must be dropped. Called before any decoding or hashing so a flood + * costs us the admission check and nothing else. */ - private fun admitRequest(fromPeerID: String): Int { + private fun admitRequest(budgetKey: String): Int { val capacity = try { configProvider.seenCapacity() } catch (_: Exception) { @@ -213,7 +213,7 @@ class GossipSyncManager( val now = nowMs() synchronized(requesterBudgets) { - val budget = requesterBudgets.getOrPut(fromPeerID) { + val budget = requesterBudgets.getOrPut(budgetKey) { RequesterBudget( requestTokens = AppConstants.Sync.REQUEST_BURST.toDouble(), responseTokens = capacity, @@ -245,16 +245,28 @@ class GossipSyncManager( requesterBudgets.size } - private fun chargeResponses(fromPeerID: String, sent: Int) { + private fun chargeResponses(budgetKey: String, sent: Int) { if (sent <= 0) return synchronized(requesterBudgets) { - val budget = requesterBudgets[fromPeerID] ?: return + val budget = requesterBudgets[budgetKey] ?: return budget.responseTokens = (budget.responseTokens - sent).coerceAtLeast(0.0) } } - fun handleRequestSync(fromPeerID: String, request: RequestSyncPacket) { - var remaining = admitRequest(fromPeerID) + /** + * @param ingressLinkID locally-assigned identity of the link the request arrived on. REQUEST_SYNC + * is not in the set of types `SecurityManager.verifyPacketSignature` authenticates, so the + * sender ID on the wire is unauthenticated and free to rotate — budgeting by it would let a + * peer mint a fresh allowance per request. This value is assigned by the transport and never + * serialized onto the mesh, so a flooder is bounded by the links it can actually hold open. + */ + fun handleRequestSync( + fromPeerID: String, + request: RequestSyncPacket, + ingressLinkID: String? = null + ) { + val budgetKey = ingressLinkID ?: fromPeerID + var remaining = admitRequest(budgetKey) if (remaining <= 0) { Log.d(TAG, "Dropping REQUEST_SYNC from $fromPeerID: over budget") return @@ -296,7 +308,7 @@ class GossipSyncManager( } } - chargeResponses(fromPeerID, allowance - remaining) + chargeResponses(budgetKey, allowance - remaining) } private fun hexStringToByteArray(hexString: String): ByteArray { diff --git a/app/src/test/kotlin/com/bitchat/android/sync/GossipSyncRequestBudgetTest.kt b/app/src/test/kotlin/com/bitchat/android/sync/GossipSyncRequestBudgetTest.kt index 1def67b9..f131589e 100644 --- a/app/src/test/kotlin/com/bitchat/android/sync/GossipSyncRequestBudgetTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/sync/GossipSyncRequestBudgetTest.kt @@ -153,6 +153,35 @@ class GossipSyncRequestBudgetTest { ) } + @Test + fun `rotating the sender id does not mint a fresh budget on one link`() { + storeMessages(CAPACITY) + + // REQUEST_SYNC is not in the set SecurityManager.verifyPacketSignature authenticates, so + // the sender ID is free for a flooder to rotate. Cycling more IDs than the LRU tracks + // would hand out a full allowance per request if the budget were keyed on it. + repeat(AppConstants.Sync.MAX_TRACKED_REQUESTERS * 2) { i -> + manager.handleRequestSync("%016x".format(i), emptyFilter, ingressLinkID = "link-1") + } + + assertEquals(CAPACITY, sentTo.size) + assertEquals(1, manager.trackedRequesterCount()) + } + + @Test + fun `separate links keep separate budgets`() { + storeMessages(CAPACITY) + + manager.handleRequestSync(REQUESTER, emptyFilter, ingressLinkID = "link-1") + repeat(50) { manager.handleRequestSync(REQUESTER, emptyFilter, ingressLinkID = "link-1") } + sentTo.clear() + + // A different physical link is a different neighbour and must not inherit the throttle. + manager.handleRequestSync(REQUESTER, emptyFilter, ingressLinkID = "link-2") + + assertEquals(CAPACITY, sentTo.size) + } + @Test fun `a backwards clock jump does not hand out free budget`() { storeMessages(CAPACITY)