-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat: change default crypto provider to match rustls' #50
Conversation
ca3a2e4
to
bde59e1
Compare
I thought about it again, even if we forward it as is, or don't forward the feature, it is still necessary to configure |
Any thoughts on this crate not setting (or re-exposing) any of the ring/aws_lc_rs crate features? Given the top-level API here requires an application to supply a In fact, I think this crates features can be reduced to just (Naturally for tests and example code, it would be necessary to depend on rustls with default features as a dev-dependency.) |
I would be in favor of that. |
I'll make the case for surfacing all rustls features in this crate as a distinct concern from what the default features are: Currently If I'm a downstream crate depending on tokio-rustls or futures-rustls, this is rustls from my perspective and I wouldn't necessarily need to depend directly on rustls other than to enable features. This is a cargo problem in that ideally we'd be able to enable features on transitive dependencies with something like |
Sorry, yes, I also agree with that. I think tokio-rustls often acts as a kind of wrapper crate, so we should actually mirror rustls' defaults and features -- the sibling transitive dependency is definitely a bit of an anti-pattern. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a CI job to test the ring crypto provider? Presumably since we don't actually have crypto provider-dependent code in here it's not actually necessary?
Thank you! |
It would be confusing for rustls docs to say "by default it uses aws-lc-rs for implementing the cryptography in TLS" but discover that tokio-rustls changes the default.