|
| 1 | +# TLS trust configuration hook for the TCP transport |
| 2 | + |
| 3 | +Design for [issue #102](https://github.com/meshtastic/MQTTastic-Client-KMP/issues/102). |
| 4 | + |
| 5 | +## Problem |
| 6 | + |
| 7 | +A caller cannot influence TLS trust for the TCP transport. Connecting to a broker whose |
| 8 | +certificate chain is anchored in a private or self-signed CA — the common self-hosted case — |
| 9 | +is therefore impossible without replacing the transport wholesale. |
| 10 | + |
| 11 | +As of 0.5.0 there is no seam: |
| 12 | + |
| 13 | +- `TcpTransportFactory` is `final` with a no-arg constructor; `TcpTransport` is `final` too. |
| 14 | +- `MqttConfig.Builder` exposes no TLS property. |
| 15 | +- `MqttEndpoint.Tcp(host, port, tls: Boolean)` carries only an on/off flag. |
| 16 | +- ktor's `TLSConfigBuilder` is configured in a private lambda inside `TcpTransport.connect` |
| 17 | + (`transport-tcp/src/commonMain/kotlin/org/meshtastic/mqtt/transport/tcp/TcpTransport.kt:102-108`), |
| 18 | + so it never reaches the caller. |
| 19 | + |
| 20 | +The only workaround available to an Android consumer is to opt the *entire app* into trusting |
| 21 | +user-installed CAs via `network_security_config.xml`, which applies to every HTTPS connection |
| 22 | +the app makes rather than just the MQTT socket. The alternative — reimplementing |
| 23 | +`MqttTransport` + `MqttTransportFactory` in the application — duplicates packet framing and |
| 24 | +reconnect handling in every consumer that needs custom trust. |
| 25 | + |
| 26 | +## Approach |
| 27 | + |
| 28 | +Add an optional TLS-customisation lambda to `TcpTransportFactory`, threaded through to |
| 29 | +`TcpTransport`. This is option 1 from the issue. |
| 30 | + |
| 31 | +The issue's option 2 (a unified `MqttConfig.Builder.tlsConfig {}`) was rejected: it would pull |
| 32 | +ktor TLS types into `:core`, breaking the transport-free boundary that ADR-0006 establishes and |
| 33 | +that `core/build.gradle.kts`'s `verifyModuleBoundary` check plus the Konsist suite enforce. |
| 34 | +There is also no shared lambda type to unify on — `:transport-ws` configures a ktor `HttpClient` |
| 35 | +and never touches `TLSConfigBuilder`. Option 3 (making `TcpTransport` `open`) was rejected as a |
| 36 | +strictly worse version of option 1. |
| 37 | + |
| 38 | +## Public API |
| 39 | + |
| 40 | +```kotlin |
| 41 | +public class TcpTransportFactory( |
| 42 | + private val configureTls: (TLSConfigBuilder.() -> Unit)? = null, |
| 43 | +) : MqttTransportFactory { |
| 44 | + override fun supports(endpoint: MqttEndpoint): Boolean = endpoint is MqttEndpoint.Tcp |
| 45 | + |
| 46 | + override fun create(endpoint: MqttEndpoint): MqttTransport = TcpTransport(configureTls) |
| 47 | +} |
| 48 | + |
| 49 | +public class TcpTransport( |
| 50 | + private val configureTls: (TLSConfigBuilder.() -> Unit)? = null, |
| 51 | +) : MqttTransport |
| 52 | +``` |
| 53 | + |
| 54 | +Both parameters are defaulted, so existing call sites are untouched. The hook composes with the |
| 55 | +existing factory `+` operator: |
| 56 | + |
| 57 | +```kotlin |
| 58 | +transportFactory = TcpTransportFactory { trustManager = myTrustManager } + WebSocketTransportFactory() |
| 59 | +``` |
| 60 | + |
| 61 | +`TcpTransport`'s constructor parameter is public for symmetry and for callers who construct the |
| 62 | +transport directly, but the factory is the intended entry point. |
| 63 | + |
| 64 | +## The seam |
| 65 | + |
| 66 | +The TLS setup currently inlined in `TcpTransport.connect` moves into a single internal function: |
| 67 | + |
| 68 | +```kotlin |
| 69 | +internal fun TLSConfigBuilder.applyMqttTls( |
| 70 | + host: String, |
| 71 | + configureTls: (TLSConfigBuilder.() -> Unit)?, |
| 72 | +) { |
| 73 | + serverName = sniServerName(host) |
| 74 | + configureTls?.invoke(this) // caller first… |
| 75 | + configurePlatformTrust(host) // …so Android wraps their trustManager, not the reverse |
| 76 | +} |
| 77 | +``` |
| 78 | + |
| 79 | +`connect` then calls `applyMqttTls(endpoint.host, configureTls)` inside `rawSocket.tls(tlsContext) { … }`. |
| 80 | + |
| 81 | +### Ordering: caller lambda runs *before* `configurePlatformTrust` |
| 82 | + |
| 83 | +This corrects a contradiction in the issue text, which asks for the hook to run *after* the |
| 84 | +platform defaults while also stating that a caller-supplied trust manager would "compose with |
| 85 | +Android's hostname-aware checking rather than bypass it." Only one of those is achievable. |
| 86 | + |
| 87 | +`configurePlatformTrust` on Android |
| 88 | +(`transport-tcp/src/androidMain/kotlin/org/meshtastic/mqtt/transport/tcp/PlatformTls.android.kt:41-52`) |
| 89 | +reads `trustManager` off the builder, falling back to the platform default, and wraps whatever it |
| 90 | +finds in `HostnameAwareTrustManager`. Composition therefore requires the caller's assignment to |
| 91 | +already be present — i.e. the caller's lambda must run first. Running it last would replace |
| 92 | +`HostnameAwareTrustManager` outright and silently drop Android's 3-arg hostname verification. |
| 93 | + |
| 94 | +Running the caller first delivers the intended behaviour: a private-CA trust manager is still |
| 95 | +subject to the platform's hostname-aware check. |
| 96 | + |
| 97 | +Ordering is enforced structurally rather than by convention — `applyMqttTls` is the only place the |
| 98 | +order can be expressed, so there is no second call site to drift. |
| 99 | + |
| 100 | +`serverName` remains first and is not exposed for override beyond what the lambda can already do; |
| 101 | +a caller may reassign it inside the lambda if they need to. |
| 102 | + |
| 103 | +## Build and compatibility |
| 104 | + |
| 105 | +- `transport-tcp/build.gradle.kts`: `libs.ktor.network.tls` moves from `implementation` to `api`. |
| 106 | + A public signature now names `TLSConfigBuilder`, so consumers need it on their compile classpath. |
| 107 | + `libs.ktor.network` stays `implementation`. |
| 108 | +- `./gradlew apiDump` regenerates `transport-tcp/api/transport-tcp.klib.api` and the JVM dump. |
| 109 | + Both are committed. |
| 110 | +- Binary compatibility holds. Kotlin emits a zero-arg constructor for an all-defaults constructor, |
| 111 | + so already-compiled callers of `TcpTransportFactory()` keep linking. |
| 112 | +- No change to `:core`, `:transport-ws`, or the BOM. |
| 113 | + |
| 114 | +## Testing |
| 115 | + |
| 116 | +Extends the pure-function style already in |
| 117 | +`transport-tcp/src/commonTest/kotlin/org/meshtastic/mqtt/transport/tcp/TcpTransportTlsTest.kt`. |
| 118 | +`TLSConfigBuilder` is directly instantiable in common code, so no broker or socket is needed. |
| 119 | + |
| 120 | +`commonTest`: |
| 121 | + |
| 122 | +- `applyMqttTls` sets `serverName` to the host for a DNS name and to `null` for IPv4 and IPv6 |
| 123 | + literals — the existing SNI-suppression guarantee, now asserted through the new entry point. |
| 124 | +- The lambda is invoked exactly once (recorder counter). |
| 125 | +- A `null` lambda is a no-op and leaves `serverName` behaviour unchanged. |
| 126 | +- `TcpTransportFactory { }` still reports `supports()` correctly, `create()` returns a |
| 127 | + `TcpTransport`, and the instance composes with `WebSocketTransportFactory` via `+`. |
| 128 | + |
| 129 | +`jvmTest`: |
| 130 | + |
| 131 | +- A `trustManager` assigned inside the lambda is still the builder's `trustManager` afterwards. |
| 132 | + JVM's `configurePlatformTrust` is a no-op, so this proves the hook reaches ktor's real builder |
| 133 | + state rather than a discarded copy. |
| 134 | + |
| 135 | +Not covered: the Android wrapping order. `X509TrustManagerExtensions` is an Android framework |
| 136 | +class, so asserting that a caller's manager ends up inside `HostnameAwareTrustManager` requires an |
| 137 | +instrumentation test. That is out of scope; the single-call-site structure of `applyMqttTls` is the |
| 138 | +mitigation. |
| 139 | + |
| 140 | +Existing `koverVerify` (≥80%), `detekt`, `spotlessCheck`, and `apiCheck` gates all apply. |
| 141 | + |
| 142 | +## Documentation |
| 143 | + |
| 144 | +- `transport-tcp/Module.md` — a private-CA usage snippet, and a note that the caller's |
| 145 | + configuration is applied before platform trust so Android hostname verification is preserved. |
| 146 | +- `README.md` TLS section — a pointer to the hook, framed as the replacement for the app-wide |
| 147 | + `<certificates src="user"/>` workaround. |
| 148 | +- `AGENTS.md` — no change needed; the public-surface list already covers transport modules |
| 149 | + generically via "Transport modules add `TcpTransport`/`TcpTransportFactory`…". |
| 150 | + |
| 151 | +## Out of scope (YAGNI) |
| 152 | + |
| 153 | +- No `host` parameter on the lambda — a factory can close over whatever it needs. |
| 154 | +- No second, post-platform-trust seam. |
| 155 | +- No WebSocket equivalent. `:transport-ws` would need a differently-typed `HttpClient` hook; it can |
| 156 | + be added later without disturbing this API. |
| 157 | +- No `MqttConfig`-level plumbing or transport-neutral trust abstraction in `:core`. |
0 commit comments