fix(mqtt): harden TLS enforcement, add user CA trust, and improve error diagnostics (#5365)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
James Rich
2026-05-06 12:09:49 -05:00
committed by GitHub
parent f97900b558
commit 3c75510f01
7 changed files with 76 additions and 13 deletions

View File

@@ -3,6 +3,7 @@
<base-config cleartextTrafficPermitted="false">
<trust-anchors>
<certificates src="system"/>
<certificates src="user"/>
</trust-anchors>
</base-config>

View File

@@ -35,6 +35,7 @@ import org.meshtastic.core.model.MqttProbeStatus
import org.meshtastic.core.network.repository.MQTTRepository
import org.meshtastic.core.network.repository.resolveEndpoint
import org.meshtastic.core.repository.MqttManager
import org.meshtastic.core.repository.NodeRepository
import org.meshtastic.core.repository.PacketHandler
import org.meshtastic.core.repository.ServiceRepository
import org.meshtastic.mqtt.ConnectionState
@@ -44,13 +45,13 @@ import org.meshtastic.mqtt.ProbeResult
import org.meshtastic.mqtt.probe
import org.meshtastic.proto.MqttClientProxyMessage
import org.meshtastic.proto.ToRadio
import kotlin.time.Clock
@Single
class MqttManagerImpl(
private val mqttRepository: MQTTRepository,
private val packetHandler: PacketHandler,
private val serviceRepository: ServiceRepository,
private val nodeRepository: NodeRepository,
@Named("ServiceScope") private val scope: CoroutineScope,
) : MqttManager {
private var mqttMessageFlow: Job? = null
@@ -131,8 +132,8 @@ class MqttManagerImpl(
val endpoint = resolveEndpoint(address, tlsEnabled)
val result =
MqttClient.probe(endpoint = endpoint) {
// Provide a valid client ID for the probe; brokers reject empty identifiers
clientId = "MeshtasticProbe-${Clock.System.now().toEpochMilliseconds()}"
// Use node ID for consistent client identification across platforms
clientId = "MeshtasticAndroidMqttProbe-${nodeRepository.myId.value ?: "unknown"}"
val user = username?.takeUnless { it.isEmpty() }
val pass = password?.takeUnless { it.isEmpty() }
if (user != null) this.username = user

View File

@@ -290,4 +290,22 @@ fun resolveEndpoint(rawAddress: String, tlsEnabled: Boolean): MqttEndpoint = if
private const val DEFAULT_PUBLIC_SERVER = "mqtt.meshtastic.org"
fun effectiveTlsEnabled(address: String, tlsEnabled: Boolean): Boolean =
tlsEnabled || address.equals(DEFAULT_PUBLIC_SERVER, ignoreCase = true)
tlsEnabled || extractHost(address).equals(DEFAULT_PUBLIC_SERVER, ignoreCase = true)
/**
* Extracts the bare hostname from an address that may include a scheme, port, or path. Examples:
* - `mqtt.meshtastic.org` → `mqtt.meshtastic.org`
* - `mqtt.meshtastic.org:1883` → `mqtt.meshtastic.org`
* - `tcp://mqtt.meshtastic.org:1883` → `mqtt.meshtastic.org`
* - `ssl://mqtt.meshtastic.org` → `mqtt.meshtastic.org`
*/
internal fun extractHost(address: String): String {
val afterScheme =
if (address.contains("://")) {
address.substringAfter("://")
} else {
address
}
// Remove path (if any), then remove port
return afterScheme.substringBefore("/").substringBefore(":")
}

View File

@@ -110,6 +110,21 @@ class MQTTRepositoryImplTest {
assertEquals(true, effectiveTlsEnabled("MQTT.MESHTASTIC.ORG", tlsEnabled = false))
}
@Test
fun `default server with explicit port still forces TLS`() {
assertEquals(true, effectiveTlsEnabled("mqtt.meshtastic.org:1883", tlsEnabled = false))
}
@Test
fun `default server with tcp scheme still forces TLS`() {
assertEquals(true, effectiveTlsEnabled("tcp://mqtt.meshtastic.org:1883", tlsEnabled = false))
}
@Test
fun `default server with ssl scheme still forces TLS`() {
assertEquals(true, effectiveTlsEnabled("ssl://mqtt.meshtastic.org", tlsEnabled = false))
}
@Test
fun `custom server respects tlsEnabled false`() {
assertEquals(false, effectiveTlsEnabled("mqtt.myserver.pt", tlsEnabled = false))
@@ -122,6 +137,35 @@ class MQTTRepositoryImplTest {
// endregion
// region extractHost — address canonicalization tests.
@Test
fun `extractHost bare hostname`() {
assertEquals("mqtt.meshtastic.org", extractHost("mqtt.meshtastic.org"))
}
@Test
fun `extractHost with port`() {
assertEquals("mqtt.meshtastic.org", extractHost("mqtt.meshtastic.org:8883"))
}
@Test
fun `extractHost with tcp scheme and port`() {
assertEquals("mqtt.meshtastic.org", extractHost("tcp://mqtt.meshtastic.org:1883"))
}
@Test
fun `extractHost with ssl scheme no port`() {
assertEquals("mqtt.meshtastic.org", extractHost("ssl://mqtt.meshtastic.org"))
}
@Test
fun `extractHost with path`() {
assertEquals("broker.example.com", extractHost("ws://broker.example.com:8080/mqtt"))
}
// endregion
// region MqttJsonPayload — keep the existing JSON contract tests.
@Test

View File

@@ -719,7 +719,7 @@
<string name="mqtt_probe_success_with_info">Reachable (%1$s)</string>
<string name="mqtt_probe_tcp_failure">Cannot reach broker (TCP)</string>
<string name="mqtt_probe_timeout">Timed out after %1$d ms</string>
<string name="mqtt_probe_tls_failure">TLS handshake failed</string>
<string name="mqtt_probe_tls_failure">TLS handshake failed: %1$s</string>
<string name="mqtt_status_connected">Connected</string>
<string name="mqtt_status_connecting">Connecting…</string>
<string name="mqtt_status_disconnected">Disconnected</string>

View File

@@ -36,6 +36,7 @@ kotlin {
implementation(projects.core.domain)
implementation(projects.core.model)
implementation(projects.core.navigation)
implementation(projects.core.network)
implementation(projects.core.proto)
implementation(projects.core.repository)
implementation(projects.core.service)

View File

@@ -49,9 +49,9 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle
import org.jetbrains.compose.resources.stringResource
import org.meshtastic.core.model.MqttConnectionState
import org.meshtastic.core.model.MqttProbeStatus
import org.meshtastic.core.network.repository.effectiveTlsEnabled
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.address
import org.meshtastic.core.resources.default_mqtt_address
import org.meshtastic.core.resources.encryption_enabled
import org.meshtastic.core.resources.json_output_enabled
import org.meshtastic.core.resources.map_reporting
@@ -183,9 +183,8 @@ fun MQTTConfigScreen(viewModel: RadioConfigViewModel, onBack: () -> Unit) {
containerColor = CardDefaults.cardColors().containerColor,
)
HorizontalDivider()
val defaultAddress = stringResource(Res.string.default_mqtt_address)
val isDefault = formState.value.address.isEmpty() || formState.value.address.contains(defaultAddress)
val enforceTls = isDefault && formState.value.proxy_to_client_enabled
val resolvedAddress = formState.value.address.ifEmpty { "mqtt.meshtastic.org" }
val enforceTls = effectiveTlsEnabled(resolvedAddress, tlsEnabled = false)
SwitchPreference(
title = stringResource(Res.string.tls_enabled),
checked = formState.value.tls_enabled || enforceTls,
@@ -318,14 +317,13 @@ private fun MqttAddressAndProbe(
},
)
HorizontalDivider()
val defaultAddress = stringResource(Res.string.default_mqtt_address)
MqttProbeRow(
enabled = enabled && formState.value.address.isNotBlank(),
status = probeStatus,
onTestClick = {
focusManager.clearFocus()
val isDefault = formState.value.address.isEmpty() || formState.value.address.contains(defaultAddress)
val effectiveTls = formState.value.tls_enabled || isDefault
val resolvedAddress = formState.value.address.ifEmpty { "mqtt.meshtastic.org" }
val effectiveTls = effectiveTlsEnabled(resolvedAddress, formState.value.tls_enabled)
onProbe(formState.value.address, effectiveTls, formState.value.username, formState.value.password)
},
)
@@ -376,7 +374,7 @@ private fun MqttProbeStatus?.toLabel(): Pair<String, Color>? = when (this) {
stringResource(Res.string.mqtt_probe_tcp_failure) to MaterialTheme.colorScheme.error
is MqttProbeStatus.TlsFailure ->
stringResource(Res.string.mqtt_probe_tls_failure) to MaterialTheme.colorScheme.error
stringResource(Res.string.mqtt_probe_tls_failure, message ?: "unknown") to MaterialTheme.colorScheme.error
is MqttProbeStatus.Timeout ->
stringResource(Res.string.mqtt_probe_timeout, timeoutMs.toInt()) to MaterialTheme.colorScheme.error