refactor: feature-switch for object_store CA certs

[crepererum]

Which issue does this PR close?

Closes #4870.

Rationale for this change

See issue.

What changes are included in this PR?

Feature switches, do NOT make any choice by default.

Are there any user-facing changes?

Breaking: Users now must choose their CA source.

[crepererum]

Technically you could silently ignore that and do not use any CA source (apart from the manual one). However this leads to runtime errors ("invalid cert") and is likely surprising to many users (esp. if they don't test any real cloud store in CI), so I've opted for forcing people to make a choice during compilation time.

[tustvold]

I'm a little torn on this, I feel quite strongly that things should work out of the box without users having to understand the nuances of bundled vs system provided CA bundles... Is there any way we could use whatever the current default is, but provide a runtime mechanism to override this on ClientOptions or similar?

[tustvold]

I also seem to remember @roeap mentioning using openssl as an option with delta-rs, this may or may not be relevant here?

[crepererum]

If you wanna have a default, IMHO this should be "system" rather than "webpki" for the following reasons:

• CAs should be a system admin config, not an application choice
• related to prev. point: bundles CAs don't play nice with all sorts things people want (or have to) do, like TLS interception (for debugging or compliance reasons), hardening (e.g. by restricting the set of CAs)
• also related to first point: having one set of CAs makes packaging way easier (ref: Arch Linux change)
• license issues (webpki-roots is MPL-2.0, which strictly speaking is a copyleft license and way weaker than what is standard in the Rust ecosystem)
• you easily have multiple bundles in a single application (anecdotal evidence: InfluxDB IOx had THREE different bundles at some point)

[tustvold]

So to check my understanding is correct, currently we enable rustls-tls which is equivalent to rustls-tls-webpki-roots which uses webpki-roots.

If you then add rustls-tls-native-roots it will also include any system CA present, see here.

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.

I wonder if as part of #5034 we could expose methods like use_preconfigured_tls. This would give downstreams full control of how they want to configure TLS should they so wish. Would this be sufficient?

If you wanna have a default, IMHO this should be "system" rather than "webpki"

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.

[crepererum]

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.

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 #5034 we could expose methods like 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

[tustvold]

Having spoken with Marco, we've decided on an alternative path forward for this, so closing for now to save other reviewer's time