Harden guarded proxy access and process cleanup

Cargo.toml

@@ -7,6 +7,8 @@ description = "A GNOME download manager (GTK 4 / libadwaita, reqwest backend)"
 license = "GPL-3.0-only"
 
 [dependencies]
+# Encode the per-run guarded-proxy credential for HTTP Proxy-Authorization.
+base64 = "0.23.1"
 # Version features must match the system libs (GTK 4.22+ / Adw 1.9+, GNOME 51 runtime).
 gtk4 = { version = "0.11", features = ["v4_10", "v4_12", "v4_14", "v4_16", "v4_18", "v4_20", "v4_22"] }
 libadwaita = { version = "0.9", features = ["v1_4", "v1_5", "v1_6", "v1_7", "v1_8", "v1_9"] }

src/cookies.rs

@@ -277,9 +277,8 @@ async fn stage_cookie_file(
     // Both cookie sources are intentional here: `--cookies` is this export's
     // output file and `--cookies-from-browser` is what fills it, so this spawn
     // must not use the handoff-aware ytdlp_spawn_identity.
-    let mut identity_args =
-        crate::video_tools::ytdlp_identity_args(cookies_browser, None, page_url);
-    let guarded_proxy = match crate::video_spawn::guard_untrusted_args(&mut identity_args) {
+    let identity_args = crate::video_tools::ytdlp_identity_args(cookies_browser, None, page_url);
+    let guarded_proxy = match crate::video_spawn::guard_untrusted_args(&identity_args) {
         Ok(guarded) => guarded,
         Err(e) => {
             tracing::warn!(error = %e, "refusing cookie export without guarded extractor networking");

src/gallery_probe.rs

@@ -303,13 +303,7 @@ fn apply_guarded_proxy(
         ));
     }
     let proxy = crate::subprocess_proxy::ensure_started().map_err(GalleryError::probe)?;
-    cmd.arg("--proxy").arg(&proxy);
-    cmd.env("NO_PROXY", "")
-        .env("no_proxy", "")
-        .env("HTTP_PROXY", &proxy)
-        .env("http_proxy", &proxy)
-        .env("HTTPS_PROXY", &proxy)
-        .env("https_proxy", &proxy);
+    crate::video_spawn::apply_guarded_proxy_env(cmd, Some(&proxy));
     Ok(true)
 }
 
@@ -423,9 +417,6 @@ async fn run_to_completion(
         .spawn()
         .map_err(|e| GalleryError::install(format!("couldn't spawn gallery-dl: {e}")))?;
     let mut guard = ProcessGroupGuard::new(&child);
-    // Recorded before the wait so the drain-timeout branch can still signal
-    // the group after the reap disarms the guard.
-    let pgid = guard.pgid();
     let stdout = child
         .stdout
         .take()
@@ -502,26 +493,12 @@ async fn run_to_completion(
             )));
         }
         Err(_) => {
-            // EOF never came; the length caps already watch the bytes, so
-            // this is a pipe inherited by a surviving descendant. Abort the
-            // drains, then kill the group by the recorded PGID (the guard
-            // disarmed at reap) and fail the probe instead of hanging it.
-            //
-            // The reap is up to 60 s old (DRAIN_AFTER_EXIT_TIMEOUT), so the
-            // PGID may be free or recycled: this is a best-effort signal for
-            // a descendant still in the group, not a guarantee. One that
-            // setsid'd away is unreachable and survives (now without pipe
-            // readers).
+            // EOF never came: a descendant may have inherited a pipe after
+            // the direct child was reaped. Abort the drains and fail the probe,
+            // but do not signal the old PGID: it may have been recycled after
+            // the leader exited, so killpg could terminate unrelated work.
             out_abort.abort();
             err_abort.abort();
-            #[cfg(unix)]
-            if let Some(pgid) = pgid {
-                // SAFETY: constant signal number; ESRCH (already dead) is
-                // harmless.
-                unsafe {
-                    libc::killpg(pgid, libc::SIGKILL);
-                }
-            }
             return Err(GalleryError::probe(format!(
                 "{what} didn't close its pipes after exiting"
             )));
@@ -1133,7 +1110,7 @@ pub async fn count_gallery_files_with_proxy(
     if untrusted_handoff && proxy.is_some() {
         return None;
     }
-    let mut argv = crate::gallery_argv::count_argv_with_proxy(
+    let argv = crate::gallery_argv::count_argv_with_proxy(
         tool,
         url,
         cookies_file,
@@ -1143,9 +1120,8 @@ pub async fn count_gallery_files_with_proxy(
         proxy,
     );
     let guarded_proxy = if untrusted_handoff {
-        let proxy = crate::subprocess_proxy::ensure_started().ok()?;
-        crate::video_spawn::insert_guarded_proxy_arg(&mut argv, &proxy).ok()?;
-        Some(proxy)
+        crate::video_spawn::validate_guarded_proxy_args(&argv).ok()?;
+        Some(crate::subprocess_proxy::ensure_started().ok()?)
     } else {
         None
     };

src/gallery_runner.rs

@@ -129,7 +129,7 @@ pub async fn run_gallery_download_with_proxy(
             "Cannot safely use a configured proxy for an untrusted browser handoff".to_string(),
         ));
     }
-    let mut argv = download_argv_with_proxy(
+    let argv = download_argv_with_proxy(
         tool,
         &job.page_url,
         &job.dest_dir,
@@ -141,10 +141,8 @@ pub async fn run_gallery_download_with_proxy(
         proxy,
     );
     let guarded_proxy = if untrusted_handoff {
-        let proxy = crate::subprocess_proxy::ensure_started().map_err(GalleryError::probe)?;
-        crate::video_spawn::insert_guarded_proxy_arg(&mut argv, &proxy)
-            .map_err(GalleryError::probe)?;
-        Some(proxy)
+        crate::video_spawn::validate_guarded_proxy_args(&argv).map_err(GalleryError::probe)?;
+        Some(crate::subprocess_proxy::ensure_started().map_err(GalleryError::probe)?)
     } else {
         None
     };

src/subprocess_proxy.rs

@@ -7,6 +7,8 @@
 //! HTTP proxy requests. It is only enabled for browser-handoff hosts whose
 //! public-resolution policy is active.
 
+use base64::{Engine as _, engine::general_purpose::STANDARD};
+use std::fmt::Write as _;
 use std::io;
 use std::net::{SocketAddr, TcpListener as StdTcpListener};
 use std::pin::Pin;
@@ -18,7 +20,64 @@ use tokio::io::{AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _, Re
 use tokio::net::{TcpListener, TcpStream};
 use tokio::sync::Semaphore;
 
-static PROXY_ADDR: OnceLock<SocketAddr> = OnceLock::new();
+static PROXY_STATE: OnceLock<ProxyState> = OnceLock::new();
+
+#[derive(Clone)]
+struct ProxyState {
+    addr: SocketAddr,
+    token: String,
+    expected_authorization: String,
+}
+
+impl ProxyState {
+    fn proxy_url(&self) -> String {
+        format!("http://grab:{}@{}", self.token, self.addr)
+    }
+}
+
+fn proxy_authorization_header(token: &str) -> String {
+    format!("Basic {}", STANDARD.encode(format!("grab:{token}")))
+}
+
+fn random_proxy_token() -> Result<String, String> {
+    let mut bytes = [0u8; 32];
+    getrandom::fill(&mut bytes)
+        .map_err(|error| format!("couldn't generate guarded proxy token: {error}"))?;
+    let mut token = String::with_capacity(bytes.len() * 2);
+    for byte in bytes {
+        write!(&mut token, "{byte:02x}").expect("writing to a String cannot fail");
+    }
+    Ok(token)
+}
+
+fn constant_time_equal(left: &[u8], right: &[u8]) -> bool {
+    if left.len() != right.len() {
+        return false;
+    }
+    let mut difference = 0u8;
+    for (&a, &b) in left.iter().zip(right) {
+        difference |= a ^ b;
+    }
+    difference == 0
+}
+
+/// Match exactly one Proxy-Authorization header; duplicated headers are rejected.
+fn proxy_authorization_matches(header: &[u8], expected: &str) -> bool {
+    let text = String::from_utf8_lossy(header);
+    let mut provided = None;
+    for line in text.split("\r\n").skip(1) {
+        let Some((name, value)) = line.split_once(':') else {
+            continue;
+        };
+        if name.trim().eq_ignore_ascii_case("proxy-authorization") {
+            if provided.is_some() {
+                return false;
+            }
+            provided = Some(value.trim());
+        }
+    }
+    provided.is_some_and(|value| constant_time_equal(value.as_bytes(), expected.as_bytes()))
+}
 const MAX_HEADER_BYTES: usize = 64 * 1024;
 const HEADER_TIMEOUT: Duration = Duration::from_secs(10);
 /// Bound DNS resolution and sequential public-address connection attempts too:
@@ -148,38 +207,49 @@ pub(crate) fn validate_proxy_compatibility() -> Result<(), String> {
     Ok(())
 }
 
-/// Start the proxy once and return its HTTP proxy URL.
+/// Start the proxy once and return an authentication-bearing URL for the
+/// child environment. The random credential is created once per Grab process.
 pub(crate) fn ensure_started() -> Result<String, String> {
     validate_proxy_compatibility()?;
-    if let Some(addr) = PROXY_ADDR.get() {
-        return Ok(format!("http://{addr}"));
+    if let Some(state) = PROXY_STATE.get() {
+        return Ok(state.proxy_url());
     }
     let listener = StdTcpListener::bind(("127.0.0.1", 0))
         .map_err(|e| format!("couldn't start the guarded extractor proxy: {e}"))?;
-    start_proxy_listener(listener, &PROXY_ADDR)
+    start_proxy_listener(listener, &PROXY_STATE, random_proxy_token()?)
 }
 
 /// Register and serve a listener under Grab's process-lifetime runtime. The
 /// state parameter lets tests exercise isolated initialization state too.
 fn start_proxy_listener(
     listener: StdTcpListener,
-    state: &OnceLock<SocketAddr>,
+    state: &OnceLock<ProxyState>,
+    token: String,
 ) -> Result<String, String> {
     listener
         .set_nonblocking(true)
         .map_err(|e| format!("couldn't configure the guarded extractor proxy: {e}"))?;
     let addr = listener
         .local_addr()
         .map_err(|e| format!("couldn't read guarded proxy address: {e}"))?;
+    let expected_authorization = proxy_authorization_header(&token);
     let runtime = crate::runtime::tokio_rt();
     let listener = {
         let _entered = runtime.enter();
         TcpListener::from_std(listener)
             .map_err(|e| format!("couldn't start the guarded extractor proxy: {e}"))?
     };
 
-    match state.set(addr) {
+    match state.set(ProxyState {
+        addr,
+        token,
+        expected_authorization,
+    }) {
         Ok(()) => {
+            let expected_authorization = state
+                .get()
+                .map(|state| state.expected_authorization.clone())
+                .ok_or_else(|| "guarded proxy state was not initialized".to_string())?;
             runtime.spawn(async move {
                 loop {
                     let permit = match Arc::clone(&PROXY_CONNECTIONS).acquire_owned().await {
@@ -188,9 +258,12 @@ fn start_proxy_listener(
                     };
                     match listener.accept().await {
                         Ok((stream, _peer)) => {
+                            let expected_authorization = expected_authorization.clone();
                             tokio::spawn(async move {
                                 let _permit = permit;
-                                if let Err(e) = serve_connection(stream).await {
+                                if let Err(e) =
+                                    serve_connection(stream, &expected_authorization).await
+                                {
                                     tracing::debug!(error = %e, "guarded extractor proxy request failed");
                                 }
                             });
@@ -203,12 +276,15 @@ fn start_proxy_listener(
                     }
                 }
             });
-            Ok(format!("http://{addr}"))
+            state
+                .get()
+                .map(ProxyState::proxy_url)
+                .ok_or_else(|| "guarded proxy state was not initialized".to_string())
         }
-        Err(_) => Ok(format!(
-            "http://{}",
-            state.get().expect("proxy address initialized")
-        )),
+        Err(_) => state
+            .get()
+            .map(ProxyState::proxy_url)
+            .ok_or_else(|| "guarded proxy state was not initialized".to_string()),
     }
 }
 
@@ -360,7 +436,18 @@ async fn reply_status(stream: &mut TcpStream, code: u16, reason: &str) {
     let _ = stream.write_all(response.as_bytes()).await;
 }
 
-async fn serve_connection(mut client: TcpStream) -> std::io::Result<()> {
+async fn reply_proxy_auth_required(stream: &mut TcpStream) -> io::Result<()> {
+    stream
+        .write_all(
+            b"HTTP/1.1 407 Proxy Authentication Required\r\nProxy-Authenticate: Basic realm=\"Grab guarded extractor proxy\"\r\nConnection: close\r\nContent-Length: 0\r\n\r\n",
+        )
+        .await
+}
+
+async fn serve_connection(
+    mut client: TcpStream,
+    expected_authorization: &str,
+) -> std::io::Result<()> {
     let (header, trailing) = match read_header_with_timeout(&mut client, HEADER_TIMEOUT).await {
         Ok(parts) => parts,
         Err(error) => {
@@ -372,6 +459,10 @@ async fn serve_connection(mut client: TcpStream) -> std::io::Result<()> {
             return Err(error);
         }
     };
+    if !proxy_authorization_matches(&header, expected_authorization) {
+        reply_proxy_auth_required(&mut client).await?;
+        return Ok(());
+    }
     let text = String::from_utf8_lossy(&header);
     let mut lines = text.split("\r\n");
     let Some(request_line) = lines.next() else {
@@ -485,17 +576,22 @@ mod tests {
             caller_runtime
                 .block_on(async {
                     let listener = StdTcpListener::bind(("127.0.0.1", 0)).unwrap();
-                    start_proxy_listener(listener, &state)
+                    start_proxy_listener(listener, &state, "test-token".to_string())
                 })
                 .expect("start guarded proxy")
         })
         .join()
         .expect("caller thread");
-        let address: SocketAddr = proxy_url
-            .strip_prefix("http://")
-            .expect("proxy URL scheme")
-            .parse()
-            .expect("proxy address");
+        let parsed = url::Url::parse(&proxy_url).expect("proxy URL");
+        assert_eq!(parsed.username(), "grab");
+        assert_eq!(parsed.password(), Some("test-token"));
+        let address: SocketAddr = format!(
+            "{}:{}",
+            parsed.host_str().expect("proxy host"),
+            parsed.port().expect("proxy port")
+        )
+        .parse()
+        .expect("proxy address");
 
         let response = crate::runtime::tokio_rt()
             .block_on(async {
@@ -514,8 +610,8 @@ mod tests {
             .expect("proxy connection and request");
 
         assert!(
-            String::from_utf8_lossy(&response).starts_with("HTTP/1.1 403"),
-            "proxy should survive caller runtime shutdown: {}",
+            String::from_utf8_lossy(&response).starts_with("HTTP/1.1 407"),
+            "proxy should require authentication and survive caller runtime shutdown: {}",
             String::from_utf8_lossy(&response)
         );
     }
@@ -613,16 +709,20 @@ mod tests {
     async fn proxy_rejects_private_connect_destinations() {
         let listener = TcpListener::bind(("127.0.0.1", 0)).await.unwrap();
         let proxy_addr = listener.local_addr().unwrap();
+        let expected_authorization = proxy_authorization_header("test-token");
         let server = tokio::spawn(async move {
             let (stream, _) = listener.accept().await.unwrap();
-            serve_connection(stream).await.unwrap();
+            serve_connection(stream, &expected_authorization)
+                .await
+                .unwrap();
         });
 
         let mut client = TcpStream::connect(proxy_addr).await.unwrap();
-        client
-            .write_all(b"CONNECT 127.0.0.1:9 HTTP/1.1\r\nHost: 127.0.0.1:9\r\n\r\n")
-            .await
-            .unwrap();
+        let request = format!(
+            "CONNECT 127.0.0.1:9 HTTP/1.1\r\nHost: 127.0.0.1:9\r\nProxy-Authorization: {}\r\n\r\n",
+            proxy_authorization_header("test-token")
+        );
+        client.write_all(request.as_bytes()).await.unwrap();
         let mut response = Vec::new();
         client.read_to_end(&mut response).await.unwrap();
         assert!(