Skip to content
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

Only use webpki certs despite enabled rustls-tls-native-roots feature #1843

Closed
daxpedda opened this issue May 21, 2023 · 5 comments · Fixed by #2232
Closed

Only use webpki certs despite enabled rustls-tls-native-roots feature #1843

daxpedda opened this issue May 21, 2023 · 5 comments · Fixed by #2232

Comments

@daxpedda
Copy link
Contributor

daxpedda commented May 21, 2023

I would like to have more fine-grained control over the root certificates added regardless of enabled crate features. ClientBuilder::tls_built_in_root_certs() (added in #1150) only allows to disable/enable all of them.

One solution would be to disable all of them and add any desired ones yourself. This didn't turn out to be ideal because webpki offers already parsed certificates, but ClientBuilder::add_root_certificate() takes a Certificate, which only takes DER or PEM encoded certificates.

Suggested Solutions

  1. Add a new method, Certificate::from_rustls(), to Certificate, which takes a OwnedTrustAnchor. This would make it easy to add arbitrary root certificates to rustls without having to serialize them to DER first, only to have them deserialized by rustls right after.
  2. Add new methods to ClientBuilder allowing for more fine-grained control over which built-in certificates are added. E.g. ClientBuilder::tls_webpki_root_certs() and ClientBuilder::tls_native_root_certs(). Potentially removing ClientBuilder::tls_built_in_root_certs() completely in the next version.

I actually find both solutions could work quite well simultaneously.

@seanmonstar
Copy link
Owner

We could do something like no_gzip(), no_proxy(), etc, where even if a feature is enabled, that ClientBuilder will not use them.

@daxpedda
Copy link
Contributor Author

Will make a PR shortly.

@charliermarsh
Copy link

We also had this need in uv and ended up doing the configuration out of reqwest: astral-sh/uv#2362. It would be great if the builder API supported this!

charliermarsh added a commit to astral-sh/uv that referenced this issue Mar 12, 2024
## Summary

It turns out that on macOS, reading the native certificates can add
hundreds of milliseconds to client initialization. This PR makes
`--native-tls` a command-line flag, to toggle (at runtime) the choice of
the `webpki` roots or the native system roots.

You can't accomplish this kind of configuration with the `reqwest`
builder API, so instead, I pulled out the heart of that logic from the
crate
(https://github.com/seanmonstar/reqwest/blob/e3192638518d577759dd89da489175b8f992b12f/src/async_impl/client.rs#L498),
and modified it to allow toggling a choice of root.

Note that there's an open PR for this in reqwest
(seanmonstar/reqwest#1848), along with an issue
(seanmonstar/reqwest#1843), which I may ping,
but it's been around for a while and I believe reqwest is focused on its
next major release.

Closes #2346.
@seanmonstar
Copy link
Owner

After a change in mind as to how exactly to expose this, there's a new PR at #2232.

@charliermarsh
Copy link

That looks great. We'll definitely use it. Thanks @seanmonstar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants