Configuration ergonomics for client auth cert?

[Imberflur]

I noticed when providing a client authentication certificate via https://docs.rs/rustls/0.23.13/rustls/struct.ConfigBuilder.html#method.with_client_auth_cert, that I end up needing to repeat some of the configuration that is done internally in QuicClientConfig::inner (since I need to use the TryFrom impl to construct a QuicClientConfig).

The configuration for the server side of this is similar.

It seems like it would be nice if there was a way to avoid repeating this configuration (and keeping up to date with it if it needs to change) (assuming this is possible without introducing footguns in the API).

[djc]

Can you show some of the code that you think is janky?

[Imberflur]

With the goal of having the same configuration as quinn::ClientConfig::with_platform_verifier() plus with_client_auth_cert, I have some code that looks like this:

fn client_config(
    client_auth: Option<(PrivatePkcs8KeyDer<'static>, CertificateDer<'static>)>,
) -> anyhow::Result<quinn::ClientConfig> {
    let verifier: Arc<dyn rustls::client::danger::ServerCertVerifier> = Arc::new(
        rustls_platform_verifier::Verifier::new()
            .with_provider(rustls::crypto::ring::default_provider().into())
    );

    let builder = rustls::ClientConfig::builder_with_provider(
        rustls::crypto::ring::default_provider().into(),
    )
    .with_protocol_versions(&[&rustls::version::TLS13])
    .unwrap()
    .dangerous()
    .with_custom_certificate_verifier(verifier);

    let mut config = if let Some((key, client_certificate)) = client_auth {
        builder.with_client_auth_cert(vec![client_certificate], PrivateKeyDer::Pkcs8(key))?
    } else {
        builder.with_no_client_auth()
    };

    config.enable_early_data = true;

    let config = quinn::crypto::rustls::QuicClientConfig::try_from(config)?;
    Ok(quinn::ClientConfig::new(Arc::new(config)))
}

(I simplified it slightly by removing an option for a server verifier based on a RootCertStore like using quinn::ClientConfig::with_root_certificates)

[djc]

So what would your proposal be to simplify this? I agree it's a little verbose but I'm not sure we want to maintain an entirely parallel building API for rustls' ClientConfig, and it's not obvious to me how else we'd solve this.

[Imberflur]

Tbh I agree that there isn't an obvious nice solution for this and I didn't really have any particular proposal in mind when creating this issue. I'm happy if the conclusion is that there isn't any good way to improve this.

The best I can think of is a method like this on quinn::ClientConfig, and I will admit it is already a bit awkward trying to support the functionality of both with_platform_verifier and with_root_certificates by using an Option parameter:

/// If `custom_roots` is `None`, the created configuration will trust the platform's native roots 
fn with_extra_configuration(
   custom_roots: Option<Arc<RootCertStore>>,
   // user provides a closure that configures the client certificate and applies in additional configuration the the constructed ClientConfig before returning it
   f: impl FnOnce(ConfigBuilder<rustls::client::ClientConfig, WantsClientCert>) -> Result<rustls::client::ClientConfig, Error>,
) -> Result<Self, Error> { /* .. */ }

Another idea might be to expose some utility methods to create a ConfigBuilder<rustls::client::ClientConfig, WantsClientCert> with the default configuration used by quinn and to apply the default changes made to the final rustls::client::ClientConfig.

FWIW in the past few days I also realized that the documentation on QuicClientConfig is pretty clear about what configuration is needed when using the TryFrom impl to construct it (i.e. everything done in QuicClientConfig::inner is mentioned there), which actually alleviates a lot of my concerns somehow.