fix: address location privacy review

This commit is contained in:
callebtc 2026-07-27 16:07:15 +02:00
parent 09c5c74c09
commit 078f5b2644
7 changed files with 174 additions and 36 deletions

View File

@ -21,6 +21,7 @@ import androidx.lifecycle.repeatOnLifecycle
import androidx.lifecycle.Lifecycle
import com.bitchat.android.mesh.BluetoothMeshService
import com.bitchat.android.mesh.MeshService
import com.bitchat.android.geohash.LocationChannelManager
import com.bitchat.android.onboarding.BluetoothCheckScreen
import com.bitchat.android.onboarding.BluetoothStatus
import com.bitchat.android.onboarding.BluetoothStatusManager
@ -743,6 +744,9 @@ class MainActivity : OrientationAwareActivity() {
override fun onResume() {
super.onResume()
// Revoke stale live-location work before any resumed UI can use cached channels.
LocationChannelManager.getInstance(applicationContext).syncPermissionState()
// Check Bluetooth and Location status on resume and handle accordingly
if (mainViewModel.onboardingState.value == OnboardingState.COMPLETE) {
// Reattach mesh delegate to new ChatViewModel instance after Activity recreation

View File

@ -30,7 +30,16 @@ class AndroidGeocoderProvider(context: Context) : GeocoderProvider {
maxResults,
object : Geocoder.GeocodeListener {
override fun onGeocode(addresses: MutableList<Address>) {
if (cont.isActive) cont.resume(addresses)
if (cont.isActive) {
val result = if (liveLocationToken == null ||
LiveLocationPrivacyGate.accepts(liveLocationToken)
) {
addresses
} else {
emptyList()
}
cont.resume(result)
}
}
override fun onError(errorMessage: String?) {
@ -59,20 +68,25 @@ class AndroidGeocoderProvider(context: Context) : GeocoderProvider {
} else {
@Suppress("DEPRECATION")
try {
var addresses: List<Address> = emptyList()
val request = {
addresses = geocoder.getFromLocation(
latitude,
longitude,
maxResults
) ?: emptyList()
}
if (liveLocationToken == null) {
request()
if (liveLocationToken != null &&
!LiveLocationPrivacyGate.accepts(liveLocationToken)
) return emptyList()
// This legacy API blocks and cannot be cancelled. Never hold the privacy
// gate's read lock across the call: revocation must remain immediate.
val addresses = geocoder.getFromLocation(
latitude,
longitude,
maxResults
) ?: emptyList()
if (liveLocationToken == null ||
LiveLocationPrivacyGate.accepts(liveLocationToken)
) {
addresses
} else {
LiveLocationPrivacyGate.runIfAllowed(liveLocationToken, request)
emptyList()
}
addresses
} catch (e: Exception) {
Log.e(TAG, "Geocode failed")
emptyList()

View File

@ -7,6 +7,7 @@ import kotlinx.coroutines.*
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import java.util.UUID
/**
* Manages location notes (kind=1 text notes with geohash tags)
@ -378,7 +379,7 @@ class LocationNotesManager private constructor() {
since = null,
limit = 200
)
val subId = "location-notes-$gh"
val subId = "location-notes-${UUID.randomUUID()}"
try {
var id: String? = null
LiveLocationPrivacyGate.runIfAllowed(token) {

View File

@ -0,0 +1,13 @@
package com.bitchat.android.nostr
internal object NostrLiveSubscriptionPrivacy {
fun closeTargets(
liveSubscriptionIds: Set<String>,
subscriptionsByRelay: Map<String, Set<String>>,
): Map<String, Set<String>> = buildMap {
subscriptionsByRelay.forEach { (relayUrl, relaySubscriptionIds) ->
val matchingIds = relaySubscriptionIds.intersect(liveSubscriptionIds)
if (matchingIds.isNotEmpty()) put(relayUrl, matchingIds)
}
}
}

View File

@ -10,6 +10,7 @@ import com.google.gson.JsonArray
import com.google.gson.JsonParser
import kotlinx.coroutines.*
import okhttp3.*
import java.util.UUID
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.TimeUnit
import kotlin.math.min
@ -254,6 +255,33 @@ class NostrRelayManager private constructor() {
LiveLocationPrivacyGate.runIfAllowed(liveLocationToken, action)
}
/**
* Privacy teardown is allowed to bypass an already-revoked token solely to stop
* server-side delivery. Live subscription IDs are opaque, so CLOSE carries no
* geohash. If a CLOSE cannot be queued, fail closed by dropping that socket.
*/
private fun closeSubscriptionsOnConnectedRelays(subscriptionIds: Set<String>) {
if (subscriptionIds.isEmpty()) return
val closeTargets = NostrLiveSubscriptionPrivacy.closeTargets(
liveSubscriptionIds = subscriptionIds,
subscriptionsByRelay = subscriptions,
)
closeTargets.forEach { (relayUrl, relaySubscriptionIds) ->
val webSocket = connections[relayUrl] ?: return@forEach
relaySubscriptionIds.forEach { subscriptionId ->
val request = NostrRequest.Close(subscriptionId)
val message = gson.toJson(request, NostrRequest::class.java)
val closeQueued = runCatching { webSocket.send(message) }
.getOrDefault(false)
if (!closeQueued) {
connections.remove(relayUrl, webSocket)
webSocket.cancel()
}
}
}
}
private fun revokeLiveLocationAccess() {
liveLocationConnectionJobs.forEach(Job::cancel)
liveLocationConnectionJobs.clear()
@ -261,6 +289,7 @@ class NostrRelayManager private constructor() {
val liveSubscriptionIds = activeSubscriptions.values
.filter { it.liveLocationToken != null }
.mapTo(mutableSetOf()) { it.id }
closeSubscriptionsOnConnectedRelays(liveSubscriptionIds)
liveSubscriptionIds.forEach { id ->
activeSubscriptions.remove(id)
messageHandlers.remove(id)
@ -452,13 +481,13 @@ class NostrRelayManager private constructor() {
var success = false
runNetworkAction(subscriptionInfo.liveLocationToken) {
success = webSocket.send(message)
if (success) {
val currentSubs = subscriptions[relayUrl] ?: emptySet()
subscriptions[relayUrl] =
currentSubs + subscriptionInfo.id
}
}
if (success) {
// Track subscription for this relay
val currentSubs = subscriptions[relayUrl] ?: emptySet()
subscriptions[relayUrl] = currentSubs + subscriptionInfo.id
} else {
if (!success) {
Log.w(TAG, "Failed to send subscription: WebSocket send failed")
}
} catch (e: Exception) {
@ -485,11 +514,20 @@ class NostrRelayManager private constructor() {
return
}
if (subscriptionInfo.liveLocationToken != null &&
!isNetworkActionAllowed(subscriptionInfo.liveLocationToken)
) {
closeSubscriptionsOnConnectedRelays(setOf(id))
subscriptions.replaceAll { _, ids -> ids - id }
return
}
val request = NostrRequest.Close(id)
val message = gson.toJson(request, NostrRequest::class.java)
scope.launch {
if (!isNetworkActionAllowed(subscriptionInfo.liveLocationToken)) {
closeSubscriptionsOnConnectedRelays(setOf(id))
subscriptions.replaceAll { _, ids -> ids - id }
return@launch
}
@ -926,7 +964,7 @@ class NostrRelayManager private constructor() {
}
private fun generateSubscriptionId(): String {
return "sub-${System.currentTimeMillis()}-${(Math.random() * 1000).toInt()}"
return "sub-${UUID.randomUUID()}"
}
/**
@ -952,13 +990,13 @@ class NostrRelayManager private constructor() {
var success = false
runNetworkAction(subscriptionInfo.liveLocationToken) {
success = webSocket.send(message)
if (success) {
val currentSubs = subscriptions[relayUrl] ?: emptySet()
subscriptions[relayUrl] =
currentSubs + subscriptionInfo.id
}
}
if (success) {
// Track subscription for this relay
val currentSubs = subscriptions[relayUrl] ?: emptySet()
subscriptions[relayUrl] = currentSubs + subscriptionInfo.id
} else {
if (!success) {
Log.w(TAG, "Failed to restore subscription: WebSocket send failed")
}
} catch (e: Exception) {

View File

@ -79,10 +79,17 @@ class GeohashViewModel(
private var globalPresenceJob: Job? = null
private var locationChannelManager: com.bitchat.android.geohash.LocationChannelManager? = null
private val activeSamplingGeohashes = mutableSetOf<String>()
private val samplingSubscriptionIds = mutableMapOf<String, String>()
private val liveSamplingSubscriptionGeohashes = mutableSetOf<String>()
private var requestedLiveSamplingGeohashes: Set<String> = emptySet()
private var requestedUserSamplingGeohashes: Set<String> = emptySet()
private val liveLocationRevocationListener: () -> Unit = {
activeSamplingGeohashes.removeAll { it !in requestedUserSamplingGeohashes }
val revokedLiveGeohashes = liveSamplingSubscriptionGeohashes.toSet()
revokedLiveGeohashes.forEach { geohash ->
samplingSubscriptionIds.remove(geohash)
activeSamplingGeohashes.remove(geohash)
}
liveSamplingSubscriptionGeohashes.clear()
requestedLiveSamplingGeohashes = emptySet()
}
@ -310,17 +317,27 @@ class GeohashViewModel(
val toRemove = currentSet - newSet
val toAdd = newSet - currentSet
val toPromoteToUserSelection = currentSet
.intersect(requestedUserSamplingGeohashes)
.intersect(liveSamplingSubscriptionGeohashes)
if (toAdd.isEmpty() && toRemove.isEmpty()) return
if (toAdd.isEmpty() && toRemove.isEmpty() && toPromoteToUserSelection.isEmpty()) return
Log.d(TAG, "🌍 Updating sampling: +${toAdd.size} new, -${toRemove.size} removed")
// Remove old subscriptions
toRemove.forEach { geohash ->
subscriptionManager.unsubscribe("sampling-$geohash")
unsubscribeSampling(geohash)
activeSamplingGeohashes.remove(geohash)
}
// A bookmark must remain functional after live access is revoked. Replace a
// live-tagged subscription with an untagged manual subscription immediately.
toPromoteToUserSelection.forEach { geohash ->
unsubscribeSampling(geohash)
if (isAppInForeground()) performSubscribeSampling(geohash)
}
// Add new subscriptions
activeSamplingGeohashes.addAll(toAdd)
if (isAppInForeground()) {
@ -337,7 +354,7 @@ class GeohashViewModel(
Log.d(TAG, "🌍 Ending geohash sampling (cleaning up ${activeSamplingGeohashes.size} subs)")
activeSamplingGeohashes.toList().forEach { geohash ->
subscriptionManager.unsubscribe("sampling-$geohash")
unsubscribeSampling(geohash)
}
activeSamplingGeohashes.clear()
}
@ -496,7 +513,7 @@ class GeohashViewModel(
geohash: String,
liveLocationToken: Long?
) {
val subId = "geohash-$geohash"; currentGeohashMsgSubId = subId
val subId = "geohash-${UUID.randomUUID()}"; currentGeohashMsgSubId = subId
subscriptionManager.subscribeGeohashMessages(
geohash = geohash,
sinceMs = System.currentTimeMillis() - 3600000L,
@ -516,7 +533,7 @@ class GeohashViewModel(
geohash: String,
liveLocationToken: Long?
) {
val subId = "geohash-presence-$geohash"; currentGeohashPresenceSubId = subId
val subId = "geohash-presence-${UUID.randomUUID()}"; currentGeohashPresenceSubId = subId
subscriptionManager.subscribeGeohashPresence(
geohash = geohash,
sinceMs = System.currentTimeMillis() - 3600000L,
@ -565,6 +582,10 @@ class GeohashViewModel(
override fun onStart(owner: LifecycleOwner) {
Log.d(TAG, "🌍 App foregrounded: resuming Nostr streaming")
// Android permission may have changed while backgrounded. Invalidate the
// process-wide token before restoring any subscription or heartbeat.
locationChannelManager?.syncPermissionState()
// Restore the presence heartbeat firehose for the selected geohash channel.
// (The chat message stream is kept alive in the background, so it is not restored here.)
val selected = locationChannelManager?.selectedChannel?.value
@ -597,7 +618,7 @@ class GeohashViewModel(
// The chat message stream (kind 20000) is intentionally left active so messages still arrive.
currentGeohashPresenceSubId?.let { subscriptionManager.unsubscribe(it); currentGeohashPresenceSubId = null }
// Drop geohash sampling subscriptions
activeSamplingGeohashes.forEach { subscriptionManager.unsubscribe("sampling-$it") }
activeSamplingGeohashes.forEach(::unsubscribeSampling)
// Stop broadcasting presence heartbeats
globalPresenceJob?.cancel(); globalPresenceJob = null
// Stop participant-refresh polling
@ -607,14 +628,18 @@ class GeohashViewModel(
}
private fun performSubscribeSampling(geohash: String) {
val subscriptionId = samplingSubscriptionIds.getOrPut(geohash) {
"sampling-${UUID.randomUUID()}"
}
// Sampling only needs participant counts, never message bodies, so it subscribes to
// presence heartbeats only (kind 20001) to keep the payload small.
val subscribe = {
liveSamplingSubscriptionGeohashes.remove(geohash)
subscriptionManager.subscribeGeohashPresence(
geohash = geohash,
sinceMs = System.currentTimeMillis() - 86400000L,
limit = 200,
id = "sampling-$geohash",
id = subscriptionId,
handler = { event -> geohashMessageHandler.onEvent(event, geohash) }
)
}
@ -629,11 +654,12 @@ class GeohashViewModel(
if (!isCurrentLiveTarget) return
val token = LiveLocationPrivacyGate.captureToken() ?: return
LiveLocationPrivacyGate.runIfAllowed(token) {
liveSamplingSubscriptionGeohashes.add(geohash)
subscriptionManager.subscribeGeohashPresence(
geohash = geohash,
sinceMs = System.currentTimeMillis() - 86400000L,
limit = 200,
id = "sampling-$geohash",
id = subscriptionId,
handler = { event -> geohashMessageHandler.onEvent(event, geohash) },
liveLocationToken = token
)
@ -641,6 +667,11 @@ class GeohashViewModel(
}
}
private fun unsubscribeSampling(geohash: String) {
samplingSubscriptionIds.remove(geohash)?.let(subscriptionManager::unsubscribe)
liveSamplingSubscriptionGeohashes.remove(geohash)
}
private fun isAppInForeground(): Boolean {
return ProcessLifecycleOwner.get().lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)
}

View File

@ -0,0 +1,37 @@
package com.bitchat.android.nostr
import org.junit.Assert.assertEquals
import org.junit.Test
class NostrLiveSubscriptionPrivacyTest {
@Test
fun `teardown closes live subscriptions on shared relays`() {
val targets = NostrLiveSubscriptionPrivacy.closeTargets(
liveSubscriptionIds = setOf("live-a", "live-b"),
subscriptionsByRelay = mapOf(
"shared-relay" to setOf("dm", "live-a"),
"live-relay" to setOf("live-a", "live-b"),
"dm-relay" to setOf("dm"),
),
)
assertEquals(
mapOf(
"shared-relay" to setOf("live-a"),
"live-relay" to setOf("live-a", "live-b"),
),
targets,
)
}
@Test
fun `teardown ignores relays without live subscriptions`() {
assertEquals(
emptyMap<String, Set<String>>(),
NostrLiveSubscriptionPrivacy.closeTargets(
liveSubscriptionIds = emptySet(),
subscriptionsByRelay = mapOf("default-relay" to setOf("dm")),
),
)
}
}