-
-
Notifications
You must be signed in to change notification settings - Fork 285
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
rustls: optionally use WebPKI roots to avoid panicking on Android & iOS #1323
base: main
Are you sure you want to change the base?
Conversation
Hey, thanks for this! I do think this, or some variant of this makes sense, and am happy to include a variant of this. The few bits i am sceptical of atm are:
There are two alternatives I think;
I think 1. makes the most sense, but it might be hairier to make a feature for this? At the very least reqwest has a quite hairy feature selection setup to get all the things exposed. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1323 +/- ##
=======================================
+ Coverage 72.4% 72.4% +0.1%
=======================================
Files 75 75
Lines 6343 6343
=======================================
+ Hits 4586 4587 +1
+ Misses 1757 1756 -1
|
Makes sense. Happy to try out either one of the two scenarios! But is the failing CI check about |
It shouldn't be a problem. Particularly if it's an opt-in feature, as then it's up to users whether they want the stricter license the transitive dependency comes with. |
Given we want opt-in for many reasons, i think if we add a feature for it in webpki-roots = ["hyper-rustls/webpki-roots"] plus re-export it from then we should be good. we would need a |
Gotcha. Do you think the feature should gate the current check to use WebPKI roots only on Android and iOS, or just alternate between native and WebPKI roots on every platform by setting the feature flag? My gut feeling is the latter is probably the cleaner thing to do and less confusing for an API consumer. |
Signed-off-by: Elias Wilken <elias@nautik.io>
…cargo-deny Signed-off-by: Elias Wilken <elias@nautik.io>
Signed-off-by: Elias Wilken <eliasw@me.com>
Motivation
Using
kube-rs
on iOS, I came across a panic in the underlyinghyper-rustls
root cert initialization trying to access the native roots, which doesn't work on Android or iOS, according to this open issue.Solution
My solution was to add a check to fall back to the WebPKI roots when on Android or iOS, which fixes the issue for me, but could make sense upstream, too.
Please let me know if this change makes sense to you or whether I'm missing anything. And thanks a lot for all the work you've done on the
kube-rs
ecosystem! 😊