`Transport::Wss` with a `ws://` broker address connects in cleartext, silently

[holovskyi]

With Transport::Wss(..) set, a broker_addr that uses the ws:// scheme
produces an unencrypted connection. The TLS connector is built and then
discarded. No error is returned, and nothing on that path emits a warning; the
client completes the handshake and delivers a ConnAck, so the application
believes it has TLS.

Affects rumqttc 0.25.1 and current main (e886a78), on both the v4 and v5
stacks, with the websocket feature plus use-rustls-no-provider (the latter
implied by the default use-rustls; websocket is opt-in). Under
use-native-tls alone the Wss variant does not exist, so this does not apply
there.

Reproduction

Against a broker with the usual two WebSocket listeners, plaintext on 8083 and
TLS on 8084 (stock EMQX here, but Mosquitto's layout is the same):

let tls = TlsConfiguration::Simple { ca, alpn: None, client_auth: None };
let mut opts = MqttOptions::new("repro", "ws://localhost:8083/mqtt", 1883);
opts.set_transport(Transport::Wss(tls));   // TLS requested
// ... poll until ConnAck

Observed:

#	transport	broker address	listener	result
A	Wss(tls)	ws://localhost:8083/mqtt	plaintext	CONNECTED
B	Ws	ws://localhost:8083/mqtt	plaintext	CONNECTED (control)
C	Wss(tls)	wss://localhost:8084/mqtt	TLS	CONNECTED (control)
D	Wss(tls)	ws://localhost:8084/mqtt	TLS	httparse error: invalid HTTP version
E	Wss(tls), v5	ws://localhost:8083/mqtt	plaintext	CONNECTED

A is the defect: TLS was requested, and the client is talking to the plaintext
listener happily.

C and D are what rule out the alternative readings. C uses the same
TlsConfiguration against the TLS listener and works, so the config is not the
problem. D is the identical configuration to A pointed at the TLS listener, and
it fails because the client sent a plaintext HTTP GET into the TLS port and then
tried to parse a TLS record as an HTTP response. Taken together, no TLS is being
applied in either case. E shows v5 behaving identically.

Where it comes from

The mode is decided from the URL scheme rather than from the transport variant,
and the connector is dropped on the plaintext path:

• src/eventloop.rs:461-478
  (v4) and
  src/v5/eventloop.rs:370-387
  (v5): the Wss arm builds the connector and calls
  client_async_tls_with_connector(request, tcp_stream, Some(connector)).
• async-tungstenite 0.29.1, src/tokio.rs:257-268 then
  src/tokio/rustls.rs:87-105: the connector is forwarded, but :103 derives
  the mode with uri_mode(request.uri()).
• tungstenite 0.26.2, src/client.rs:144-150: uri_mode maps ws to
  Mode::Plain and wss to Mode::Tls.
• async-tungstenite src/tokio/rustls.rs:25-36: wrap_stream on
  Mode::Plain returns StreamSwitcher::Plain(..), and the connector goes
  unused.

The same scheme-over-variant assumption appears independently in rumqttc's own
split_url: with no explicit port in the address, src/websockets.rs:67-71
defaults ws to 80 rather than 443.

Why it is reachable in practice

parse_url cannot reach this state, but not because it is guarded: for
ws/wss it stores only url.host_str() as broker_addr
(src/lib.rs:797-811), which split_url then rejects at connect time. That is
#808.

So the way to a WebSocket connection today is MqttOptions::new(..) with the
full URL in the host position plus an explicit set_transport(..), which is
what examples/websocket.rs demonstrates for Ws and the natural way to reach
Wss. Nothing in the type or the docs indicates that the scheme, and not the
variant, is what selects TLS.

For what it is worth, we had this wrong in our own documentation for a release:
we told users to write ws:// for Wss on the grounds that the variant states
the TLS. That is how we found this.

Possible fixes

In preference order:

1. Derive the mode from the transport variant: pass Mode::Tls explicitly when
   a connector is present, instead of re-deriving it from the URI.
2. Return an error when Wss is paired with a non-wss scheme, rather than
   downgrading.
3. At minimum, document that the scheme is load-bearing for Wss.

This overlaps with #808. The fix sketched there rebuilds the request from the
parsed components (format!("ws://{domain}:{port}/{path}")) so the port stops
being dropped; if that lands, the scheme in the rebuilt request would need to
come from the transport variant rather than being fixed to ws, which is the
same decision as option 1 above. Probably worth doing in one go.

Possibly related: #982 (websocket feature and TLS plumbing), #881 (native-tls
WSS, which routes through async-tungstenite's tokio/native_tls.rs, where
wrap_stream and uri_mode have the same shape, so a fix likely wants to cover
both connectors), #789 (the MqttOptions refactor, where option 2 would
naturally live).

---

Reaching this takes a specific address and transport pairing on the
application's side rather than being remotely triggerable, so a public issue
seemed right; say the word if you would rather have had it privately. Happy to
attach a packet capture, or to open a PR for whichever of the three options you
prefer.

[thehouseisonfire]

This is a real issue, one of the first I fixed back in March on my end. The way I did it was to make so when Transport::Wss is selected, the TCP stream is wrapped in TLS before starting the WebSocket handshake.

[holovskyi]

Thanks for confirming, that is useful to know, and wrapping the TCP stream before the handshake is option 1 above.

Since a working implementation already exists on your side rather than only in the abstract: would you want to upstream it? That seems better than a second, independently written version of the same fix. If you would rather not, we are still happy to open the PR here, and we would take the approach from your fix with attribution.

Either way it probably wants a maintainer to say which of the three options they prefer first, so this is not urgent.