(WIP — do not merge) Switch from native-tls to rustls

8573 commented 5 years ago

This is an unfinished patch to switch irc from depending on tokio-tls and native-tls, which @aatxe says "has caused a reasonable amount of annoyance", to tokio-rustls and (through it) rustls.

This patch uses the 0.8.0-alpha version of tokio-rustls, which has a tokio-tls-esque API. Even if this patch were finished, it should not be applied until that tokio-rustls API fully is released.

While irc compiles with this patch applied, the following work remains to be done:

Implement CertFP — rustls provides a less convenient API for client certificate authentication than does native-tls, and I (@8573) don't understand cryptographic APIs well enough to implement CertFP with it. This WIP version of my patch provides only a stub implementation of the necessary rustls trait, ResolvesClientCert, which implementation will not actually provide any client certificate to rustls, even if one is specified in the irc-level client Config.
Run irc's test suite with this patch applied.
Confirm that irc's dependents still work with this patch applied.

8573 commented 5 years ago

I can run irc's test suite (I assume) and check that my irc-bot still works with this patched version of irc, but I'm not sure I, being unlearned in cryptography, should try to plug sundry components together to be the one to get a working CertFP implementation.

retep998 commented 5 years ago

Note that native-tls is able to avoid building any C dependencies on Windows whereas rustls would pull in ring which has to build a C library.

DoumanAsh commented 5 years ago

As of 0.13 ring is pretty safe to be build on windows.

Though making feature to choose between native-tls and rustls could be convenient

aatxe commented 5 years ago

Just a note to clarify the "reasonable amount of annoyance" comment, it was directed mainly at OpenSSL which seems to break builds frequently, rather than native-tls in general.

retep998 commented 5 years ago

@DoumanAsh Unless you're using pc-windows-gnu, in which case you'd have to setup a whole MinGW toolchain instead of using the stuff bundled with Rust.

tirz commented 4 years ago

Any news on this topic ?

If needed, I will be happy to fill a PR for the RustTls support.

I guess I will have to:

put native-tls and tokio-tls on a feature (it will allow us to compile the project without them - and will make it much easier to compile for android)
add a new feature with tokio-rustls and webpki-roots (and choose which one should be enabled by default)
then, it may be nice to isolate the ssl logic to a ssl.rs for readability
finally, cleanup/refactor conn.rs since we may have some code duplication

I can send all in one PR or split each task into a their own PR.

8573 commented 4 years ago

@tirz: If you're familiar with the relevant cryptography, that sounds good to me. I certainly am unlikely to finish this work myself.

tirz commented 4 years ago

@tirz: If you're familiar with the relevant cryptography, that sounds good to me. I certainly am unlikely to finish this work myself.

I successfully implemented it on my fork: https://github.com/tirz/irc/tree/feature-ssl_backends I will fill a PR soon.

Edit: I am just waiting for the proxy feature to be reviewed and then I will still have to implement a custom certificate selection for rustls.

aatxe commented 4 years ago

gonna close this PR since we're hopefully accepting #203 (as soon as Travis goes through)

thank you for the initial work on this though, @8573

aatxe / irc

(WIP — do not merge) Switch from native-tls to rustls #151