-
Notifications
You must be signed in to change notification settings - Fork 1k
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(tls): Remove tls roots implicit configuration #1731
feat(tls): Remove tls roots implicit configuration #1731
Conversation
Mentioned this on Discord, just repeating it here so it doesn't get lost: why do you want to remove the TLS roots features? In #1724 you didn't want to require the caller to have to depend on rustls-pemfile directly, so this seems to be reasoning in the opposite direction. |
They share the same philosophy of increasing the user's freedom of configuration without compromising ease of use as much as possible, and reducing the overall complexity of tonic to lower maintenance costs. Considering that using PEM encoded certificate files when using tls is a typical use case, I think it is a redundant interface to leave the user to make that conversion as web framework. Removing tls roots and adding interfaces to add them (in exchage for increasing internal These are intended to balance easiness to use, configurability, and tonic's maintenance costs. |
I am okay with adding the ability to use your own CA certs but I like that idea that we provide the ability to just work with either the system or webpki root certs. I think removing this kinda makes it more tough for users. If they want more flexibility they should be using their own customer connector imo. |
hyper-rustls offers high-level APIs to use rustls-platform-verifier, rustls-native-certs and webpki-roots. Maybe we can reuse that code here? (I forget if tonic can build on hyper-rustls or whether there's some reason it needs to work with tokio-rustls directly.) |
probably legacy stuff, though I am inclined to not change this code that much. We have a new transport on the way anyways. |
a575f87
to
9bdf3a9
Compare
Instead of removing these features, added options to enable tls roots, and removes the implicit configuration of them. I think this change simplifies the implementation with keeping the user friendly features. |
This makes sense to me. |
Would've been nice to mark this as a breaking change. Was confused that I could not connect after bumping tonic. |
The implicit TLS roots configuration [was removed](hyperium/tonic#1731) on `tonic` `v0.12.0`.
Adds ability to add CA certificates and removes
tls-webpki-roots
andtls-roots
features.