snapview / tokio-tungstenite

Future-based Tungstenite for Tokio. Lightweight stream-based WebSocket implementation
MIT License
1.88k stars 236 forks source link

Clarification regarding `CryptoProvider` #353

Closed mhutter closed 2 weeks ago

mhutter commented 3 weeks ago

From the discussions in #336 and #339 it is unclear what users are supposed to do now; nor is there any mention of this (breaking) change in the CHANGELOG; nor does the documentation say anything about it.

Unfortunately the note

Disable default features for rustls giving the user more flexibility.

does not help for someone that never used rustls directly :)

daniel-abramov commented 2 weeks ago

We follow the semver to indicate breaking changes, but it's true that we did not specifically describe what must be called from the rustls crate for the problem to disappear. That was partially due to the fact that we discovered the particular error only after the release once some users started complaining.

it is unclear what users are supposed to do now

Did you have a chance to check suggestions from other users? AFAIR the fix was mentioned a couple of times across different issues and solved the problem for those affected.

If I'm following you right, I assume you suggest including the information about that line of code (rustls stuff) in the README?

mhutter commented 2 weeks ago

Hi @daniel-abramov, thanks for taking the time!

Did you have a chance to check suggestions from other users?

Yes, but there were multiple solutions mentioned, and most of them mention adding random additional dependencies and code, but none explained why.

From what I read I understand that I can add rustls as a direct dependency, and call either rustls::crypto::aws_lc_rs::default_provider().install_default().unwrap() or rustls::crypto::ring::default_provider().install_default().unwrap() to make the problem go away, but I don't understand why this is happening.

Is this behavior that changed in the latest rustls version, did tungstenite's usage of rustls change, or is the issue on my end?

mhutter commented 2 weeks ago

After some digging & experimentation, I think I have a more complete understanding of the issue now :-)

Now in my case, two crates I use depend on rustls: tokio-tungstenite and async-nats.

Which one is the "correct way to do things" :tm: is another discussion. If I understand the rustls docs on the topic correctly, their intention is to do it as tungstenite does (and I tend to agree). But I'm sure many people have strong opinion on that topic, so I'm not going down THAT rabbit hole today :-)


TL;DR for people that don't care and just want to fix their app:

  1. Add rustls with default features (or at least the aws-lc-rs feature) to your Cargo.toml
  2. Add rustls::crypto::aws_lc_rs::default_provider().install_default().unwrap() near the top of your fn main()

Regarding

If I'm following you right, I assume you suggest including the information about that line of code (rustls stuff) in the README?

I (incorrectly) assumed there was a straight-forward, singular solution to the issue. I would have liked a pointer on how to fix the breakage in the release notes though.

Thanks for rubber-ducking!

daniel-abramov commented 2 weeks ago

You're right that it's a bit perplexing, especially for users unfamiliar with rustls. I agree that it's not optimal, and we might have resolved this by adding another feature that calls install_default(). However, we already have multiple TLS features, and I'm afraid that at some point, it gets too complicated. Hitting a balance between flexibility and simplicity is not always an easy task, but I'm open to solutions!

Meanwhile, I've added a small note to the README that links this discussion, so hopefully, it will make it less confusing!