From c747a57b7e605e1d0d7e1fada0d8dc0b1d3b8235 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 19 May 2025 17:19:40 -0700 Subject: [PATCH] Revert "client: Add support for HTTP/HTTPS proxy" (#30979) Reverts zed-industries/zed#30812 This PR broke nightly builds on linux by adding an OpenSSL dependency to the `remote_server` binary, which failed to link when building against musl. Release Notes: - N/A --- Cargo.lock | 3 - crates/client/Cargo.toml | 3 - crates/client/src/client.rs | 6 +- crates/client/src/proxy.rs | 66 ------- crates/client/src/proxy/http_proxy.rs | 171 ------------------ .../src/{proxy/socks_proxy.rs => socks.rs} | 129 ++++++++----- 6 files changed, 90 insertions(+), 288 deletions(-) delete mode 100644 crates/client/src/proxy.rs delete mode 100644 crates/client/src/proxy/http_proxy.rs rename crates/client/src/{proxy/socks_proxy.rs => socks.rs} (50%) diff --git a/Cargo.lock b/Cargo.lock index 09f58daabd4003b450d16de318c1adeb0a8a39c1..62c65bc5a58f98d8e6148e30d0b1128473c26826 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2814,7 +2814,6 @@ dependencies = [ "anyhow", "async-recursion 0.3.2", "async-tungstenite", - "base64 0.22.1", "chrono", "clock", "cocoa 0.26.0", @@ -2826,7 +2825,6 @@ dependencies = [ "gpui_tokio", "http_client", "http_client_tls", - "httparse", "log", "parking_lot", "paths", @@ -2847,7 +2845,6 @@ dependencies = [ "time", "tiny_http", "tokio", - "tokio-native-tls", "tokio-socks", "url", "util", diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index 70936b09fb451f961718ce94f4790942d75992e9..1ebea995df33407b3001b61b1a26967c11c43ed5 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -19,7 +19,6 @@ test-support = ["clock/test-support", "collections/test-support", "gpui/test-sup anyhow.workspace = true async-recursion = "0.3" async-tungstenite = { workspace = true, features = ["tokio", "tokio-rustls-manual-roots"] } -base64.workspace = true chrono = { workspace = true, features = ["serde"] } clock.workspace = true collections.workspace = true @@ -30,7 +29,6 @@ gpui.workspace = true gpui_tokio.workspace = true http_client.workspace = true http_client_tls.workspace = true -httparse = "1.10" log.workspace = true paths.workspace = true parking_lot.workspace = true @@ -49,7 +47,6 @@ text.workspace = true thiserror.workspace = true time.workspace = true tiny_http = "0.8" -tokio-native-tls = "0.3" tokio-socks = { version = "0.5.2", default-features = false, features = ["futures-io"] } url.workspace = true util.workspace = true diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index abd3bc613a3a3c59d7ee7e344d677f41d88fd600..55e81bbccf6c63b8aa3fe7d81eaefac0869179fe 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -1,7 +1,7 @@ #[cfg(any(test, feature = "test-support"))] pub mod test; -mod proxy; +mod socks; pub mod telemetry; pub mod user; pub mod zed_urls; @@ -24,13 +24,13 @@ use gpui::{App, AsyncApp, Entity, Global, Task, WeakEntity, actions}; use http_client::{AsyncBody, HttpClient, HttpClientWithUrl}; use parking_lot::RwLock; use postage::watch; -use proxy::connect_proxy_stream; use rand::prelude::*; use release_channel::{AppVersion, ReleaseChannel}; use rpc::proto::{AnyTypedEnvelope, EnvelopedMessage, PeerId, RequestMessage}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use settings::{Settings, SettingsSources}; +use socks::connect_socks_proxy_stream; use std::pin::Pin; use std::{ any::TypeId, @@ -1156,7 +1156,7 @@ impl Client { let handle = cx.update(|cx| gpui_tokio::Tokio::handle(cx)).ok().unwrap(); let _guard = handle.enter(); match proxy { - Some(proxy) => connect_proxy_stream(&proxy, rpc_host).await?, + Some(proxy) => connect_socks_proxy_stream(&proxy, rpc_host).await?, None => Box::new(TcpStream::connect(rpc_host).await?), } }; diff --git a/crates/client/src/proxy.rs b/crates/client/src/proxy.rs deleted file mode 100644 index 7ec61458916368b1f6f76dfe1cc3534016de2591..0000000000000000000000000000000000000000 --- a/crates/client/src/proxy.rs +++ /dev/null @@ -1,66 +0,0 @@ -//! client proxy - -mod http_proxy; -mod socks_proxy; - -use anyhow::{Context, Result, anyhow}; -use http_client::Url; -use http_proxy::{HttpProxyType, connect_http_proxy_stream, parse_http_proxy}; -use socks_proxy::{SocksVersion, connect_socks_proxy_stream, parse_socks_proxy}; - -pub(crate) async fn connect_proxy_stream( - proxy: &Url, - rpc_host: (&str, u16), -) -> Result> { - let Some(((proxy_domain, proxy_port), proxy_type)) = parse_proxy_type(proxy) else { - // If parsing the proxy URL fails, we must avoid falling back to an insecure connection. - // SOCKS proxies are often used in contexts where security and privacy are critical, - // so any fallback could expose users to significant risks. - return Err(anyhow!("Parsing proxy url failed")); - }; - - // Connect to proxy and wrap protocol later - let stream = tokio::net::TcpStream::connect((proxy_domain.as_str(), proxy_port)) - .await - .context("Failed to connect to proxy")?; - - let proxy_stream = match proxy_type { - ProxyType::SocksProxy(proxy) => connect_socks_proxy_stream(stream, proxy, rpc_host).await?, - ProxyType::HttpProxy(proxy) => { - connect_http_proxy_stream(stream, proxy, rpc_host, &proxy_domain).await? - } - }; - - Ok(proxy_stream) -} - -enum ProxyType<'t> { - SocksProxy(SocksVersion<'t>), - HttpProxy(HttpProxyType<'t>), -} - -fn parse_proxy_type<'t>(proxy: &'t Url) -> Option<((String, u16), ProxyType<'t>)> { - let scheme = proxy.scheme(); - let host = proxy.host()?.to_string(); - let port = proxy.port_or_known_default()?; - let proxy_type = match scheme { - scheme if scheme.starts_with("socks") => { - Some(ProxyType::SocksProxy(parse_socks_proxy(scheme, proxy))) - } - scheme if scheme.starts_with("http") => { - Some(ProxyType::HttpProxy(parse_http_proxy(scheme, proxy))) - } - _ => None, - }?; - - Some(((host, port), proxy_type)) -} - -pub(crate) trait AsyncReadWrite: - tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin + Send + 'static -{ -} -impl AsyncReadWrite - for T -{ -} diff --git a/crates/client/src/proxy/http_proxy.rs b/crates/client/src/proxy/http_proxy.rs deleted file mode 100644 index 26ec964719a99e99f65a3db5cdf31dc3da922aae..0000000000000000000000000000000000000000 --- a/crates/client/src/proxy/http_proxy.rs +++ /dev/null @@ -1,171 +0,0 @@ -use anyhow::{Context, Result}; -use base64::Engine; -use httparse::{EMPTY_HEADER, Response}; -use tokio::{ - io::{AsyncBufReadExt, AsyncWriteExt, BufStream}, - net::TcpStream, -}; -use tokio_native_tls::{TlsConnector, native_tls}; -use url::Url; - -use super::AsyncReadWrite; - -pub(super) enum HttpProxyType<'t> { - HTTP(Option>), - HTTPS(Option>), -} - -pub(super) struct HttpProxyAuthorization<'t> { - username: &'t str, - password: &'t str, -} - -pub(super) fn parse_http_proxy<'t>(scheme: &str, proxy: &'t Url) -> HttpProxyType<'t> { - let auth = proxy.password().map(|password| HttpProxyAuthorization { - username: proxy.username(), - password, - }); - if scheme.starts_with("https") { - HttpProxyType::HTTPS(auth) - } else { - HttpProxyType::HTTP(auth) - } -} - -pub(crate) async fn connect_http_proxy_stream( - stream: TcpStream, - http_proxy: HttpProxyType<'_>, - rpc_host: (&str, u16), - proxy_domain: &str, -) -> Result> { - match http_proxy { - HttpProxyType::HTTP(auth) => http_connect(stream, rpc_host, auth).await, - HttpProxyType::HTTPS(auth) => https_connect(stream, rpc_host, auth, proxy_domain).await, - } - .context("error connecting to http/https proxy") -} - -async fn http_connect( - stream: T, - target: (&str, u16), - auth: Option>, -) -> Result> -where - T: AsyncReadWrite, -{ - let mut stream = BufStream::new(stream); - let request = make_request(target, auth); - stream.write_all(request.as_bytes()).await?; - stream.flush().await?; - check_response(&mut stream).await?; - Ok(Box::new(stream)) -} - -async fn https_connect( - stream: T, - target: (&str, u16), - auth: Option>, - proxy_domain: &str, -) -> Result> -where - T: AsyncReadWrite, -{ - let tls_connector = TlsConnector::from(native_tls::TlsConnector::new()?); - let stream = tls_connector.connect(proxy_domain, stream).await?; - http_connect(stream, target, auth).await -} - -fn make_request(target: (&str, u16), auth: Option>) -> String { - let (host, port) = target; - let mut request = format!( - "CONNECT {host}:{port} HTTP/1.1\r\nHost: {host}:{port}\r\nProxy-Connection: Keep-Alive\r\n" - ); - if let Some(HttpProxyAuthorization { username, password }) = auth { - let auth = - base64::prelude::BASE64_STANDARD.encode(format!("{username}:{password}").as_bytes()); - let auth = format!("Proxy-Authorization: Basic {auth}\r\n"); - request.push_str(&auth); - } - request.push_str("\r\n"); - request -} - -async fn check_response(stream: &mut BufStream) -> Result<()> -where - T: AsyncReadWrite, -{ - let response = recv_response(stream).await?; - let mut dummy_headers = [EMPTY_HEADER; MAX_RESPONSE_HEADERS]; - let mut parser = Response::new(&mut dummy_headers); - parser.parse(response.as_bytes())?; - - match parser.code { - Some(code) => { - if code == 200 { - Ok(()) - } else { - Err(anyhow::anyhow!( - "Proxy connection failed with HTTP code: {code}" - )) - } - } - None => Err(anyhow::anyhow!( - "Proxy connection failed with no HTTP code: {}", - parser.reason.unwrap_or("Unknown reason") - )), - } -} - -const MAX_RESPONSE_HEADER_LENGTH: usize = 4096; -const MAX_RESPONSE_HEADERS: usize = 16; - -async fn recv_response(stream: &mut BufStream) -> Result -where - T: AsyncReadWrite, -{ - let mut response = String::new(); - loop { - if stream.read_line(&mut response).await? == 0 { - return Err(anyhow::anyhow!("End of stream")); - } - - if MAX_RESPONSE_HEADER_LENGTH < response.len() { - return Err(anyhow::anyhow!("Maximum response header length exceeded")); - } - - if response.ends_with("\r\n\r\n") { - return Ok(response); - } - } -} - -#[cfg(test)] -mod tests { - use url::Url; - - use super::{HttpProxyAuthorization, HttpProxyType, parse_http_proxy}; - - #[test] - fn test_parse_http_proxy() { - let proxy = Url::parse("http://proxy.example.com:1080").unwrap(); - let scheme = proxy.scheme(); - - let version = parse_http_proxy(scheme, &proxy); - assert!(matches!(version, HttpProxyType::HTTP(None))) - } - - #[test] - fn test_parse_http_proxy_with_auth() { - let proxy = Url::parse("http://username:password@proxy.example.com:1080").unwrap(); - let scheme = proxy.scheme(); - - let version = parse_http_proxy(scheme, &proxy); - assert!(matches!( - version, - HttpProxyType::HTTP(Some(HttpProxyAuthorization { - username: "username", - password: "password" - })) - )) - } -} diff --git a/crates/client/src/proxy/socks_proxy.rs b/crates/client/src/socks.rs similarity index 50% rename from crates/client/src/proxy/socks_proxy.rs rename to crates/client/src/socks.rs index 300207be22b80940cb838093eda1c2f29b9c601a..1b283c14f9c40cc5a4e41bdb53746ed56316fe95 100644 --- a/crates/client/src/proxy/socks_proxy.rs +++ b/crates/client/src/socks.rs @@ -1,19 +1,15 @@ //! socks proxy - -use anyhow::{Context, Result}; -use tokio::net::TcpStream; +use anyhow::{Context, Result, anyhow}; +use http_client::Url; use tokio_socks::tcp::{Socks4Stream, Socks5Stream}; -use url::Url; - -use super::AsyncReadWrite; /// Identification to a Socks V4 Proxy -pub(super) struct Socks4Identification<'a> { +struct Socks4Identification<'a> { user_id: &'a str, } /// Authorization to a Socks V5 Proxy -pub(super) struct Socks5Authorization<'a> { +struct Socks5Authorization<'a> { username: &'a str, password: &'a str, } @@ -22,50 +18,45 @@ pub(super) struct Socks5Authorization<'a> { /// /// V4 allows idenfication using a user_id /// V5 allows authorization using a username and password -pub(super) enum SocksVersion<'a> { +enum SocksVersion<'a> { V4(Option>), V5(Option>), } -pub(super) fn parse_socks_proxy<'t>(scheme: &str, proxy: &'t Url) -> SocksVersion<'t> { - if scheme.starts_with("socks4") { - let identification = match proxy.username() { - "" => None, - username => Some(Socks4Identification { user_id: username }), - }; - SocksVersion::V4(identification) - } else { - let authorization = proxy.password().map(|password| Socks5Authorization { - username: proxy.username(), - password, - }); - SocksVersion::V5(authorization) - } -} - -pub(super) async fn connect_socks_proxy_stream( - stream: TcpStream, - socks_version: SocksVersion<'_>, +pub(crate) async fn connect_socks_proxy_stream( + proxy: &Url, rpc_host: (&str, u16), ) -> Result> { - match socks_version { + let Some((socks_proxy, version)) = parse_socks_proxy(proxy) else { + // If parsing the proxy URL fails, we must avoid falling back to an insecure connection. + // SOCKS proxies are often used in contexts where security and privacy are critical, + // so any fallback could expose users to significant risks. + return Err(anyhow!("Parsing proxy url failed")); + }; + + // Connect to proxy and wrap protocol later + let stream = tokio::net::TcpStream::connect(socks_proxy) + .await + .context("Failed to connect to socks proxy")?; + + let socks: Box = match version { SocksVersion::V4(None) => { let socks = Socks4Stream::connect_with_socket(stream, rpc_host) .await .context("error connecting to socks")?; - Ok(Box::new(socks)) + Box::new(socks) } SocksVersion::V4(Some(Socks4Identification { user_id })) => { let socks = Socks4Stream::connect_with_userid_and_socket(stream, rpc_host, user_id) .await .context("error connecting to socks")?; - Ok(Box::new(socks)) + Box::new(socks) } SocksVersion::V5(None) => { let socks = Socks5Stream::connect_with_socket(stream, rpc_host) .await .context("error connecting to socks")?; - Ok(Box::new(socks)) + Box::new(socks) } SocksVersion::V5(Some(Socks5Authorization { username, password })) => { let socks = Socks5Stream::connect_with_password_and_socket( @@ -73,9 +64,44 @@ pub(super) async fn connect_socks_proxy_stream( ) .await .context("error connecting to socks")?; - Ok(Box::new(socks)) + Box::new(socks) } - } + }; + + Ok(socks) +} + +fn parse_socks_proxy(proxy: &Url) -> Option<((String, u16), SocksVersion<'_>)> { + let scheme = proxy.scheme(); + let socks_version = if scheme.starts_with("socks4") { + let identification = match proxy.username() { + "" => None, + username => Some(Socks4Identification { user_id: username }), + }; + SocksVersion::V4(identification) + } else if scheme.starts_with("socks") { + let authorization = proxy.password().map(|password| Socks5Authorization { + username: proxy.username(), + password, + }); + SocksVersion::V5(authorization) + } else { + return None; + }; + + let host = proxy.host()?.to_string(); + let port = proxy.port_or_known_default()?; + + Some(((host, port), socks_version)) +} + +pub(crate) trait AsyncReadWrite: + tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin + Send + 'static +{ +} +impl AsyncReadWrite + for T +{ } #[cfg(test)] @@ -87,18 +113,20 @@ mod tests { #[test] fn parse_socks4() { let proxy = Url::parse("socks4://proxy.example.com:1080").unwrap(); - let scheme = proxy.scheme(); - let version = parse_socks_proxy(scheme, &proxy); + let ((host, port), version) = parse_socks_proxy(&proxy).unwrap(); + assert_eq!(host, "proxy.example.com"); + assert_eq!(port, 1080); assert!(matches!(version, SocksVersion::V4(None))) } #[test] fn parse_socks4_with_identification() { let proxy = Url::parse("socks4://userid@proxy.example.com:1080").unwrap(); - let scheme = proxy.scheme(); - let version = parse_socks_proxy(scheme, &proxy); + let ((host, port), version) = parse_socks_proxy(&proxy).unwrap(); + assert_eq!(host, "proxy.example.com"); + assert_eq!(port, 1080); assert!(matches!( version, SocksVersion::V4(Some(Socks4Identification { user_id: "userid" })) @@ -108,18 +136,20 @@ mod tests { #[test] fn parse_socks5() { let proxy = Url::parse("socks5://proxy.example.com:1080").unwrap(); - let scheme = proxy.scheme(); - let version = parse_socks_proxy(scheme, &proxy); + let ((host, port), version) = parse_socks_proxy(&proxy).unwrap(); + assert_eq!(host, "proxy.example.com"); + assert_eq!(port, 1080); assert!(matches!(version, SocksVersion::V5(None))) } #[test] fn parse_socks5_with_authorization() { let proxy = Url::parse("socks5://username:password@proxy.example.com:1080").unwrap(); - let scheme = proxy.scheme(); - let version = parse_socks_proxy(scheme, &proxy); + let ((host, port), version) = parse_socks_proxy(&proxy).unwrap(); + assert_eq!(host, "proxy.example.com"); + assert_eq!(port, 1080); assert!(matches!( version, SocksVersion::V5(Some(Socks5Authorization { @@ -128,4 +158,19 @@ mod tests { })) )) } + + /// If parsing the proxy URL fails, we must avoid falling back to an insecure connection. + /// SOCKS proxies are often used in contexts where security and privacy are critical, + /// so any fallback could expose users to significant risks. + #[tokio::test] + async fn fails_on_bad_proxy() { + // Should fail connecting because http is not a valid Socks proxy scheme + let proxy = Url::parse("http://localhost:2313").unwrap(); + + let result = connect_socks_proxy_stream(&proxy, ("test", 1080)).await; + match result { + Err(e) => assert_eq!(e.to_string(), "Parsing proxy url failed"), + Ok(_) => panic!("Connecting on bad proxy should fail"), + }; + } }