Implicit reliance on the OS CA certificate root store

[djc]

I am building an application which I'd like to deploy as a minimal container (into Google Cloud Run, or equivalent). As such, I'm compiling my application as a single binary that gets pushed into a container based on the empty scratch image. However, yup-oauth2 uses hyper-rustls to build the connection used to acquire access tokens, which (since version 0.18.0) relies on the OS CA root store by default. As such, requests to grab a token from within my container fail with a WebPKI::UnknownIssuer error. I see a few options to improve on this behavior:

1. Always use webpki-roots by adding the relevant feature to the dependency on hyper-rustls
2. Optionally use webpki-roots by adding a feature to yup-oauth2 that makes it possible to configure the behavior
3. Some change the API such that users who want to rely on hyper-rustls must pass in an explicit rustls::ClientConfig

There are definitely advantages to rustls-native-certs -- in a lot of contexts it makes sense to rely on the OS root store, making it so that the root store can be modified in technology neutral way and applications can pick up updates from the OS without the applications themselves having to be updated. So 1 does not seem the best option here. Option 2 is okay, but I think I prefer option 3: I feel like configuring the set of TLS root stores is a highly relevant, security-sensitive piece of configuration that should probably be made explicit.

Would like to hear your thoughts on this. I'm happy to work on a solution in either case.

[djc]

I ended up filing rustls/hyper-rustls#125 instead, since I found that hyper-rustls does try to abstract over the differences between webpki-roots and rustls-native-certs. Maybe this crate should grow a Cargo feature to forward to hyper-rustls?

[dermesser]

Hello! Thanks for filing this issue.

Between (1) and (2) I would obviously prefer (2) - optional is always better :)

On (3): We already allow (in fact - require) passing in your own hyper client to the Authenticator. I haven't found out so far how to configure an HttpsConnector in a way that would use a different set of PKI roots (like rustls allows to do). In any case -- shouldn't the TLS configuration of the client happen in the calling code, not this library?

[djc]

Well, you also provide a DefaultHyperClient, which will be used by default when using the ServiceAccountAuthenticator::builder(); so at least the downstream crate I was looking at (opentelemetry-stackdriver) just went with that by default. I can submit a PR for something like this:

diff --git a/Cargo.toml b/Cargo.toml
index cd6c2d1..868c5bb 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -10,12 +10,16 @@ keywords = ["google", "oauth", "v2"]
 license = "MIT OR Apache-2.0"
 edition = "2018"
 
+[features]
+default = ["hyper-rustls/native-tokio"]
+webpki-roots = ["hyper-rustls/webpki-tokio"]
+
 [dependencies]
 base64 = "0.12"
 chrono = { version = "0.4", features = ["serde"] }
 http = "0.2"
 hyper = "0.13.5"
-hyper-rustls = "0.20"
+hyper-rustls = { version = "0.20", default-features = false }
 log = "0.4"
 rustls = "0.17"
 seahash = "4"

but IMO it would be preferable to force downstream crates to pass in their own rustls::ClientConfig and/or at least make them chose between webpki-roots and rustls-native-certs explicitly, because of the different runtime and security trade-offs.