Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: add support for rustls #26

Closed
wants to merge 1 commit into from
Closed

Conversation

aidanhs
Copy link

@aidanhs aidanhs commented Mar 6, 2017

Putting rustls inside native-tls is motivated by issues like tokio-rs/tokio-tls#16 and seanmonstar/reqwest#63 - if the Rust ecosystem can just depend on one tls provider crate with a common interface and swappable backends, that'd be great (indeed, that's what native-tls offers right now with a hardcoded choice per platform).

This PR is an initial attempt at adding rustls support to native-tls, implementing most of the support for tls clients by re-using code from the license-compatible hyper-rustls. Both client tests (connect_google, connect_bad_hostname) now pass for me. You can try it out with RUSTFLAGS='--cfg nonnative_tls' cargo test.

The largest piece of work to do for the server tests is getting pkcs#12 support (which I've started a discussion about), but I also want to get some early thoughts on what this will look like to an end-user. One question is how to expose the option to use the rustls backend, as there are a few requirements:

  • it's not ideal to require users to use RUSTFLAGS="--cfg blah" (though I'm perfectly happy with it for my own use!)
  • libraries depending on this crate ideally should be blind to the backend (though the *Ext traits make that a little tricky) and so they should not generally specify which backend they want - that should be the role of the top level crate being built to specify (if it wants to)
  • you'd like the backends to be mutually exclusive by default, but not forcibly so - it isn't an error to have native-tls with multiple backends (it could rank backends and if not instructed specifically to use one, choose the highest ranked), it's just a bit...odd

I don't know there's a good solution for 'soft' mutual exclusion. Hard mutual exclusion might actually be fine - I'm not sure there's any reason to allow multiple tls backends in the same program (and I don't think this has to cover every possible use-case ever anyway).

So let's say the backends are mutually exclusive and libraries don't get to choose - how do you pick one? I wonder if automatic features could be leveraged(/misused) here (I've not read them in depth) - I think the natural way you'd combine them with mutual exclusion is a ranking i.e. we detect openssl and rustls, but only one is allowed and so we choose openssl since that's ranked higher, for backwards compatibility if nothing else (in fact, even without mutual exclusion, ranking would be doable inside native-tls). And now the top level can choose the crate that automatically implements their preferred native-tls backend.

Anyway, there are some thoughts, some of which may be crazy :) As I say, I'm totally happy if (when this PR is ready) rustls is merged as an 'unstable' feature only accessible through RUSTFLAGS, but I wanted to get the ball rolling on the discussion so you're aware.

@aidanhs
Copy link
Author

aidanhs commented Mar 6, 2017

@ctz ping just so you're aware of this as the author of rustls :)

@sfackler
Copy link
Owner

Thanks for the detailed writeup! I've been a bit conflicted over this exact issue in the past.

First, on the technical side, I'm imagining that if we did this that there'd just be a force-rustls feature, which would disable the normal backend in favor of rustls.

However, I'm not sure this matches up with this library's objectives. This crate is designed to specifically target the "I don't particularly care what I'm using but I just want the common case to work with minimal effort on my part" use case. If you want to use RusTLS in particular, you should use RusTLS, just as you should use OpenSSL if you want OpenSSL in particular. An abstract provider interface would be cool - there's some prior art in Hyper that could be yanked out somewhere, but this crate would just be one of many consumers of that interface IMO.

@aidanhs
Copy link
Author

aidanhs commented Mar 28, 2017

However, I'm not sure this matches up with this library's objectives. This crate is designed to specifically target the "I don't particularly care what I'm using but I just want the common case to work with minimal effort on my part" use case.

I had a think about this and I do see your point. However, it might also be worth thinking about use cases - assuming there was a 'tls-interface' crate which (somehow) picked a default tls implementation but also permitted you to override with rustls or something else, when would you choose to use native-tls directly? Or, to restate the question, when would you want to forbid any implementation except the platform default?

If positioned like that, tls-interface and native-tls effectively become one entity - there isn't much benefit separating them apart from conceptual purity. Not that that's necessarily a bad thing.

@sfackler
Copy link
Owner

sfackler commented Apr 2, 2017

This style of crate is the wrong way to define a "global" abstraction, IMO. The various libraries just don't expose enough of a unified interface for it to be sufficiently powerful. A more limited setup like what's used by hyper or postgres is the right direction to go.

@aidanhs
Copy link
Author

aidanhs commented Apr 6, 2017

I'm not sure I understand what you mean by "powerful" - the 'crait' (crate-trait) can be very limited in scope so that it covers the bare essentials necessary to perform some task (native-tls being the obvious example of exposing some minimum to do tls).

It seems that I'm effectively talking about virtual packages (https://www.debian.org/doc/debian-policy/ch-relationships.html#s-virtual, http://askubuntu.com/questions/738980/how-does-apt-choose-the-specific-package-to-install-in-order-to-provide-a-virtua) as they're called in apt, though with some niceties to integrate better with cargo/the rust ecosystem. I may raise this idea on the cargo repo once I've thought it through a bit more.

@sfackler
Copy link
Owner

sfackler commented Apr 6, 2017

You can't even select cipher suites with native-tls. See #4 for why even an operation like that is arbitrarily hard.

@sfackler sfackler closed this Apr 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants