crepererum commented on PR #5030: URL: https://github.com/apache/arrow-rs/pull/5030#issuecomment-1792203233
> So to check my understanding is correct, currently we enable `rustls-tls` which is equivalent to `rustls-tls-webpki-roots` which uses `webpki-roots`. correct. > If you then add `rustls-tls-native-roots` it will **also** include any system CA present, see [here](https://github.com/seanmonstar/reqwest/blob/50dbaf391087cfa951accc765126b4f5d017d8a3/src/async_impl/client.rs#L481). correct. > This PR is therefore only adding the ability to use feature flags to only use system roots. This feels like a less common requirement, but is definitely something we should permit not correct: this PR removes `rustls-tls` and replaces it by `rustls-tls-manual-roots` which is "enable rustls but do NOT include any roots at all, neither webpki nor system". It then offers you two feature switches which allows you to include one or both of them (in the latter case they are merged). > I wonder if as part of https://github.com/apache/arrow-rs/issues/5034 we could expose methods like [use_preconfigured_tls](https://docs.rs/reqwest/latest/reqwest/struct.ClientBuilder.html#method.use_preconfigured_tls). This would give downstreams full control of how they want to configure TLS should they so wish. Would this be sufficient? You would still need to expose a "use system CAs" feature because w/o that system CAs aren't available at all. The reason is that using system CAs needs additional dependencies, see <https://crates.io/crates/rustls-native-certs/>. We could also bake-in both (webpki and system) and expose a switch, but that makes `object_store` quite heavy (and also doesn't provide users any way to escape the license issue). We could also clone all features (`cloud`, `aws`, ...) and introduce new variants (e.g. `cloud-no-ca`, `aws-no-ca`, ...) but I'm not sure if that's any better. > I don't disagree, this was an oversight on my part I enabled rustls-tls not realising it would override the system CA store, but I would really rather avoid a breaking change if we can avoid it. With my prev. paragraph, I think we basically have the following options: - do nothing - include everything (webpki and system) all the time and expose runtime config options, that makes this crate here quite heavy (due to the larger amount of indirect deps) - have more feature variants, comes w/ maintenance burden - breaking change -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
