Restore support for native-tls

[Shnatsel]

I see that ureq used to support native-tls, but that was dropped in version 2.0.

This is a bit of a problem for my use case which prioritizes portability over confidentiality of the data.

As much as I love rustls, it relies on ring, which has the drawback of only working on x86 and ARM. This excludes e.g. POWER9, which is currently the only option for a high-end desktop or server with fully open-source firmware.

On the other hand, I'm querying publicly available information over HTTPS, so confidentiality of the data is not a high priority.

I understand that native-tls support was removed due to the added maintenance burden; however, I see that most of the other HTTP clients support both (e.g. attohttpc, minreq, reqwest, surf/http-client, mio_httpc, and others). Perhaps they have discovered a good way to abstract over both libraries?

Lack of native-tls support is particularly unfortunate since ureq is the most feature-rich blocking client, and with this, #29 and #267 squared away, the ecosystem could simply converge on it as opposed to the current fragmented state.

[algesten]

If we are to support it again, I think we can make the abstraction over it ourselves. The previous implementation was too much conditional code spread throughout the crate – that can be avoided.

[jsha]

Agreed. I think the above rationale (platform support) is a good reason to reintroduce native-tls. And hopefully this time around we can improve the abstraction, as you say.

[nviennot]

Hello, I concur with OpenSSL. I'm getting the following streaming upload rates:

• rustls: 590 MB/s
• OpenSSL: 800 MB/s

It seems that for maximum performance, OpenSSL is the way to go at the moment.

[dontlaugh]

Can we talk about the API we want here? I'm not familiar with the way things were before, and what made it difficult.

Most of the direct use of rustls is in the stream module. And most of that usage is conditionally-compiled away free functions, so it is kind of hard to picture how to shim native-tls in.

Presumably you want to keep the Agent API the same. It seems okay to let the user build their own TLS config with whatever underlying library they pick. They can figure it out. E.g. something like:

#[cfg(feature = "native-tls")]
pub(crate) struct TLSClientConfig(pub(crate) Arc<TLSClientConfigNative>);

#[cfg(feature = "native-tls")]
pub(crate) struct TLSClientConfigNative;  // hand-waving here. I don't know what we'll need :)

#[cfg(feature = "tls")]
#[derive(Clone)]
pub(crate) struct TLSClientConfig(pub(crate) Arc<rustls::ClientConfig>);

So, the action is over in stream.rs.

If you still like your current Stream type, we can put the native-tls type on the Inner, conditionally compiled.

pub(crate) struct Stream {
    inner: BufReader<Inner>,
}

#[allow(clippy::large_enum_variant)]
enum Inner {
    Http(TcpStream),
    #[cfg(feature = "tls")]
    Https(rustls::StreamOwned<rustls::ClientSession, TcpStream>),
    #[cfg(feature = "native-tls")]
    Https(native_tls::TlsStream<TcpStream>),   // new stuff
    Test(Box<dyn Read + Send + Sync>, Vec<u8>),
}

Both of the TLS libraries have these stream types, and they're both Read and Write.

Now, constructing these will both be totally different, probably. Especially with the certs and stuff. But I am not sure there's a nice abstraction to be had. Did anyone have anything in mind?

What kind of situation do you want to avoid this time, specifically?

[algesten]

Thanks for bringing this up.

I started hacking away at this the other day but didn't complete it.

I'm keen to not get compilations errors if tls and native-tls both are enabled. If both are enabled, I think I'd like there to be some config option in Agent to select which impl to use. And If just one is enabled, it will use that one.

That means the stream enum values would have different names Https vs HttpsNative or maybe a rename altogether.

This is up for discussion however.

[jsha]

@algesten: interesting! That's not something I would have thought of. What are the use cases you're thinking about? I do want to make sure it's possible to turn off native-tls at compile time, because we've gotten feedback that it's helpful to not need those libraries installed. But we can do that and also allow linking with both. For my part, what I disliked about the old way was that there were a large number of places throughout stream.rs where we needed to write substantively duplicate logic on multiple branches of a match. And also that it was easy to update things for one feature flag but mot the other, and realize it only when running the full test matrix. I think what I'd like is a trait that expresses what we need from a TLS stream: mainly read, write, and the ability to unwrap into a TcpStream. And then implementations of that trait for each of the two TLS libraries. I'm fine having users provide config options using the native config types of each library.

[algesten]

@jsha it's a bit of a "why not"?

We would definitely still have two feature flags tls and native-tls, and we can turn off either (and still default to only tls).

One problem with the previous native-tls was that any refactoring or feature I did with one feature flag, I constantly forgot to switch over to the other. If both can be enabled, we can just have all flags on while editing the code.

Also, if a project links multiple times to ureq through different dependencies, cargo would make the sum of feature flags on the top level. If this sum enables mutually exclusive flags, this will cause sadness.

[jsha]

Bringing some of the conversation from #389 (comment) and #389 (comment) over to the issue thread rather than the PR.

We have a couple of possible goals in supporting native-tls:

a) Allow ureq to work on platforms unsupported by rustls, like power9.
b) Allow applications and/or libraries to connect to hosts that use obsolete TLS versions or ciphers.

I think we're all pretty well agreed on (a), and we can probably do (a) without feature flags.

For (b), there's the question of how we want to do this. Let's imagine three crates:

1. aws-api: makes ureq requests solely to the AWS API, which supports modern TLS
2. zerbert-api: makes ureq requests solely to the (imaginary) Zerbert API, which only supports TLS 1.0.
3. html-scraper: makes ureq requests to arbitrary URLs on various hosts, some of which support modern TLS, and some of which don't.

If an application depends on aws-api and zerbert-api, we'd like aws-api to use rustls, and zerbert-api to use native-tls.

If an application depends only on aws-api, we'd like to not link native-tls at all.

If an application depends on html-scraper, we'd like to leave the choice up to the application, not the library. If the application wants to support obsolete TLS, it should be possible to configure things such that html-scraper will use native-tls for everything. If that's not explicitly configured, we should default to rustls for everything.

We can achieve the first two requirements by adding an option in AgentBuilder to provide an arbitrary HttpsConnector. Crates like zerbert-api would need to instantiate their own Agent and configure a native-tls backend for it.

For the third requirement: We could achieve this by saying that libraries that wrap ureq should offer a way for the application to provide an Agent for the library to use. That way the application can configure the desired TLS backend on that Agent. This has the added advantage that the application could share an Agent across multiple libraries, possibly improving connection reuse.

[jsha]

I've updated #391 (comment) with some of my latest thinking here.

[dontlaugh]

@algesten @jsha This isn't immediately actionable, but you should probably keep an eye on this issue

rustls/rustls#521

There is a chance that rustls itself will provide an abstraction for multiple crypto backends in the future.

[algesten]

This has landed in 2.4.0 that is in main will be released very soon.

[algesten]

Just to clarify. Enabling native-tls will not change the default TLS implementation for all of ureq (like it did in 1.x), but instead make it possible to configure on AgentBuilder.

The reason was explained here: #426 (comment)