ConfigBuilder: default crypto provider

[jsha]

Right now on main the way to build a ClientConfig or ServerConfig is:

ClientConfig::<Ring>::builder()
    .with_safe_defaults()
    ...

This has two downsides:

1. Users need to know about crypto provider backends and the possibility of having different ones.
2. Changing the crypto provider requires an update to all user code.

Most users don't care about the crypto provider backend and are happy to use whatever good default rustls provides.

It would be better if the basic config builder boilerplate did not specify the crypto provider, leaving it up to rustls. That would minimize the mental overhead for users; and it would also allow us to change the default on the rustls side without requiring all users to update their code.

Because the crypto provider is part of the type parameters of ClientConfig, ServerConfig, and (transitively) ConfigBuilder, this might be a little challenging to achieve, but I think it's possible. At a minimum we would have to document that the type parameter could change across versions.

Here's a straw proposal:

diff --git a/rustls/src/client/client_conn.rs b/rustls/src/client/client_conn.rs
index 497d02a6..fb076e13 100644
--- a/rustls/src/client/client_conn.rs
+++ b/rustls/src/client/client_conn.rs
@@ -232,6 +232,18 @@ impl<C: CryptoProvider> fmt::Debug for ClientConfig<C> {
     }
 }
 
+/// Start building a ClientConfig using the default crypto provider.
+///
+/// The [`CryptoProvider`] type parameter of `ClientConfig` may change without being considered
+/// a semver-incompatible change.
+pub fn default_client_config_builder(
+) -> ConfigBuilder<ClientConfig<crate::crypto::ring::Ring>, WantsCipherSuites> {
+    ConfigBuilder {
+        state: WantsCipherSuites(()),
+        side: PhantomData,
+    }
+}
+
 impl<C: CryptoProvider> ClientConfig<C> {
     /// Create a builder to build up the client configuration.
     ///

[djc]

I think we discussed this in a PR, but I can't find it now. Some history:

• WIP Pluggable crypto galore. cpu/rustls#3
• Extract traits under crypto use #1319
• Extract traits under crypto use (part 1) #1350

IIRC the thought was that people should be aware of which crypto provider they pull in, since it's a security-sensitive choice. However, it does also make sense to me that we should probably have a fast path for the default backend that doesn't require an extra import. Given that the WantsCipherSuites doesn't actually need the CryptoProvider, maybe the way forward could be that the existing with_safe_defaults() also picks the default crypto provider?

As for how to then spell the choice between alternative providers, it might make sense to tackle #1372 first before we make further changes here.

[ctz]

I was thinking of something like:

rustls::crypto::ClientConfig<T> is the base type (location and name is up for grabs) and must be given a mandatory CryptoProvider type parameter. This is what we currently have (though it is in a different location.) People can use this irrespective of which features were used to build the crate.

rustls::ClientConfig is type ClientConfig = crypto::ClientConfig<DefaultProvider>; is the "I don't care" type. DefaultProvider is Ring if cfg(feature = "ring") else AwsLcRs if cfg(feature = "aws_lc_rs"), or else the entire item is missing if neither.

(Note AwsLcRs doesn't exist anywhere but on my disk, but hopefully the meaning is clear. My immediate goal is to make cargo test --no-default-features --feature aws_lc_rs,tls12,webpki do something useful -- this is pretty gnarly because there are lots of ClientConfig<Ring>s.)

[ctz]

Er, didn't really read the above comment from djc fully. I think we'd have to move ClientConfig::builder() somewhere else, because ClientConfig definitely does need aCryptoProvider.

[jsha]

We could make a lot of things easier if we used dynamic dispatch, so that the type of the CryptoProvider does not affect the type of the ClientConfig. I'm assuming we're doing static dispatch for performance reasons? We have a nice benchmarking system now and could test how much difference it makes.

[ctz]

I think, on reflection, I'd prefer that. Just because, like you say, the type of the CryptoProvider is being spread about in quite a number of places that don't really need to commit to that decision. I realise callers can alias it away, but that is only really achieving clearer code and less typing.

[ctz]

Rough sketch of this on #1448 -- the API ClientConfig::builder() API is restored and suitable for most uses, but now is conditional on the ring crate feature. ClientConfig::builder_with_provider(&'static dyn CryptoProvider) is unconditionally provided for being explicit or use with custom providers.

The answer to the performance question is a small but consistent improvement, which is odd but perhaps not at a significant level.

Noteworthy instruction count differences

Scenario	Baseline	Candidate	Diff
handshake_session_id_1.2_rsa_aes_server	4466192	4432459	✅ -33733 (-0.76%)
handshake_session_id_1.2_rsa_aes_client	4482597	4453976	✅ -28621 (-0.64%)
handshake_tickets_1.2_rsa_aes_client	4782890	4758818	✅ -24072 (-0.50%)
handshake_tickets_1.2_rsa_aes_server	4907282	4891753	✅ -15529 (-0.32%)
handshake_session_id_1.3_rsa_chacha_client	34648624	34578422	✅ -70202 (-0.20%)
handshake_session_id_1.3_rsa_aes_client	34676462	34606274	✅ -70188 (-0.20%)