httpd: extract reusable server code

src/bin/varlink-httpd/main.rs

@@ -319,23 +319,6 @@ async fn get_varlink_connection(
     Ok(connection)
 }
 
-/// Accept a TCP connection, configure socket options, and retry on transient errors.
-async fn accept_and_configure(
-    listener: &TcpListener,
-) -> (tokio::net::TcpStream, std::net::SocketAddr) {
-    loop {
-        match listener.accept().await {
-            Ok((stream, addr)) => {
-                if let Err(e) = varlink_http_bridge::set_tcp_keepalive_and_nodelay(&stream) {
-                    warn!("on accept from {addr}: {e:#}");
-                }
-                return (stream, addr);
-            }
-            Err(e) => warn!("TCP accept failed: {e}"),
-        }
-    }
-}
-
 fn format_x509_subject(cert: &openssl::x509::X509Ref) -> String {
     cert.subject_name()
         .entries()
@@ -363,20 +346,14 @@ async fn tls_accept<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin>(
     config: &TlsConfig,
     stream: S,
 ) -> anyhow::Result<tokio_openssl::SslStream<S>> {
-    let mut ssl = openssl::ssl::Ssl::new(config.acceptor.context()).context("SSL context error")?;
-    if let Some(trust) = &config.client_trust {
-        // Per handshake so a CA that changed on disk applies to new connections
-        // without restarting the listener.
-        ssl.set_verify_cert_store(trust.store()?)
-            .context("installing client CA store")?;
-    }
-    let mut tls_stream =
-        tokio_openssl::SslStream::new(ssl, stream).context("SSL stream creation failed")?;
-    std::pin::Pin::new(&mut tls_stream)
-        .accept()
-        .await
-        .context("TLS handshake failed")?;
-    Ok(tls_stream)
+    // Per handshake so a CA that changed on disk applies to new connections
+    // without restarting the listener.
+    let client_store = config
+        .client_trust
+        .as_ref()
+        .map(|trust| trust.store())
+        .transpose()?;
+    varlink_http_bridge::listen::tls_accept(&config.acceptor, client_store, stream).await
 }
 
 /// TLS wrapper for any `axum::serve::Listener`. Performs handshakes concurrently
@@ -404,13 +381,18 @@ where
                 let tx = tx.clone();
                 let config = config.clone();
                 tokio::spawn(async move {
-                    match tls_accept(&config, stream).await {
-                        Ok(tls_stream) => {
+                    let handshake = tls_accept(&config, stream);
+                    match tokio::time::timeout(config.handshake_timeout, handshake).await {
+                        Ok(Ok(tls_stream)) => {
                             if tx.send((tls_stream, addr)).await.is_err() {
                                 warn!("TLS listener receiver dropped");
                             }
                         }
-                        Err(e) => warn!("TLS handshake from {addr}: {e:#}"),
+                        Ok(Err(e)) => warn!("TLS handshake from {addr}: {e:#}"),
+                        Err(_) => warn!(
+                            "TLS handshake from {addr}: nothing in {:?}, dropping the connection",
+                            config.handshake_timeout
+                        ),
                     }
                 });
             }
@@ -453,7 +435,7 @@ impl axum::serve::Listener for PlainListener {
     type Addr = std::net::SocketAddr;
 
     async fn accept(&mut self) -> (Self::Io, Self::Addr) {
-        accept_and_configure(&self.inner).await
+        varlink_http_bridge::listen::accept_and_configure(&self.inner).await
     }
 
     fn local_addr(&self) -> std::io::Result<Self::Addr> {
@@ -617,33 +599,33 @@ struct TlsConfig {
     acceptor: openssl::ssl::SslAcceptor,
     /// `None` when mTLS is off, so no client certificate is requested.
     client_trust: Option<Arc<ClientTrust>>,
+    /// How long a client gets to complete its handshake before the
+    /// connection is dropped.
+    handshake_timeout: Duration,
 }
 
+const TLS_HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(10);
+
 fn load_tls_config(
     cert_path: &str,
     key_path: &str,
     client_ca_path: Option<&str>,
     require_client_cert: bool,
 ) -> anyhow::Result<TlsConfig> {
-    use openssl::ssl::{SslAcceptor, SslFiletype, SslMethod, SslVerifyMode};
-
-    let mut builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?;
-    // mozilla_modern_v5 allows TLS 1.2, but we need 1.3 for channel binding
-    // (export_keying_material requires TLS 1.3).
-    builder.set_min_proto_version(Some(openssl::ssl::SslVersion::TLS1_3))?;
-    builder.set_certificate_chain_file(cert_path)?;
-    builder.set_private_key_file(key_path, SslFiletype::PEM)?;
-    builder.check_private_key()?;
+    let mut builder = varlink_http_bridge::listen::tls_acceptor_builder(cert_path, key_path)?;
 
     // The CAs store is configured per-handshake.
     let client_trust = require_client_cert.then(|| {
-        builder.set_verify(SslVerifyMode::PEER | SslVerifyMode::FAIL_IF_NO_PEER_CERT);
+        builder.set_verify(
+            openssl::ssl::SslVerifyMode::PEER | openssl::ssl::SslVerifyMode::FAIL_IF_NO_PEER_CERT,
+        );
         Arc::new(ClientTrust::new(client_ca_path))
     });
 
     Ok(TlsConfig {
         acceptor: builder.build(),
         client_trust,
+        handshake_timeout: TLS_HANDSHAKE_TIMEOUT,
     })
 }
 

src/bin/varlink-httpd/tests.rs

@@ -3493,3 +3493,39 @@ async fn test_integration_openapi_requires_more_flag() {
         "CleanPool should NOT have json response (requires 'more')"
     );
 }
+
+/// A client that connects and then says nothing must not hold the
+/// handshake task and its fd until its TCP dies.
+#[test_with::executable(openssl)]
+#[tokio::test]
+async fn test_tls_handshake_timeout_drops_a_silent_client() {
+    use axum::serve::Listener as _;
+    use tokio::io::AsyncReadExt as _;
+
+    let pki = make_test_pki();
+    let mut tls = load_tls_config(
+        pki.server_cert_path.to_str().unwrap(),
+        pki.server_key_path.to_str().unwrap(),
+        None,
+        false,
+    )
+    .unwrap();
+    tls.handshake_timeout = Duration::from_millis(100);
+
+    let inner = TcpListener::bind("127.0.0.1:0").await.unwrap();
+    let addr = inner.local_addr().unwrap();
+    let mut listener = AsyncTlsListener::new(PlainListener { inner }, tls).unwrap();
+
+    let mut client = tokio::net::TcpStream::connect(addr).await.unwrap();
+    let mut buf = [0u8; 1];
+    let read = tokio::time::timeout(Duration::from_secs(5), client.read(&mut buf))
+        .await
+        .expect("the connection must be dropped on a silent client");
+    assert_eq!(read.unwrap(), 0);
+    assert!(
+        tokio::time::timeout(Duration::from_millis(100), listener.accept())
+            .await
+            .is_err(),
+        "nothing must have been handed to axum"
+    );
+}

src/lib.rs

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: LGPL-2.1-or-later
 
+pub mod listen;
 #[cfg(feature = "sshauth")]
 pub mod sshauth_token;
 pub mod sysconf;

src/listen.rs

@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+//! Listening helpers for the server code.
+
+/// Accept a TCP connection and configure socket options. A failed
+/// accept (a client gone between SYN and accept, no file descriptors
+/// left) or setsockopt must never take the whole listener down.
+pub async fn accept_and_configure(
+    listener: &tokio::net::TcpListener,
+) -> (tokio::net::TcpStream, std::net::SocketAddr) {
+    use log::warn;
+    use rustix::io::Errno;
+
+    const RETRY_DELAY: std::time::Duration = std::time::Duration::from_millis(100);
+
+    loop {
+        match listener.accept().await {
+            Ok((stream, addr)) => {
+                if let Err(e) = crate::set_tcp_keepalive_and_nodelay(&stream) {
+                    warn!("on accept from {addr}: {e:#}");
+                }
+                return (stream, addr);
+            }
+            Err(e) => {
+                // out of file descriptors: tell the admin what to do
+                let hint = match Errno::from_io_error(&e) {
+                    Some(Errno::MFILE) => ", increase LimitNOFILE= in the systemd unit file",
+                    Some(Errno::NFILE) => {
+                        ", the system-wide fs.file-max is exhausted (check fs.file-max)"
+                    }
+                    _ => "",
+                };
+                warn!(
+                    "TCP accept failed: {e}; retrying in {}ms{hint}",
+                    RETRY_DELAY.as_millis()
+                );
+                // nothing we can do here but wait and hope resource exhaustion gets better
+                tokio::time::sleep(RETRY_DELAY).await;
+            }
+        }
+    }
+}
+
+/// Returns a SslAcceptorBuilder from the given {cert,key}_path with
+/// a minimium TLS1.3 version requirement for TLS channel binding.
+///
+/// # Errors
+/// Returns an error if the certificate or key cannot be loaded or do
+/// not match.
+pub fn tls_acceptor_builder(
+    cert_path: &str,
+    key_path: &str,
+) -> anyhow::Result<openssl::ssl::SslAcceptorBuilder> {
+    use openssl::ssl::{SslAcceptor, SslFiletype, SslMethod};
+
+    let mut builder = SslAcceptor::mozilla_modern_v5(SslMethod::tls_server())?;
+    // mozilla_modern_v5 allows TLS 1.2, but we need 1.3 for channel binding
+    // (export_keying_material requires TLS 1.3).
+    builder.set_min_proto_version(Some(openssl::ssl::SslVersion::TLS1_3))?;
+    builder.set_certificate_chain_file(cert_path)?;
+    builder.set_private_key_file(key_path, SslFiletype::PEM)?;
+    builder.check_private_key()?;
+    Ok(builder)
+}
+
+/// Perform a TLS handshake on an already-accepted
+/// stream. `client_store` holds the CAs a client certificate is
+/// verified against. This runs per handshake the CAs can be rotated
+/// without rebuilding the acceptor.
+///
+/// # Errors
+/// Returns an error if the handshake fails.
+pub async fn tls_accept<S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin>(
+    acceptor: &openssl::ssl::SslAcceptor,
+    client_store: Option<openssl::x509::store::X509Store>,
+    stream: S,
+) -> anyhow::Result<tokio_openssl::SslStream<S>> {
+    use anyhow::Context;
+    let mut ssl = openssl::ssl::Ssl::new(acceptor.context()).context("SSL context error")?;
+    if let Some(store) = client_store {
+        ssl.set_verify_cert_store(store)
+            .context("installing client CA store")?;
+    }
+    let mut tls_stream =
+        tokio_openssl::SslStream::new(ssl, stream).context("SSL stream creation failed")?;
+    std::pin::Pin::new(&mut tls_stream)
+        .accept()
+        .await
+        .context("TLS handshake failed")?;
+    Ok(tls_stream)
+}