Add native TLS support with tokio-rustls

[juhaku]

This commit adds native TLS support using tokio-rustls. The feature is gated behind a feature flag and can be optionally enabled.

Since axum already had all the necessary connection handling logic in place but was missing TLS acceptor functionality, this PR adds that support by implementing a TlsListener wrapper for TcpListener that first accepts TCP connections and then performs TLS handshakes.

Example

Wrap TcpListener with TlsListener to allow axum to serve HTTPS connections.

let cert = CertificateDer::from_slice(&[0]);
let key = PrivateKeyDer::from_pem_slice(&[0]).unwrap();
let config = ServerConfig::builder()
    .with_no_client_auth()
    .with_single_cert(vec![cert], key).unwrap();

let tcp = TcpListener::bind(("0.0.0.0", 8443)).await.unwrap();
let tls_listener = TlsListener::new(tcp, config);
let app = Router::new().route("/", routing::get(|| async { "Hello" }));

let _ = axum::serve(tls_listener, app.into_make_service());

Motivation

Axum has all necessary plumbing in place for handling connections but lacks the ability to serve TLS connections directly. While there are multiple examples on repository how to achieve this, I feel it should be something that is part of the library for completeness and simplified usage.

With having this functionality in axum directly there is no need for users to duplicate the loop logic to their codebase just for few lines of code.

No goals

While this commit adds support for tokio-rustls uses of openssl was not considered, but could be something to consider in future.

Solution

In it's simplicity this functionality integrates to existing axum::serve functionality with one single line addition as shown below:

let tcp = TcpListener::bind(("0.0.0.0", 8443)).await.unwrap();
+let tls_listener = TlsListener::new(tcp, config);
let app = Router::new().route("/", routing::get(|| async { "Hello" }));

let _ = axum::serve(tls_listener, app.into_make_service());

[juhaku]

As far for tests, there are doc tests, but I did not yet write any unit tests for the change. I'd like some pointers for direction whether it is useful to test the server TLS somehow? Should set the server up using TLS, (self signed cert & key needed ), and then fire some requests to see whether they come through.

This comes with an example as well which can be found from examples folder.

[juhaku]

But I guess there is noway around this deny, since rustls has ISC license becuase of the webpki 🤦‍♂️

[juhaku]

However, should that be allowed? The ISC license is comparable to MIT and is non-copyleft permissive license.

Example of ISC license

Copyright (c) <year> <copyright holders>

Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.

THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

[jplatte]

We've always been intentionally avoiding the complexity that comes with supporting TLS, and I don't think anything has changed.

This can just be a third-party crate (just like axum-server is), no?

[juhaku]

Well, if that is something you decide so, though I did not notice any complex with this implementation. But if we were to support multiple different versions of rustls it might be complicated? However what this does is that it only optionally adds TLS acceptor to the loop that is already there thus the implementation is quite simple.

Though this could be a seprate crate. If so decided, I belive that implemeting listener type in external crate should work just fine also? Could be tried.

I somehow would imagine it to reside in under tokio organization if it is a separate crate. Any thoughts on this?

[mladedav]

Something similar was already attempted at least in #3400 and there is a closed issue about this in #2494.

---

By the way, you're running into the same problem as the first implementation initially, you're blocking accepting new TCP connections until the previous TLS handshake finishes. That's what makes the implementation simple but also a single client can make the server unavailable this way.

I don't think this needs to be in a tokio repository. For example axum-server is also 3rd party and I don't think there were any issues with that.

[juhaku]

By the way, you're running into the same problem as the first implementation initially, you're blocking accepting new TCP connections until the previous TLS handshake finishes. That's what makes the implementation simple but also a single client can make the server unavailable this way.

Okay, yeah I was wondering whether the handshake should be done withing tokio::spawn but didn't take that route because that would need a lot more changes than the simple listener implementation.

Nevertheless, if axum is not willing to invest in native TLS support then the most viable solution is for people hand rolling their own or using axum-server. That is, if the implementation was as simple as implementing a Listener it could be a separate crate. But since it needs changes to the handle connections loop, that cannot be done only with 3rd party crate.

So what it is, it's your call, should this be moved forward or not. E.g. for it to work, the the TLS listener should be able to pass the tls acceptor to the handle connections loop and within tokio::spawn it should try to do the handshake and then again wrap the tls stream with TokioIo. And yes this should be done conditionally to the case whether TSL is acually enabled.

By the way, you're running into the same problem as the first implementation initially, you're blocking accepting new TCP connections until the previous TLS handshake finishes. That's what makes the implementation simple but also a single client can make the server unavailable this way.

I was considering that it would perhaps be better for it to be sort of "official" axum extension etc. And that it cold be maintained withing tokio, instead of one do once, then it becomes abandonware kind of a thing.

[mladedav]

TLS can still be done by 3rd party crates, for example the last implementation in #3400 woul also work outside axum. It drives the handshake as part of the AsyncRead/AsyncWrite implemnetations. I also think there's some kind of integration of axum and tls-acceptor (maybe in their examples?) that does this by spawning a background task that handles the handshakes, but I'm not exactly sure.

I don't think we should do any kind of special casing for TLS in the connection loop. On the other hand, if we do it generally like #3484, that could work. Though we still don't know if that's really the way we'll go.

[jplatte]

Closing in favor of #3484 for now. Let's discuss native TLS support in a separate issue after that is merged (if it is merged, though it seems likely given it solves a couple more problems than just TLS termination).

[juhaku]

#3484 That is pretty neat indeed. But isn't that the "casing" exactly, what you have done there. Instead you wrap the whole connection instead of TlsAcceptor only. 🙂 With that connection builder the connection is itself initialized within the tokio::spawn thus evading the issue mentioned above.

I'm on board with general approach, if axum provides these out of the box gated with feature flag, nice, if done with separate crate fine then. It is good when it is pluggable to the app without hassle.

I'd generally avoid all sorts of monkey patching with separate background tasks and then juggling with channels to get the tls accepted stream into the handle connections loop. Sounds fairly unnecessarily complicated. Even if it could be hacked together in 3rd party crate, doesn't mean that it should be done so. IMHO the solution should be simple, robust, pluggable and done properly. Anyhow, I think that the #3484 is pointing towards right direction.