-
Notifications
You must be signed in to change notification settings - Fork 177
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
refactor: use rustls-platform-verifier cert store #1373
Conversation
@@ -522,16 +522,6 @@ impl<T: Clone> RequestIdGuard<T> { | |||
} | |||
} | |||
|
|||
/// What certificate store to use |
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.
removed this from here because I don't want to expose another feature flag for TLS stuff
client/http-client/Cargo.toml
Outdated
default = ["native-tls"] | ||
native-tls = ["hyper-rustls/native-tokio", "hyper-rustls/ring", "__tls"] | ||
webpki-tls = ["hyper-rustls/webpki-tokio", "hyper-rustls/ring", "__tls"] | ||
default = ["tls"] | ||
|
||
# Internal feature to indicate whether TLS is enabled. |
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.
Not internal any more then? :)
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.
We had two feature flags for tls (native-tls, webpki-tls) before which was public but we don't need them anymore because we only support rustls-platform-verifier and the internal feature flag is not needed anymore
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.
ah ok, I guess you were referring to the outdated comment, removed now
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.
Little comment to remove on the tls feature and wondering if we need some builder fns but looks good to me!
The builder fns in this PR should be sufficient :) |
Close #1340
This PR replaces the current certificate stores
rust-native-cert
andwebpki
withrustls-platform-verifier
and additionally it adds an API to use a custom certificate store for special use-cases e.g. such as disabling cert verification or use some custom certificate store on some niche platform.