Handle HTTP redirects correctly in masque-server

2 files changed, 53 insertions, 45 deletions

diff --git a/mullvad-masque-proxy/src/client/mod.rs b/mullvad-masque-proxy/src/client/mod.rs
index 26a069bc81..33dcb0e397 100644
--- a/mullvad-masque-proxy/src/client/mod.rs
+++ b/mullvad-masque-proxy/src/client/mod.rs
@@ -346,6 +346,7 @@ impl Client {
                 ))
                 .await
             }
+
             status => Err(Error::UnexpectedStatus(status)),
         }
     }
diff --git a/mullvad-masque-proxy/src/server/mod.rs b/mullvad-masque-proxy/src/server/mod.rs
index 60e8db010b..4f160eaea6 100644
--- a/mullvad-masque-proxy/src/server/mod.rs
+++ b/mullvad-masque-proxy/src/server/mod.rs
@@ -14,7 +14,7 @@ use h3::{
     server::{self, Connection, RequestStream},
 };
 use h3_datagram::{datagram::Datagram, datagram_traits::HandleDatagramsExt};
-use http::{Request, StatusCode, Uri};
+use http::{StatusCode, Uri};
 use quinn::{crypto::rustls::QuicServerConfig, Endpoint, Incoming};
 use tokio::{net::UdpSocket, select, sync::mpsc, task};
 
@@ -121,54 +121,49 @@ impl Server {
     }
 
     async fn handle_incoming_connection(connection: Incoming, server_params: Arc<ServerParams>) {
-        match connection.await {
-            Ok(conn) => {
-                log::debug!("new connection established");
-
-                let quinn_conn = conn.clone();
+        let conn = match connection.await {
+            Ok(conn) => conn,
+            Err(err) => {
+                log::error!("accepting connection failed: {:?}", err);
+                return;
+            }
+        };
 
-                let Ok(mut connection) = server::builder()
-                    .enable_datagram(true)
-                    .build(h3_quinn::Connection::new(conn))
-                    .await
-                else {
-                    log::error!("Failed to construct a new H3 server connection");
-                    return;
-                };
+        log::debug!("new connection established");
 
-                match connection.accept().await {
-                    Ok(Some((req, stream))) => {
-                        tokio::spawn(Self::handle_proxy_request(
-                            connection,
-                            quinn_conn,
-                            req,
-                            stream,
-                            server_params,
-                        ));
-                    }
+        let quinn_conn = conn.clone();
 
-                    // indicating no more streams to be received
-                    Ok(None) => {}
+        let Ok(connection) = server::builder()
+            .enable_datagram(true)
+            .build(h3_quinn::Connection::new(conn))
+            .await
+        else {
+            log::error!("Failed to construct a new H3 server connection");
+            return;
+        };
 
-                    Err(err) => {
-                        log::error!("error on accept {}", err);
-                    }
-                }
-            }
-            Err(err) => {
-                log::error!("accepting connection failed: {:?}", err);
-            }
-        }
+        Self::accept_proxy_request(quinn_conn, connection, server_params).await;
     }
 
-    async fn handle_proxy_request<T: BidiStream<Bytes>>(
-        connection: Connection<h3_quinn::Connection, Bytes>,
-        quinn_conn: quinn::Connection,
-        request: Request<()>,
-        mut stream: RequestStream<T, Bytes>,
+    /// Accept an HTTP request and try to handle it as a proxy request.
+    async fn accept_proxy_request(
+        quic_conn: quinn::Connection,
+        mut http_conn: Connection<h3_quinn::Connection, Bytes>,
         server_params: Arc<ServerParams>,
     ) {
-        let proxy_uri = match ProxyUri::try_from(request.uri()) {
+        let (http_request, mut stream) = match http_conn.accept().await {
+            Ok(Some((req, stream))) => (req, stream),
+
+            // indicating no more streams to be received
+            Ok(None) => return,
+
+            Err(err) => {
+                log::error!("error on accept {}", err);
+                return;
+            }
+        };
+
+        let proxy_uri = match ProxyUri::try_from(http_request.uri()) {
             Ok(proxy_uri) => proxy_uri,
             Err(e) => {
                 log::debug!("Bad proxy URI: {e}");
@@ -182,7 +177,19 @@ impl Server {
                     hostname: hostname.to_string(),
                     ..proxy_uri
                 };
-                return handle_invalid_hostname(stream, valid_uri).await;
+
+                respond_with_redirect(stream, valid_uri).await;
+
+                // NOTE: Recursing like this makes us vulnerable to DoS if the client keeps
+                // sending the wrong hostname. This is fine since this is just an example server.
+                Box::pin(Self::accept_proxy_request(
+                    quic_conn,
+                    http_conn,
+                    server_params,
+                ))
+                .await;
+
+                return;
             }
         }
 
@@ -212,10 +219,10 @@ impl Server {
         let (send_tx, send_rx) = mpsc::channel(MAX_INFLIGHT_PACKETS);
 
         let mut connection_task =
-            task::spawn(connection_task(stream_id, connection, send_rx, client_tx));
+            task::spawn(connection_task(stream_id, http_conn, send_rx, client_tx));
         let mut proxy_rx_task = task::spawn(proxy_rx_task(
             stream_id,
-            quinn_conn,
+            quic_conn,
             proxy_uri.target_addr,
             server_params.mtu,
             Arc::clone(&udp_socket),
@@ -398,7 +405,7 @@ async fn handle_failed_socket<T: BidiStream<Bytes>>(mut stream: RequestStream<T,
     let _ = stream.send_response(response).await;
 }
 
-async fn handle_invalid_hostname<T: BidiStream<Bytes>>(
+async fn respond_with_redirect<T: BidiStream<Bytes>>(
     mut stream: RequestStream<T, Bytes>,
     valid_uri: ProxyUri,
 ) {