From e3190c888a203b5d2459e8b7bb71ab42f38c5821 Mon Sep 17 00:00:00 2001
From: Nirbheek Chauhan <nirbheek@centricular.com>
Date: Wed, 20 Dec 2023 18:01:11 +0530
Subject: [PATCH] webrtc/signalling: Fix potential hang and FD leak

If a peer connects via TCP and never initiates TLS, then the server
will get stuck in the accept loop. Spawn a task when accepting a TLS
connection, and timeout if it doesn't complete in 5 seconds.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/1456>
---
 net/webrtc/signalling/src/bin/server.rs | 28 +++++++++++++++++--------
 net/webrtc/signalling/src/server/mod.rs |  4 ++++
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/net/webrtc/signalling/src/bin/server.rs b/net/webrtc/signalling/src/bin/server.rs
index 52219da9..8630ae1f 100644
--- a/net/webrtc/signalling/src/bin/server.rs
+++ b/net/webrtc/signalling/src/bin/server.rs
@@ -3,15 +3,18 @@
 use async_std::task;
 use clap::Parser;
 use gst_plugin_webrtc_signalling::handlers::Handler;
-use gst_plugin_webrtc_signalling::server::Server;
+use gst_plugin_webrtc_signalling::server::{Server, ServerError};
 use tracing_subscriber::prelude::*;
 
 use anyhow::Error;
 use async_native_tls::TlsAcceptor;
 use async_std::fs::File as AsyncFile;
 use async_std::net::TcpListener;
+use std::time::Duration;
 use tracing::{info, warn};
 
+const TLS_HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(5);
+
 #[derive(Parser, Debug)]
 #[clap(about, version, author)]
 /// Program arguments
@@ -84,15 +87,22 @@ fn main() -> Result<(), Error> {
 
             info!("Accepting connection from {}", address);
 
-            if let Some(ref acceptor) = acceptor {
-                let stream = match acceptor.accept(stream).await {
-                    Ok(stream) => stream,
-                    Err(err) => {
-                        warn!("Failed to accept TLS connection from {}: {}", address, err);
-                        continue;
+            if let Some(acceptor) = acceptor.clone() {
+                task::spawn(async move {
+                    match async_std::future::timeout(TLS_HANDSHAKE_TIMEOUT, acceptor.accept(stream))
+                        .await
+                    {
+                        Ok(Ok(stream)) => server_clone.accept_async(stream).await,
+                        Ok(Err(err)) => {
+                            warn!("Failed to accept TLS connection from {}: {}", address, err);
+                            Err(ServerError::TLSHandshake(err))
+                        }
+                        Err(err) => {
+                            warn!("TLS connection timed out {} after {}", address, err);
+                            Err(ServerError::TLSHandshakeTimeout(err))
+                        }
                     }
-                };
-                task::spawn(async move { server_clone.accept_async(stream).await });
+                });
             } else {
                 task::spawn(async move { server_clone.accept_async(stream).await });
             }
diff --git a/net/webrtc/signalling/src/server/mod.rs b/net/webrtc/signalling/src/server/mod.rs
index eaafff62..f7d42c6e 100644
--- a/net/webrtc/signalling/src/server/mod.rs
+++ b/net/webrtc/signalling/src/server/mod.rs
@@ -32,6 +32,10 @@ pub struct Server {
 pub enum ServerError {
     #[error("error during handshake {0}")]
     Handshake(#[from] async_tungstenite::tungstenite::Error),
+    #[error("error during TLS handshake {0}")]
+    TLSHandshake(#[from] async_native_tls::Error),
+    #[error("timeout during TLS handshake {0}")]
+    TLSHandshakeTimeout(#[from] async_std::future::TimeoutError),
 }
 
 impl Server {