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

Acceptor timeout config #4

Open
vasilakisfil opened this issue Jun 26, 2023 · 5 comments
Open

Acceptor timeout config #4

vasilakisfil opened this issue Jun 26, 2023 · 5 comments

Comments

@vasilakisfil
Copy link

vasilakisfil commented Jun 26, 2023

Is there a way to limit how much time the acceptor is going to wait for the TLS handshake to take place from the client side? If a malicious client never sends its TLS handshake request part, will this block forever?

If yes then probably the example should be updated to use tokio's timeout.

@rfarr
Copy link

rfarr commented Jul 20, 2023

@vasilakisfil This is how I've done it. The key is to not await on the accept() call but instead wrap that future in the tokio timeout.

let tls_config = rustls::ServerConfig::builder()
    .with_safe_defaults()
    .with_no_client_auth()
    .with_single_cert(certs, private_key)
    .unwrap();

let tls_acceptor = tokio_rustls::TlsAcceptor::from(tls_config);

// ...
// Inside the acceptor loop after you have a tcp connection already accepted
// ...

let tls_stream = tls_acceptor.accept(tcp_stream).map_err(|e| {
    std::io::Error::new(std::io::ErrorKind::Other, format!("Client TLS connect fail: {:?}", e))
});

// Wrap the accept future with a timeout to put an upper limit on how long a client can take to complete
// the TLS handshake and get to a TLS "connected" state
let tls_handshake_timeout_seconds: u8 = 5;
let tls_stream = tokio::time::timeout(tokio::time::Duration::from_secs(tls_handshake_timeout_seconds.into()), tls_stream).map_err(move |_e| {
  std::io::Error::new(std::io::ErrorKind::TimedOut, format!("Client TLS handshake timeout after {} seconds", tls_handshake_timeout_seconds))
 });
 
let tls_stream = match tls_stream.await {
    // Happy path
    Ok(tls_stream) => tls_stream,
    // Some kind of error
    Err(e) => ...
    // Timeout
    Ok(Err(e)) => ...
};

@vasilakisfil
Copy link
Author

Thanks, yes I have done the same thing. On event-driven architectures (aka channels), not having a timeout limit by default can lead to serious unexpected stalls in an application. Probably it's a good idea to add something on the documentation about it.

@nirbheek
Copy link

nirbheek commented Dec 11, 2023

This seems like a serious issue to me, the lack of a TLS timeout means that applications built on tokio-rustls are vulnerable out of the box to a DoS attack to exhaust FDs, like a very basic Slowloris attack but with TLS.

I think by default there should be an aggressive configurable timeout for completion of the TLS handshake.

You can do a DoS on the default rustls server example by just spawning tons of nc <host> <port> instances that will never, ever timeout.

@Congyuwang
Copy link

Congyuwang commented Jul 11, 2024

I think this is indeed quite needed! I find my server with a lot of ESTABLISHED connection with 0/0 bytes communication that hangs forever, even though I have a timeout layer in my Axum service. It turns out that these connections await at tls_acceptor.accept(stream).await.

nirbheek added a commit to terrylowry/gigroom-signalling that referenced this issue Sep 3, 2024
Otherwise we are vulnerable to a DoS attack to exhaust our FDs by
opening an unlimited number of TCP connections and never starting
a TLS handshake.

rustls/tokio-rustls#4 (comment)
@djc
Copy link
Member

djc commented Dec 11, 2024

(Make sure to also take care of the LazyConfigAcceptor when we tackle this.)

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

No branches or pull requests

5 participants