Add feature to support rustls-native-certs root certificates#134
Add feature to support rustls-native-certs root certificates#134sbstp merged 3 commits intosbstp:masterfrom
Conversation
| }; | ||
| #[cfg(feature = "tls-rustls-native-roots")] | ||
| use rustls_native_certs::load_native_certs; | ||
| use webpki_roots::TLS_SERVER_ROOTS; |
There was a problem hiding this comment.
Shouldn't webpki_roots be disabled when native certs are used?
There was a problem hiding this comment.
I think that it's likely that users that enable the native certs feature do not want the webpki certs. They want to rely on the OS' cert store and not on Mozilla's.
There was a problem hiding this comment.
That's a good question that I wasn't totally sure of. I noted in the description that I left it as-is initially so that the feature flag would be additive as opposed to a toggle. Reqwest has two separate feature flags for webpki and native, so you could enable both or only one. I'm not super knowledgeable about the expected behaviors, so I don't have a good sense of if having only the native certs would cause issues.
There was a problem hiding this comment.
Ah, that loaded weird, didn't see your second comment right away! But yeah, that makes sense. I'll update to make the native certs exclusive of the webpki ones.
There was a problem hiding this comment.
Doesn't have to be you who does this, but I think the TLS features should be cleaned up. Maybe something like:
tls-nativetls-native-vendoredtls-rustls-webpki-rootstls-rustls-native-roots
There was a problem hiding this comment.
Yeah, that would make sense. Would probably need to keep the various aliases around, at least for backwards compatibility, but it would definitely make it clearer which features are intended for which use-cases.
I took a quick look, my guess is it should be a separate PR, because there's a lot of various places that reference the current feature names that would need to be changed. I was also running into some weird behavior from Cargo that I didn't fully understand, where it wasn't importing the optional dependency even though it was declared to include it 🙁
There was a problem hiding this comment.
Actually I think I got it working in #135 (which is stacked on this one to also include the tls-rustls-native-roots feature flag).
|
Yeah I'm planning to today |
|
Perfect, thanks! |
|
Published 0.24.0 |
Closes #133
Info
rustls-native-certsas root certificates allows consumers to easily opt-in to the native certificate stores.tls-rustlsas well.Changes
tls-rustls-native-rootsto enable loading the native certificates as root certificates for requests.Notes
"."to'.'to resolve a clippy warning about using a single-character string as a pattern.