`rustls-tls-native-roots` does not imply `rustls-tls-native-roots` for `tungstenite`

[DCNick3]

I have encountered something that is is not a bug per se, but rather a hard-to-debug gotcha.

It is a combination of two facts:

1. tokio-tungstenite re-exports the entire tungstenite API surface
2. rustls-tls-native-roots feature in tokio-tungstenite does not enable the rustls-tls-native-roots feature in tungstenite
3. still, it enables some support through the internal __rustls-tls feature

In our project we provide both the async and sync versions of the API by using either tungstenite or tokio-tungstenite in the implementation. Some time ago we decided not to depend on tungstenite directly, but use the re-exported version of tokio-tungstenite.

However, even though rustls-tls-native-roots on tokio-tungstenite is enabled, an attempt to made a sync connection with tungstenite to a server fails.

And the hard-to-debug part of this is the error message: Io(Custom { kind: InvalidData, error: InvalidCertificate(UnknownIssuer) }). It suggests that there's some kind of problem with the TLS certificate.

However, in actuality, it is due to the fact that the re-exported tungstenite has TLS support enabled, but doesn't have any way to verify the certificate, so it just fails with this message. It doesn't point to the root issue (lack of the TLS verification features on the `tungstenite). I think this is not a good UX.

It might have been better if rustls-tls-native-roots feature on tokio-tungstenite (and, I think, the same problem affects rustls-tls-webpki-roots) also enabled rustls-tls-native-roots feature of tungstenite. If this is infeasible, tungstenite might detect such a condition and provide an error message pointing out the lack of configured cert verification methods.

[agalakhov]

Thank you. Indeed it is a bug. We just forgot to add corresponding feature dependencies.

[jbg]

Why does tokio_tungstenite need to enable TLS features in tungstenite at all? It looks to me that the TLS is implemented using tokio-rustls or tokio-native-tls here in this crate, and unencrypted data is being passed to/from tungstenite, so enabling those features in tungstenite should be unnecessary, shouldn't it?

[daniel-abramov]

In principle, your assumptions are correct. However, we're still using some types from tungstenite such as Error::TlsError and some stuff from the stream.rs that are only available when certain features are enabled for tungstenite. But in theory, decoupling these should be possible.