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

fix: Explicitly set rustls provider before using rustls #1424

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

popzxc
Copy link
Contributor

@popzxc popzxc commented Jul 10, 2024

Recently rustls changed default crypto provider from ring to aws-lc-rs, and it caused some crates to depend on both features (e.g. if one dependency uses a default provider, and other one enables ring).
In such cases, rustls would panic, because it can't choose a default provider (see this issue).

This PR makes sure that the provider is set before using rustls functionality, thus preventing panics in runtime.

This isn't the most elegant solution to the problem, but it does fix it (confirmed in my project), so I hope it's fine to merge it as a quickfix.

@popzxc popzxc requested a review from a team as a code owner July 10, 2024 08:09
Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey,

Did you try specifying ring = { version = *, default-features = false, features = ["ring", "logging", "std", "tls12"] } in jsonrpsee to fix it?

Or is this the case that some other crate brings in aws-ls-rs and rustls priorities aws-lc-rc when both ring and aws-ls-rc are defined?

Anyway, I'm fine with this hack.

@popzxc
Copy link
Contributor Author

popzxc commented Jul 29, 2024

Or is this the case that some other crate brings in aws-ls-rs and rustls priorities aws-lc-rc when both ring and aws-ls-rc are defined?

Yes, I had both jsonrpsee (which brings ring) and other crate (which brings aws-ls-rs) in deps

@niklasad1 niklasad1 merged commit c70c10b into paritytech:master Jul 29, 2024
10 checks passed
@niklasad1
Copy link
Member

Thanks @popzxc, we'll release a new version tomorrow.

@popzxc popzxc deleted the explicitly-setup-tls-provider branch July 30, 2024 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants