Skip to content

Commit af222b8

Browse files
jamesarichclaude
andcommitted
fix(transport): restore no-arg klib ctors, narrow trust docs
Addresses PR #103 review feedback. - Add explicit `constructor() : this(null)` to TcpTransport and TcpTransportFactory. The klib dump now carries `constructor <init>()` alongside the Function1 overload, so the native ABI change is additive rather than a replacement. The JVM dump is unchanged — no signature clash with the constructor Kotlin already synthesises for an all-defaults primary constructor. This contradicts the PR body's claim that the klib replacement was inherent and unfixable; it was not. - Narrow the platform-trust claim in PlatformTls.kt, TcpTransport.kt, README.md, and Module.md. Installing a trust manager REPLACES the platform's trust decision; network_security_config anchors, pinning, and CT policy then hold only insofar as that manager enforces them, and apply as before only when trustManager is left unset. The wrapping preserves the hostname-aware call path, not the platform's policy. X509TrustManagerExtensions also accepts any manager declaring the three-arg checkServerTrusted reflectively, so "must come from TrustManagerFactory" was too strong. - Module.md heading h3 -> h2; four-backtick outer fences in the plan doc so nested Kotlin fences stop terminating the block early. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent f0438b2 commit af222b8

6 files changed

Lines changed: 86 additions & 50 deletions

File tree

README.md

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -348,23 +348,30 @@ client.connect(MqttEndpoint.parse("mqtts://broker.internal:8883"))
348348
```
349349

350350
The hook is applied after the SNI server name is resolved and before platform trust is configured.
351-
On Android that ordering matters: your trust manager is *wrapped* by the hostname-aware trust
352-
manager rather than replacing it, so your trust anchors remain subject to the platform's
353-
domain-specific `network_security_config.xml` rules, certificate pinning, and Certificate
354-
Transparency policy instead of replacing that policy wholesale.
355-
356-
Be clear about what the wrapping does *not* buy you. Android's hostname-aware
357-
`checkServerTrusted(chain, authType, hostname)` uses the hostname to look up that policy — it does
358-
not perform RFC 6125 subject-name matching. That check comes from ktor and only runs when the SNI
351+
On Android that ordering means your trust manager is reached through the hostname-aware
352+
`checkServerTrusted(chain, authType, hostname)` overload, which the platform requires whenever
353+
`network_security_config.xml` holds any domain-specific configuration — rather than being discarded
354+
in favour of the platform wrapper.
355+
356+
Be clear about what that does *not* buy you. Installing your own trust manager **replaces the
357+
platform's trust decision**: your anchors are used instead of the platform's, and
358+
`network_security_config.xml` anchors, certificate pinning, and Certificate Transparency policy are
359+
then enforced only insofar as your manager enforces them itself. Those platform policies apply as
360+
before only if you leave `trustManager` unset. The wrapping preserves the hostname-aware *call
361+
path*, not the platform's *policy*.
362+
363+
RFC 6125 subject-name matching is separate again. Android's 3-arg overload uses the hostname for
364+
policy lookup, not for subject-name matching; that check comes from ktor and only runs when the SNI
359365
server name is set, so it is absent for IP-literal brokers such as `mqtts://192.168.1.50:8883`. On
360-
JVM and native targets there is no platform trust wrapping at all, so ktor's SNI-gated
361-
subject-name check is the only peer-identity verification beyond chain validation. If your trust
362-
manager accepts any chain, nothing else will stop a mismatched certificate.
363-
364-
On Android the trust manager you install must be one the platform can wrap: obtain it from a
365-
`TrustManagerFactory` initialised with a `KeyStore` containing your CA, rather than hand-writing an
366-
`X509TrustManager`. A hand-written one that implements only the two-arg `checkServerTrusted` cannot
367-
be wrapped and the handshake fails with an `IllegalArgumentException` explaining this.
366+
JVM and native targets there is no platform trust wrapping at all, so ktor's SNI-gated subject-name
367+
check is the only peer-identity verification beyond chain validation. If your trust manager accepts
368+
any chain, nothing else will stop a mismatched certificate.
369+
370+
On Android the manager must be one `X509TrustManagerExtensions` can wrap: either obtained from a
371+
`TrustManagerFactory` initialised with a `KeyStore` containing your CA, or declaring the three-arg
372+
`checkServerTrusted(chain, authType, host)` that the platform looks up reflectively. A hand-written
373+
`X509TrustManager` implementing only the two-arg overload cannot be wrapped, and the handshake fails
374+
with an `IllegalArgumentException` explaining this.
368375

369376
This scopes the extra trust to the MQTT connection alone. It replaces the app-wide workaround of
370377
adding `<certificates src="user"/>` to `network_security_config.xml`, which would affect every

docs/superpowers/plans/2026-07-25-tls-trust-hook.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ Document the hook where a consumer hitting the issue #102 problem will actually
571571
In `transport-tcp/Module.md`, insert between the existing code fence (ends line 13) and the
572572
"Available on JVM, Android, …" paragraph (line 15):
573573

574-
```markdown
574+
````markdown
575575
### Trusting a private CA
576576

577577
If the broker's certificate is issued by a private or self-signed CA that is not in the platform
@@ -587,14 +587,14 @@ The lambda is applied after the SNI server name is set and before platform trust
587587
on Android a trust manager installed here is still wrapped in the hostname-aware delegate rather
588588
than replacing it. The added trust applies only to this MQTT connection — unlike Android's
589589
app-wide `network_security_config.xml` trust anchors.
590-
```
590+
````
591591

592592
- [ ] **Step 2: Add the README section**
593593

594594
In `README.md`, insert immediately before line 335 (`## Android / KMP Integration`), after the
595595
"Log levels from most to least verbose…" line that closes the Logging section:
596596

597-
```markdown
597+
````markdown
598598
### Custom TLS trust
599599

600600
By default the TCP transport validates the broker certificate against the platform CA store. To
@@ -611,8 +611,10 @@ client.connect(MqttEndpoint.parse("mqtts://broker.internal:8883"))
611611
```
612612

613613
The hook is applied after the SNI server name is resolved and before platform trust is configured.
614-
On Android that ordering matters: your trust manager is *wrapped* by the hostname-aware trust
615-
manager rather than replacing it, so certificate hostname verification still happens.
614+
On Android that ordering means your trust manager is reached through the hostname-aware
615+
`checkServerTrusted(chain, authType, hostname)` overload rather than being discarded. Note that
616+
installing your own trust manager replaces the platform's trust decision — see the shipped
617+
`README.md` section for the full statement of what that does and does not preserve.
616618

617619
This scopes the extra trust to the MQTT connection alone. It replaces the app-wide workaround of
618620
adding `<certificates src="user"/>` to `network_security_config.xml`, which would affect every
@@ -628,7 +630,7 @@ transportFactory = TcpTransportFactory { trustManager = myPrivateCaTrustManager
628630
`TLSConfigBuilder` comes from `io.ktor:ktor-network-tls`, exposed transitively by
629631
`mqtt-client-transport-tcp` — no extra dependency needed. The WebSocket transport has no equivalent
630632
hook yet.
631-
```
633+
````
632634

633635
- [ ] **Step 3: Verify the docs build**
634636

transport-tcp/Module.md

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ val client = MqttClient("sensor") {
1212
client.connect(MqttEndpoint.Tcp("broker.example.com", port = 8883, tls = true))
1313
```
1414

15-
### Trusting a private CA
15+
## Trusting a private CA
1616

1717
If the broker's certificate is issued by a private or self-signed CA that is not in the platform
1818
trust store, pass a TLS customisation lambda. It receives ktor's `TLSConfigBuilder`:
@@ -23,18 +23,29 @@ val client = MqttClient("sensor") {
2323
}
2424
```
2525

26-
The lambda is applied after the SNI server name is set and before platform trust configuration, so
27-
on Android a trust manager installed here is wrapped in the hostname-aware delegate rather than
28-
replacing it: the trust anchors you add stay subject to the platform's domain-specific
29-
`network_security_config.xml` rules, certificate pinning, and Certificate Transparency policy. That
30-
is all the wrapping buys — RFC 6125 subject-name matching comes from ktor and only when the SNI
31-
server name is set, so it does not happen for IP-literal brokers, and the platform policy checks do
32-
not happen at all on JVM or native targets. On Android the trust manager must be one the platform
33-
can wrap (obtained from a `TrustManagerFactory`, not hand-implemented) or the handshake fails.
34-
The added trust applies only to this MQTT connection — unlike Android's
35-
app-wide `network_security_config.xml` trust anchors. `trustManager` itself is available on the
36-
JVM and Android actuals of `TLSConfigBuilder`; on Apple, Linux, and Windows the lambda still runs,
37-
but `TLSConfigBuilder` exposes a different set of properties there.
26+
The lambda is applied after the SNI server name is set and before platform trust configuration. On
27+
Android that ordering means the trust manager on the builder is reached through the hostname-aware
28+
`checkServerTrusted(chain, authType, hostname)` overload, which the platform requires whenever
29+
`network_security_config.xml` holds any domain-specific configuration.
30+
31+
Be precise about what that does *not* buy you. Installing your own trust manager **replaces the
32+
platform's trust decision**: your anchors are used instead of the platform's, and
33+
`network_security_config.xml` anchors, certificate pinning, and Certificate Transparency policy are
34+
then enforced only insofar as your manager enforces them itself. Those platform policies apply as
35+
before only if you leave `trustManager` unset. Wrapping preserves the hostname-aware *call path*,
36+
not the platform's *policy*. RFC 6125 subject-name matching is separate again — it comes from ktor
37+
and only when the SNI server name is set, so it does not happen for IP-literal brokers, and no
38+
platform wrapping happens at all on JVM or native targets.
39+
40+
On Android the manager must be one `X509TrustManagerExtensions` can wrap: either obtained from a
41+
`TrustManagerFactory`, or declaring the three-arg `checkServerTrusted(chain, authType, host)` that
42+
the platform looks up reflectively. Otherwise the handshake fails with an `IllegalArgumentException`
43+
explaining this.
44+
45+
The added trust applies only to this MQTT connection — unlike Android's app-wide
46+
`network_security_config.xml` trust anchors. `trustManager` itself is available on the JVM and
47+
Android actuals of `TLSConfigBuilder`; on Apple, Linux, and Windows the lambda still runs, but
48+
`TLSConfigBuilder` exposes a different set of properties there.
3849

3950
Available on JVM, Android, iOS, macOS, Linux, and Windows. Not available on the browser (wasmJs) —
4051
use `mqtt-client-transport-ws` there.

transport-tcp/api/transport-tcp.klib.api

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
// Library unique name: <MQTTastic-Client-KMP:transport-tcp>
99
final class org.meshtastic.mqtt.transport.tcp/TcpTransport : org.meshtastic.mqtt/MqttTransport { // org.meshtastic.mqtt.transport.tcp/TcpTransport|null[0]
10+
constructor <init>() // org.meshtastic.mqtt.transport.tcp/TcpTransport.<init>|<init>(){}[0]
1011
constructor <init>(kotlin/Function1<io.ktor.network.tls/TLSConfigBuilder, kotlin/Unit>? = ...) // org.meshtastic.mqtt.transport.tcp/TcpTransport.<init>|<init>(kotlin.Function1<io.ktor.network.tls.TLSConfigBuilder,kotlin.Unit>?){}[0]
1112

1213
final val isConnected // org.meshtastic.mqtt.transport.tcp/TcpTransport.isConnected|{}isConnected[0]
@@ -19,6 +20,7 @@ final class org.meshtastic.mqtt.transport.tcp/TcpTransport : org.meshtastic.mqtt
1920
}
2021

2122
final class org.meshtastic.mqtt.transport.tcp/TcpTransportFactory : org.meshtastic.mqtt/MqttTransportFactory { // org.meshtastic.mqtt.transport.tcp/TcpTransportFactory|null[0]
23+
constructor <init>() // org.meshtastic.mqtt.transport.tcp/TcpTransportFactory.<init>|<init>(){}[0]
2224
constructor <init>(kotlin/Function1<io.ktor.network.tls/TLSConfigBuilder, kotlin/Unit>? = ...) // org.meshtastic.mqtt.transport.tcp/TcpTransportFactory.<init>|<init>(kotlin.Function1<io.ktor.network.tls.TLSConfigBuilder,kotlin.Unit>?){}[0]
2325

2426
final fun create(org.meshtastic.mqtt/MqttEndpoint): org.meshtastic.mqtt/MqttTransport // org.meshtastic.mqtt.transport.tcp/TcpTransportFactory.create|create(org.meshtastic.mqtt.MqttEndpoint){}[0]

transport-tcp/src/commonMain/kotlin/org/meshtastic/mqtt/transport/tcp/PlatformTls.kt

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,26 @@ internal expect fun TLSConfigBuilder.configurePlatformTrust(host: String)
4848
* 3. [configurePlatformTrust] — reads whatever trust manager is now on the builder and,
4949
* on Android, wraps it in a hostname-aware delegate.
5050
*
51-
* Step 2 must precede step 3. Reversing them would let a caller-supplied trust manager
52-
* *replace* Android's `HostnameAwareTrustManager`, so the caller's trust anchors would no
53-
* longer be evaluated against the platform's domain-specific `network_security_config.xml`
54-
* rules, certificate pinning, or Certificate Transparency policy. Running the caller first
55-
* composes instead: a private-CA trust manager stays subject to those platform checks.
51+
* Step 2 must precede step 3. What step 3 provides is the call path: whatever trust manager is
52+
* on the builder is reached through Android's hostname-aware
53+
* `checkServerTrusted(chain, authType, hostname)` overload, which `NetworkSecurityTrustManager`
54+
* requires whenever `network_security_config.xml` holds any domain-specific configuration.
55+
* Reversing the two steps would discard that wrapper, leaving ktor's 2-arg call to hit the bare
56+
* manager.
5657
*
57-
* Note what this ordering does *not* provide. RFC 6125 subject-name matching (verifying the
58-
* certificate actually names the host being contacted) comes from ktor, which performs it only
59-
* when `serverName` is non-`null` — so it is absent for IP-literal brokers. Android's 3-arg
60-
* `checkServerTrusted(chain, authType, hostname)` uses the hostname for config lookup, pinning,
61-
* and CT policy, not for subject-name matching. On JVM and native targets
62-
* [configurePlatformTrust] is a no-op, so no platform policy check happens there at all.
58+
* Be precise about what this ordering does *not* provide. Installing a trust manager via
59+
* [configureTls] *replaces the platform's trust decision*: that manager's anchors are used
60+
* instead of the platform's, and `network_security_config.xml` anchors, certificate pinning,
61+
* and Certificate Transparency policy are then enforced only insofar as that manager enforces
62+
* them itself. Those platform policies apply as they did before only when the caller leaves
63+
* `trustManager` unset. Wrapping preserves the hostname-aware *call path*, not the platform's
64+
* *policy*.
65+
*
66+
* RFC 6125 subject-name matching (verifying the certificate actually names the host being
67+
* contacted) is separate again: it comes from ktor, which performs it only when `serverName` is
68+
* non-`null` — so it is absent for IP-literal brokers. Android's 3-arg overload uses the
69+
* hostname for config lookup, pinning, and CT policy, not for subject-name matching. On JVM and
70+
* native targets [configurePlatformTrust] is a no-op, so no platform wrapping happens at all.
6371
*
6472
* The hook may read or replace `serverName`, but setting it to `null` disables ktor's
6573
* subject-name verification entirely — the only name matching this transport has on JVM and

transport-tcp/src/commonMain/kotlin/org/meshtastic/mqtt/transport/tcp/TcpTransport.kt

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ import org.meshtastic.mqtt.packet.VariableByteInt
5858
public class TcpTransport(
5959
internal val configureTls: (TLSConfigBuilder.() -> Unit)? = null,
6060
) : MqttTransport {
61+
public constructor() : this(null)
62+
6163
private var socket: Socket? = null
6264
private var selectorManager: SelectorManager? = null
6365
private var readChannel: ByteReadChannel? = null
@@ -279,14 +281,18 @@ internal fun isIpLiteral(host: String): Boolean {
279281
*
280282
* @param configureTls optional hook applied to ktor's [TLSConfigBuilder] for every transport
281283
* this factory creates. It runs after the SNI server name is set and before platform trust
282-
* is configured, so on Android a trust manager installed here is wrapped in the
283-
* hostname-aware delegate rather than replacing it — the caller's trust anchors stay subject
284-
* to the platform's network-security-config, pinning, and Certificate Transparency policy.
285-
* That wrapping is Android-only and is not subject-name matching; see [applyMqttTls].
284+
* is configured, so on Android a trust manager installed here is reached through the
285+
* hostname-aware `checkServerTrusted` overload rather than bypassing it. Note that installing
286+
* a trust manager replaces the platform's trust decision — network-security-config anchors,
287+
* pinning, and Certificate Transparency policy then hold only insofar as that manager enforces
288+
* them. The wrapping is Android-only and is not subject-name matching; see [applyMqttTls] for
289+
* the full picture before relying on it.
286290
*/
287291
public class TcpTransportFactory(
288292
private val configureTls: (TLSConfigBuilder.() -> Unit)? = null,
289293
) : MqttTransportFactory {
294+
public constructor() : this(null)
295+
290296
override fun supports(endpoint: MqttEndpoint): Boolean = endpoint is MqttEndpoint.Tcp
291297

292298
override fun create(endpoint: MqttEndpoint): MqttTransport = TcpTransport(configureTls)

0 commit comments

Comments
 (0)