feat(tls): extends ServerTlsConfig for optional KeyLogFile

[markdingram]

Motivation

Tonic no longer allows setting Rustls config directly.

Solution

This PR starts off down the road adding piecemeal configurability into the Tonic TLS config to aim for Rustls config parity from Tonic once more. In this instance a setting to install KeyLogFile, reinstating the possibility of Wireshark TLS decryption.

[LucioFranco]

Is this something that is dangerous and we'd want to put behind a dangerous feature flag?

[olix0r]

Why only support a single implementation of KeyLog? Would it be better to support providing the KeyLog implementation as a Arc<dyn KeyLog>, as it's actually exposed in the ServerConfig?

https://github.com/rustls/rustls/blob/5bda754ac18f37eb39132f89fb5522494b6202eb/rustls/src/server/server_conn.rs#L221-L223

Or better yet, what about supporting providing an arbitrary rustls::ServerConfig so we don't need have helpers for each and every option exposed by rustls?

[LucioFranco]

Yeah I wonder if we should just copy the rustls struct and slap a non_exhaustive on it?

[olix0r]

@LucioFranco note that you can't actually build a ServerConfig except through the safe builder API. It would probably best to take the builder-generated ServerConfig directly when rustls is enabled.

[LucioFranco]

@olix0r by take you mean expose it publicly in tonic?

[olix0r]

@LucioFranco probably? I'm not sure if it's worth providing a simplified abstraction over TLS configuration, given the finicky nature of TLS, unless the goal is to support multiple TLS implementations...

[LucioFranco]

@olix0r only reason I bring it up is we used to expose it but ended up removing it because we would be forced to release a new version of tonic for new versions of rustls which didn't seem ideal. I wonder if there is a better way forward by extracting the tls part to its own crate similar to what hyper does. There is a lot of overlap here since axum will also need to solve this. cc @davidpdrsn

[olix0r]

@LucioFranco another option might be to expose tonic-specific Acceptor/Connector traits with implementations for tokio_rustls::TlsAcceptor/tokio_rustls::TlsConnector?

[LucioFranco]

Yeah that is what I am leaning towards. Though I think there are some complications with acceptors that I outlined in this hyper issue. hyperium/hyper#2321

[olix0r]

@LucioFranco the other benefit of that approach is that it could let us support multiple TLS implementations in an --all-features-safe way. While rustls is generally great, it currently has a number of limitations, such as being unable to use TLS certs bound to IPs instead of DNS names. Being able to integrate with tokio_boring, etc, would be nice.

[LucioFranco]

Yeah, I think that is the right solution....will be some work but probably worth it. Over actually moving over to axum.

[markdingram]

obsoleted by #968