feat: add Verifier::set_provider and Verifier::with_provider

[jbr]

This is offered as a possible solution for #79, which I just encountered.

Implementation notes, let me know if any of these assumptions were wrong:

• I renamed default_provider fields to crypto_provider to make clear that it's not necessarily the process-default.
• I added both a set_provider and with_provider, the latter of which is convenient for Verifier::new().with_provider(Arc::new(...)) but I'll replace with Verifier::new_with_provider(...) if that's more in line with this crate's style.
• I opted to panic if the crypto provider has already been set instead of returning an indication of failure, with the expectation that it's a bug if this is called multiple times on a given Verifier. We have &mut, no panic or error needed

[cpu]

Thank you for picking this up! The general approach seems like the right solution.

[nnmkhang]

Hi Everyone!

Thanks @jbr for picking this up so fast, I'm not sure what type of testing you have done but I tried to verify this on my end with a custom crypto provider but I am running into some issues.

Here is the snippet of code that I am trying to get working.

let mut config = rustls::ClientConfig::builder_with_provider(Arc::new(default_symcrypt_provider()))
        .with_safe_default_protocol_versions()
        .unwrap()
        .dangerous()
        .with_custom_certificate_verifier(Arc::new(Verifier::set_provider(Arc::new(default_symcrypt_provider))))
        .with_no_client_auth();

And im getting the following error, what am I doing wrong here?

error[E0277]: the trait bound `(): ServerCertVerifier` is not satisfied
  --> bin/sample_internet_client_platform.rs:20:43
   |
20 |         .with_custom_certificate_verifier(Arc::new(Verifier::set_provider(Arc::new(default_symcrypt_provider))))
   |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `ServerCertVerifier` is not implemented for `()`
   |
   = help: the following other types implement trait `ServerCertVerifier`:
             Verifier
             WebPkiServerVerifier
   = note: required for the cast from `Arc<()>` to `Arc<(dyn ServerCertVerifier + 'static)>`

I've also noticed on the rustls documentation for ClientConfig that builder_with_provider does not take in the WantsVerifier struct.

Please forgive my new-ness to rust but would this be the cause of the error? Or is there something wrong with the configuration on my end?

Appreciate the help here!

[jbr]

@nnmkhang It's still a PR and CI is currently failing, but with the current iteration, I think you want code that looks like

let provider = Arc::new(default_symcrypt_provider());

ClientConfig::builder_with_provider(provider.clone())
   .with_safe_default_protocol_versions()
   .unwrap()
   .dangerous()
   .with_custom_certificate_verifier(Arc::new(Verifier::new().with_provider(provider)))
   .with_no_client_auth()

Note the Verifier::new().with_provider(provider) instead of Verifier::set_provider(provider)

The awkwardness of this construction motivates #86, which that code was directly taken from

Alternatively, if you wanted to use Verifier::set_provider instead of Verifier::new().with_provider,

let provider = Arc::new(default_symcrypt_provider());

let mut verifier = Verifier::new();
verifier.set_provider(provider.clone());

ClientConfig::builder_with_provider(provider)
   .with_safe_default_protocol_versions()
   .unwrap()
   .dangerous()
   .with_custom_certificate_verifier(Arc::new(verifier))
   .with_no_client_auth()

[complexspaces]

Thanks for working through this with us! I think this is looking quite nice now.

[cpu]

Really nice! Thanks again.

[nnmkhang]

@jbr, Just tried it again on my my machine and it works with the custom provider and the platform verifier enabled! Verified with event viewer on windows as well.

Thanks for your help and sorry for my silly mistake.

[cpu]

I think we should square away #86 and #78 and then consider a 0.3.1 point release.

[complexspaces]

Sounds good to me.

[cpu]

v/0.3.1 is now available with this change included.