Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification regarding CryptoProvider #353

Closed
mhutter opened this issue Oct 30, 2024 · 4 comments
Closed

Clarification regarding CryptoProvider #353

mhutter opened this issue Oct 30, 2024 · 4 comments
Labels

Comments

@mhutter
Copy link

mhutter commented Oct 30, 2024

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
Copy link
Member

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
Copy link
Author

mhutter commented Nov 4, 2024

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
Copy link
Author

mhutter commented Nov 4, 2024

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

  • starting with v0.23, rustls allows selecting between ring and aws-lc-rs
  • when using rustls with the default features, the aws-lc-rs provider is available, but not ring.
  • if exactly one of the two providers is available (that is, your application and all its dependencies only select either via features), it will be the default one.

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

  • tungstenite enables neither of the providers and leaves selection to the application
  • async-nats on the other hand enables the ring feature by default, enabling the ring provider

Which one is the "correct way to do things" ™️ 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!

@mhutter mhutter closed this as completed Nov 4, 2024
@daniel-abramov
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants