rustls / rustls-ffi

Use Rustls from any language
Other
127 stars 30 forks source link

Make `rustls_client_config_builder_build` fallible, rm `NoneVerifier` #422

Open cpu opened 5 months ago

cpu commented 5 months ago

Followup from https://github.com/rustls/rustls-ffi/pull/421 and https://github.com/rustls/rustls-ffi/issues/409

The rustls_client_config_builder_new docs say:

/// This starts out with no trusted roots.
/// Caller must add roots with rustls_client_config_builder_load_roots_from_file
/// or provide a custom verifier.

If the caller must set up a verifier, it seems clearer to error at build-time instead of verification time.

The NoneVerifier is only used for the circumstance where we build a client config without the user providing their own verifier with rustls_client_config_builder_dangerous_set_certificate_verifier or rustls_client_config_builder_set_server_verifier. If the ClientConfigBuilder instead changed the verifier field from Arc<dyn ServerCertVerifier> to Option<Arc<dyn ServerCertVerifier>> and rustls_client_config_builder_build returned an error for the case where verifier was None, we could remove NoneVerifier and I think bugs that lead to accidentally failing to set a verifier would be easier to catch/debug. I can't think of a use-case where you would want the NoneVerifier behaviour (and users that do want it can implement it themselves, providing their impl as the explicit verifier).

This is a breaking change, so we should hold it until we're ready to make a few similar changes at once.