Rustls dangerous

[jkalez]

Resolved issues:

Resolves this comment

Description of changes:

Currently, s2n-quic provides no method of doing custom certificate validation. This presents a challenge for a variety of scenarios where the client may want to perform more complex logic around the certificate verification. This PR attempts to resolve this by exposing rustls's ServerCertVerifier trait and a corresponding method in the client Builder.

Call-outs:

IMO custom certificate validation and certificate validation built-in via rustls should be mutually exclusive. After all, it's unclear when the client should use custom validation or built-in validation. Towards that end, the builder fails if both with_certificate and with_custom_certificate_verifier are provided.

Testing:

Two new tests have been added to s2n-quic-rustls/src/lib.rs to validate these changes.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[camshaft]

Hey @jkalez. Thanks for the contribution. Unfortunately, we can't accept this change at this time, since we're unable to export any rustls types in the public API due to semver guarantees (see #2173).

Until there is a clear path forward, the only option would be to opt in to the deprecated export of rustls and construct a client with Client::new. Do note, that we can't make any guarantees around these types changing between minor s2n-quic releases.

[jkalez]

Fair enough. Is there any appetite for exposing custom certificate validation via the wrapped Client Builder API, similarly to how other TLS configuration is done? As I understand, this has been s2n_quic's approach so far to dealing with rustls's unstable API.

[camshaft]

In general that approach works. The current methods on the builder are relatively simple interactions (no callbacks) so we been able to preserve compatibility across rustls versions.

But the issue with the cert verification is it has a pretty complicated trait/interaction and anything we do to abstract over it will end up being an issue when rustls changes their API and our abstraction no longer fits into their model.