fix: change TlsRootCerts to select All by default

[tdejager]

Description

This sets the TlsRootCert to All by default. I think this should be the default to cover as much bases as possible, but maybe this is insecure, I'm not super sure. Ben Moss was reporting problems where he did not have them before and needs the native certificate store as well it seems.

How Has This Been Tested?

I hope Ben Moss, could give this a go.

[baszalmstra]

I dont understand, did we previously also always load the native certificates? There is some overhead using the native certificates, so if we dont need to load them by default thats better I think.

[tdejager]

I dont understand, did we previously also always load the native certificates? There is some overhead using the native certificates, so if we dont need to load them by default thats better I think.

Apparently: https://discord.com/channels/1082332781146800168/1458197225422196868, something regressed, I also thought we did not load them by default.

[benmoss]

Yes, we seem to have been loading native certs for both conda and pypi package installations prior to 0.61.0 / #5013. I'm kinda confused as to what that feature changed now.

[tdejager]

That change was mainly made so that we could also have uv load native certificates, with like a way of sanely setting the settings because we allow native-tls builds while uv does not.

[tdejager]

@baszalmstra

Reading: https://github.com/seanmonstar/reqwest/blob/8b8fdd2552ad645c7e9dd494930b3e95e2aedef2/src/async_impl/client.rs#L227C1-L228C49 (this sets the tls_built_in_certs_native to true in case of no-roots feature.

And (same file):

                    if config.tls_built_in_certs_native {
                        let mut valid_count = 0;
                        let mut invalid_count = 0;

                        let load_results = rustls_native_certs::load_native_certs();
                        for cert in load_results.certs {
                            // Continue on parsing errors, as native stores often include ancient or syntactically
                            // invalid certificates, like root certificates without any X509 extensions.
                            // Inspiration: https://github.com/rustls/rustls/blob/633bf4ba9d9521a95f68766d04c22e2b01e68318/rustls/src/anchors.rs#L105-L112
                            match root_cert_store.add(cert.into()) {
                                Ok(_) => valid_count += 1,
                                Err(err) => {
                                    invalid_count += 1;
                                    log::debug!("rustls failed to parse DER certificate: {err:?}");
                                }
                            }
                        }
                        if valid_count == 0 && invalid_count > 0 {
                            let err = if load_results.errors.is_empty() {
                                crate::error::builder(
                                    "zero valid certificates found in native root store",
                                )
                            } else {
                                use std::fmt::Write as _;
                                let mut acc = String::new();
                                for err in load_results.errors {
                                    let _ = writeln!(&mut acc, "{err}");
                                }

                                crate::error::builder(acc)
                            };

                            return Err(err);
                        }
                    }

Looking at Cargo.toml https://github.com/seanmonstar/reqwest/blob/8b8fdd2552ad645c7e9dd494930b3e95e2aedef2/Cargo.toml#L52:

rustls-tls-native-roots = ["rustls-tls-native-roots-no-provider", "__rustls-ring"]

And we were setting in pixi

pixi/crates/pixi_utils/Cargo.toml

Line 19 in b3de9f4

rustls-tls = [

:

rustls-tls = [
  "reqwest/rustls-tls",
  "reqwest/rustls-tls-native-roots", # <----
  "rattler_networking/rustls-tls",
]

I think we were loading native indeed, so I think this fix is necessary.

[baszalmstra]

Thanks for the deep dive!