From 40d4d05e4ee0371f4fc6b3060ef6a0fe77c5de00 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Thu, 17 Oct 2019 17:27:25 -0700 Subject: [PATCH] Remove username and password when parsing proxies (#686) --- src/connect.rs | 37 +++++++++------- src/proxy.rs | 115 ++++++++++++++++++++++++++++++++++++------------- 2 files changed, 107 insertions(+), 45 deletions(-) diff --git a/src/connect.rs b/src/connect.rs index de439f2..c553fac 100644 --- a/src/connect.rs +++ b/src/connect.rs @@ -1,5 +1,5 @@ use futures_util::FutureExt; -use http::uri::Scheme; +use http::uri::{Scheme, Authority}; use hyper::client::connect::{Connect, Connected, Destination}; use tokio::io::{AsyncRead, AsyncWrite}; @@ -255,22 +255,13 @@ impl Connector { ) -> Result<(Conn, Connected), io::Error> { log::trace!("proxy({:?}) intercepts {:?}", proxy_scheme, dst); - let (puri, _auth) = match proxy_scheme { - ProxyScheme::Http { uri, auth, .. } => (uri, auth), + let (proxy_dst, _auth) = match proxy_scheme { + ProxyScheme::Http { host, auth } => (into_dst(Scheme::HTTP, host), auth), + ProxyScheme::Https { host, auth } => (into_dst(Scheme::HTTPS, host), auth), #[cfg(feature = "socks")] ProxyScheme::Socks5 { .. } => return this.connect_socks(dst, proxy_scheme), }; - let mut ndst = dst.clone(); - - let new_scheme = puri.scheme_part().map(Scheme::as_str).unwrap_or("http"); - ndst.set_scheme(new_scheme) - .expect("proxy target scheme should be valid"); - - ndst.set_host(puri.host().expect("proxy target should have host")) - .expect("proxy target host should be valid"); - - ndst.set_port(puri.port_part().map(|port| port.as_u16())); #[cfg(feature = "tls")] let auth = _auth; @@ -285,7 +276,7 @@ impl Connector { http.set_nodelay(self.nodelay); let tls_connector = tokio_tls::TlsConnector::from(tls.clone()); let http = hyper_tls::HttpsConnector::from((http, tls_connector)); - let (conn, connected) = http.connect(ndst).await?; + let (conn, connected) = http.connect(proxy_dst).await?; log::trace!("tunneling HTTPS over proxy"); let tunneled = tunnel(conn, host.clone(), port, self.user_agent.clone(), auth).await?; let tls_connector = tokio_tls::TlsConnector::from(tls.clone()); @@ -313,7 +304,7 @@ impl Connector { http.set_nodelay(no_delay); let http = hyper_rustls::HttpsConnector::from((http, tls_proxy.clone())); let tls = tls.clone(); - let (conn, connected) = http.connect(ndst).await?; + let (conn, connected) = http.connect(proxy_dst).await?; log::trace!("tunneling HTTPS over proxy"); let maybe_dnsname = DNSNameRef::try_from_ascii_str(&host) .map(|dnsname| dnsname.to_owned()) @@ -336,10 +327,24 @@ impl Connector { Inner::Http(_) => (), } - self.connect_with_maybe_proxy(ndst, true).await + self.connect_with_maybe_proxy(proxy_dst, true).await } } +fn into_dst(scheme: Scheme, host: Authority) -> Destination { + use std::convert::TryInto; + + // TODO: Should the `http` crate get `From<(Scheme, Authority)> for Uri`? + http::Uri::builder() + .scheme(scheme) + .authority(host) + .path_and_query(http::uri::PathAndQuery::from_static("/")) + .build() + .expect("scheme and authority is valid Uri") + .try_into() + .expect("scheme and authority is valid Destination") +} + //#[cfg(feature = "trust-dns")] //fn http_connector() -> crate::Result { // TrustDnsResolver::new() diff --git a/src/proxy.rs b/src/proxy.rs index c987120..9cfccdf 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -60,7 +60,11 @@ pub struct Proxy { pub enum ProxyScheme { Http { auth: Option, - uri: hyper::Uri, + host: http::uri::Authority, + }, + Https { + auth: Option, + host: http::uri::Authority, }, #[cfg(feature = "socks")] Socks5 { @@ -226,6 +230,7 @@ impl Proxy { | Intercept::Http(ProxyScheme::Http { ref auth, .. }) => auth.clone(), Intercept::Custom(ref custom) => custom.call(uri).and_then(|scheme| match scheme { ProxyScheme::Http { auth, .. } => auth, + ProxyScheme::Https { auth, .. } => auth, #[cfg(feature = "socks")] _ => None, }), @@ -278,10 +283,18 @@ impl ProxyScheme { // To start conservative, keep builders private for now. /// Proxy traffic via the specified URL over HTTP - fn http(url: T) -> crate::Result { + fn http(host: &str) -> crate::Result { Ok(ProxyScheme::Http { auth: None, - uri: crate::into_url::expect_uri(&url.into_url()?), + host: host.parse().map_err(crate::error::builder)?, + }) + } + + /// Proxy traffic via the specified URL over HTTPS + fn https(host: &str) -> crate::Result { + Ok(ProxyScheme::Https { + auth: None, + host: host.parse().map_err(crate::error::builder)?, }) } @@ -330,7 +343,11 @@ impl ProxyScheme { ProxyScheme::Http { ref mut auth, .. } => { let header = encode_basic_auth(&username.into(), &password.into()); *auth = Some(header); - } + }, + ProxyScheme::Https { ref mut auth, .. } => { + let header = encode_basic_auth(&username.into(), &password.into()); + *auth = Some(header); + }, #[cfg(feature = "socks")] ProxyScheme::Socks5 { ref mut auth, .. } => { *auth = Some((username.into(), password.into())); @@ -338,11 +355,32 @@ impl ProxyScheme { } } + fn if_no_auth(mut self, update: &Option) -> Self { + match self { + ProxyScheme::Http { ref mut auth, .. } => { + if auth.is_none() { + *auth = update.clone(); + } + }, + ProxyScheme::Https { ref mut auth, .. } => { + if auth.is_none() { + *auth = update.clone(); + } + }, + #[cfg(feature = "socks")] + ProxyScheme::Socks5 { .. } => {} + } + + self + } + /// Convert a URL into a proxy scheme /// /// Supported schemes: HTTP, HTTPS, (SOCKS5, SOCKS5H if `socks` feature is enabled). // Private for now... fn parse(url: Url) -> crate::Result { + use url::Position; + // Resolve URL to a host and port #[cfg(feature = "socks")] let to_addr = || { @@ -357,7 +395,8 @@ impl ProxyScheme { }; let mut scheme = match url.scheme() { - "http" | "https" => Self::http(url.clone())?, + "http" => Self::http(&url[Position::BeforeHost..Position::AfterPort])?, + "https" => Self::https(&url[Position::BeforeHost..Position::AfterPort])?, #[cfg(feature = "socks")] "socks5" => Self::socks5(to_addr()?)?, #[cfg(feature = "socks")] @@ -375,13 +414,22 @@ impl ProxyScheme { } #[cfg(test)] - fn http_uri(&self) -> &http::Uri { + fn scheme(&self) -> &str { match self { - ProxyScheme::Http { ref uri, .. } => { - uri - }, + ProxyScheme::Http { .. } => "http", + ProxyScheme::Https { .. } => "https", #[cfg(feature = "socks")] - _ => panic!("socks not expected"), + ProxyScheme::Socks5 => "socks5", + } + } + + #[cfg(test)] + fn host(&self) -> &str { + match self { + ProxyScheme::Http { host, .. } => host.as_str(), + ProxyScheme::Https { host, .. } => host.as_str(), + #[cfg(feature = "socks")] + ProxyScheme::Socks5 => panic!("socks5"), } } } @@ -433,20 +481,7 @@ impl Custom { (self.func)(&url) .and_then(|result| result.ok()) - .map(|scheme| match scheme { - ProxyScheme::Http { auth, uri } => { - if auth.is_some() { - ProxyScheme::Http { auth, uri } - } else { - ProxyScheme::Http { - auth: self.auth.clone(), - uri, - } - } - } - #[cfg(feature = "socks")] - socks => socks, - }) + .map(|scheme| scheme.if_no_auth(&self.auth)) } } @@ -659,11 +694,18 @@ mod tests { } fn intercepted_uri(p: &Proxy, s: &str) -> Uri { - match p.intercept(&url(s)).unwrap() { - ProxyScheme::Http { uri, .. } => uri, + let (scheme, host) = match p.intercept(&url(s)).unwrap() { + ProxyScheme::Http { host, .. } => ("http", host), + ProxyScheme::Https { host, .. } => ("https", host), #[cfg(feature = "socks")] _ => panic!("intercepted as socks"), - } + }; + http::Uri::builder() + .scheme(scheme) + .authority(host) + .path_and_query("/") + .build() + .expect("intercepted_uri") } #[test] @@ -727,6 +769,19 @@ mod tests { assert!(p.intercept(&url(other)).is_none()); } + #[test] + fn test_proxy_scheme_parse() { + let ps = "http://foo:bar@localhost:1239".into_proxy_scheme().unwrap(); + + match ps { + ProxyScheme::Http { auth, host } => { + assert_eq!(auth.unwrap(), encode_basic_auth("foo", "bar")); + assert_eq!(host, "localhost:1239"); + }, + other => panic!("unexpected: {:?}", other), + } + } + #[test] fn test_get_sys_proxies_parsing() { // save system setting first. @@ -744,7 +799,9 @@ mod tests { env::set_var("http_proxy", "http://127.0.0.1/"); let proxies = get_sys_proxies(); - assert_eq!(proxies["http"].http_uri(), "http://127.0.0.1/"); + let p = &proxies["http"]; + assert_eq!(p.scheme(), "http"); + assert_eq!(p.host(), "127.0.0.1"); // reset user setting when guards drop } @@ -759,7 +816,7 @@ mod tests { env::set_var("HTTP_PROXY", "http://evil/"); // not in CGI yet - assert_eq!(get_sys_proxies()["http"].http_uri(), "http://evil/"); + assert_eq!(get_sys_proxies()["http"].host(), "evil"); // set like we're in CGI env::set_var("REQUEST_METHOD", "GET");