Skip to content

Commit c7f468d

Browse files
authored
fix(ext/fetch): use correct ALPN to proxies (#24696)
Sending ALPN to a proxy, and then when tunneling, requires better juggling of TLS configs. This improves the choice of TLS config in the proxy connector, based on what reqwest does. It also includes some `ext/fetch/tests.rs` that check the different combinations. Fixes #24632 Fixes #24691
1 parent b305ba3 commit c7f468d

File tree

5 files changed

+235
-36
lines changed

5 files changed

+235
-36
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/fetch/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ hyper.workspace = true
2727
hyper-rustls.workspace = true
2828
hyper-util.workspace = true
2929
ipnet.workspace = true
30+
rustls-webpki.workspace = true
3031
serde.workspace = true
3132
serde_json.workspace = true
3233
tokio.workspace = true

ext/fetch/lib.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
mod fs_fetch_handler;
44
mod proxy;
5+
#[cfg(test)]
6+
mod tests;
57

68
use std::borrow::Cow;
79
use std::cell::RefCell;
@@ -62,7 +64,6 @@ use http::Method;
6264
use http::Uri;
6365
use http_body_util::BodyExt;
6466
use hyper::body::Frame;
65-
use hyper_rustls::HttpsConnector;
6667
use hyper_util::client::legacy::connect::HttpConnector;
6768
use hyper_util::rt::TokioExecutor;
6869
use hyper_util::rt::TokioIo;
@@ -975,6 +976,10 @@ pub fn create_http_client(
975976
deno_tls::SocketUse::Http,
976977
)?;
977978

979+
// Proxy TLS should not send ALPN
980+
tls_config.alpn_protocols.clear();
981+
let proxy_tls_config = Arc::from(tls_config.clone());
982+
978983
let mut alpn_protocols = vec![];
979984
if options.http2 {
980985
alpn_protocols.push("h2".into());
@@ -987,7 +992,6 @@ pub fn create_http_client(
987992

988993
let mut http_connector = HttpConnector::new();
989994
http_connector.enforce_http(false);
990-
let connector = HttpsConnector::from((http_connector, tls_config.clone()));
991995

992996
let user_agent = user_agent
993997
.parse::<HeaderValue>()
@@ -1008,9 +1012,13 @@ pub fn create_http_client(
10081012
proxies.prepend(intercept);
10091013
}
10101014
let proxies = Arc::new(proxies);
1011-
let mut connector =
1012-
proxy::ProxyConnector::new(proxies.clone(), connector, tls_config);
1013-
connector.user_agent(user_agent.clone());
1015+
let connector = proxy::ProxyConnector {
1016+
http: http_connector,
1017+
proxies: proxies.clone(),
1018+
tls: tls_config,
1019+
tls_proxy: proxy_tls_config,
1020+
user_agent: Some(user_agent.clone()),
1021+
};
10141022

10151023
if let Some(pool_max_idle_per_host) = options.pool_max_idle_per_host {
10161024
builder.pool_max_idle_per_host(pool_max_idle_per_host);
@@ -1059,7 +1067,7 @@ pub struct Client {
10591067
user_agent: HeaderValue,
10601068
}
10611069

1062-
type Connector = proxy::ProxyConnector<HttpsConnector<HttpConnector>>;
1070+
type Connector = proxy::ProxyConnector<HttpConnector>;
10631071

10641072
// clippy is wrong here
10651073
#[allow(clippy::declare_interior_mutable_const)]

ext/fetch/proxy.rs

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ use deno_tls::rustls::ClientConfig as TlsConfig;
1717
use http::header::HeaderValue;
1818
use http::uri::Scheme;
1919
use http::Uri;
20+
use hyper_rustls::HttpsConnector;
21+
use hyper_rustls::MaybeHttpsStream;
2022
use hyper_util::client::legacy::connect::Connected;
2123
use hyper_util::client::legacy::connect::Connection;
2224
use hyper_util::rt::TokioIo;
@@ -29,10 +31,14 @@ use tower_service::Service;
2931

3032
#[derive(Debug, Clone)]
3133
pub(crate) struct ProxyConnector<C> {
32-
connector: C,
33-
proxies: Arc<Proxies>,
34-
tls: Arc<TlsConfig>,
35-
user_agent: Option<HeaderValue>,
34+
pub(crate) http: C,
35+
pub(crate) proxies: Arc<Proxies>,
36+
/// TLS config when destination is not a proxy
37+
pub(crate) tls: Arc<TlsConfig>,
38+
/// TLS config when destination is a proxy
39+
/// Notably, does not include ALPN
40+
pub(crate) tls_proxy: Arc<TlsConfig>,
41+
pub(crate) user_agent: Option<HeaderValue>,
3642
}
3743

3844
#[derive(Debug)]
@@ -361,23 +367,6 @@ impl DomainMatcher {
361367
}
362368

363369
impl<C> ProxyConnector<C> {
364-
pub(crate) fn new(
365-
proxies: Arc<Proxies>,
366-
connector: C,
367-
tls: Arc<TlsConfig>,
368-
) -> Self {
369-
ProxyConnector {
370-
connector,
371-
proxies,
372-
tls,
373-
user_agent: None,
374-
}
375-
}
376-
377-
pub(crate) fn user_agent(&mut self, val: HeaderValue) {
378-
self.user_agent = Some(val);
379-
}
380-
381370
fn intercept(&self, dst: &Uri) -> Option<&Intercept> {
382371
self.proxies.intercept(dst)
383372
}
@@ -438,20 +427,21 @@ pub enum Proxied<T> {
438427

439428
impl<C> Service<Uri> for ProxyConnector<C>
440429
where
441-
C: Service<Uri>,
442-
C::Response: hyper::rt::Read + hyper::rt::Write + Unpin + Send + 'static,
430+
C: Service<Uri> + Clone,
431+
C::Response:
432+
hyper::rt::Read + hyper::rt::Write + Connection + Unpin + Send + 'static,
443433
C::Future: Send + 'static,
444434
C::Error: Into<BoxError> + 'static,
445435
{
446-
type Response = Proxied<C::Response>;
436+
type Response = Proxied<MaybeHttpsStream<C::Response>>;
447437
type Error = BoxError;
448438
type Future = BoxFuture<Result<Self::Response, Self::Error>>;
449439

450440
fn poll_ready(
451441
&mut self,
452442
cx: &mut Context<'_>,
453443
) -> Poll<Result<(), Self::Error>> {
454-
self.connector.poll_ready(cx).map_err(Into::into)
444+
self.http.poll_ready(cx).map_err(Into::into)
455445
}
456446

457447
fn call(&mut self, orig_dst: Uri) -> Self::Future {
@@ -467,10 +457,12 @@ where
467457
dst: proxy_dst,
468458
auth,
469459
} => {
470-
let connecting = self.connector.call(proxy_dst);
460+
let mut connector =
461+
HttpsConnector::from((self.http.clone(), self.tls_proxy.clone()));
462+
let connecting = connector.call(proxy_dst);
471463
let tls = TlsConnector::from(self.tls.clone());
472464
Box::pin(async move {
473-
let mut io = connecting.await.map_err(Into::into)?;
465+
let mut io = connecting.await.map_err(Into::<BoxError>::into)?;
474466

475467
if is_https {
476468
tunnel(&mut io, &orig_dst, user_agent, auth).await?;
@@ -529,9 +521,11 @@ where
529521
}
530522
};
531523
}
524+
525+
let mut connector =
526+
HttpsConnector::from((self.http.clone(), self.tls.clone()));
532527
Box::pin(
533-
self
534-
.connector
528+
connector
535529
.call(orig_dst)
536530
.map_ok(Proxied::PassThrough)
537531
.map_err(Into::into),
@@ -721,7 +715,14 @@ where
721715
match self {
722716
Proxied::PassThrough(ref p) => p.connected(),
723717
Proxied::HttpForward(ref p) => p.connected().proxy(true),
724-
Proxied::HttpTunneled(ref p) => p.inner().get_ref().0.connected(),
718+
Proxied::HttpTunneled(ref p) => {
719+
let tunneled_tls = p.inner().get_ref();
720+
if tunneled_tls.1.alpn_protocol() == Some(b"h2") {
721+
tunneled_tls.0.connected().negotiated_h2()
722+
} else {
723+
tunneled_tls.0.connected()
724+
}
725+
}
725726
Proxied::Socks(ref p) => p.connected(),
726727
Proxied::SocksTls(ref p) => p.inner().get_ref().0.connected(),
727728
}

ext/fetch/tests.rs

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
2+
3+
use std::net::SocketAddr;
4+
use std::sync::Arc;
5+
6+
use bytes::Bytes;
7+
use http_body_util::BodyExt;
8+
use tokio::io::AsyncReadExt;
9+
use tokio::io::AsyncWriteExt;
10+
11+
use super::create_http_client;
12+
use super::CreateHttpClientOptions;
13+
14+
static EXAMPLE_CRT: &[u8] = include_bytes!("../tls/testdata/example1_cert.der");
15+
static EXAMPLE_KEY: &[u8] =
16+
include_bytes!("../tls/testdata/example1_prikey.der");
17+
18+
#[tokio::test]
19+
async fn test_https_proxy_http11() {
20+
let src_addr = create_https_server(false).await;
21+
let prx_addr = create_http_proxy(src_addr).await;
22+
run_test_client(prx_addr, src_addr, false, http::Version::HTTP_11).await;
23+
}
24+
25+
#[tokio::test]
26+
async fn test_https_proxy_h2() {
27+
let src_addr = create_https_server(true).await;
28+
let prx_addr = create_http_proxy(src_addr).await;
29+
run_test_client(prx_addr, src_addr, false, http::Version::HTTP_2).await;
30+
}
31+
32+
#[tokio::test]
33+
async fn test_https_proxy_https_h2() {
34+
let src_addr = create_https_server(true).await;
35+
let prx_addr = create_https_proxy(src_addr).await;
36+
run_test_client(prx_addr, src_addr, true, http::Version::HTTP_2).await;
37+
}
38+
39+
async fn run_test_client(
40+
prx_addr: SocketAddr,
41+
src_addr: SocketAddr,
42+
https: bool,
43+
ver: http::Version,
44+
) {
45+
let client = create_http_client(
46+
"fetch/test",
47+
CreateHttpClientOptions {
48+
root_cert_store: None,
49+
ca_certs: vec![],
50+
proxy: Some(deno_tls::Proxy {
51+
url: format!("http{}://{}", if https { "s" } else { "" }, prx_addr),
52+
basic_auth: None,
53+
}),
54+
unsafely_ignore_certificate_errors: Some(vec![]),
55+
client_cert_chain_and_key: None,
56+
pool_max_idle_per_host: None,
57+
pool_idle_timeout: None,
58+
http1: true,
59+
http2: true,
60+
},
61+
)
62+
.unwrap();
63+
64+
let req = http::Request::builder()
65+
.uri(format!("https://{}/foo", src_addr))
66+
.body(
67+
http_body_util::Empty::new()
68+
.map_err(|err| match err {})
69+
.boxed(),
70+
)
71+
.unwrap();
72+
let resp = client.send(req).await.unwrap();
73+
assert_eq!(resp.status(), http::StatusCode::OK);
74+
assert_eq!(resp.version(), ver);
75+
let hello = resp.collect().await.unwrap().to_bytes();
76+
assert_eq!(hello, "hello from server");
77+
}
78+
79+
async fn create_https_server(allow_h2: bool) -> SocketAddr {
80+
let mut tls_config = deno_tls::rustls::server::ServerConfig::builder()
81+
.with_no_client_auth()
82+
.with_single_cert(
83+
vec![EXAMPLE_CRT.into()],
84+
webpki::types::PrivateKeyDer::try_from(EXAMPLE_KEY).unwrap(),
85+
)
86+
.unwrap();
87+
if allow_h2 {
88+
tls_config.alpn_protocols.push("h2".into());
89+
}
90+
tls_config.alpn_protocols.push("http/1.1".into());
91+
let tls_acceptor = tokio_rustls::TlsAcceptor::from(Arc::from(tls_config));
92+
let src_tcp = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
93+
let src_addr = src_tcp.local_addr().unwrap();
94+
95+
tokio::spawn(async move {
96+
while let Ok((sock, _)) = src_tcp.accept().await {
97+
let conn = tls_acceptor.accept(sock).await.unwrap();
98+
if conn.get_ref().1.alpn_protocol() == Some(b"h2") {
99+
let fut = hyper::server::conn::http2::Builder::new(
100+
hyper_util::rt::TokioExecutor::new(),
101+
)
102+
.serve_connection(
103+
hyper_util::rt::TokioIo::new(conn),
104+
hyper::service::service_fn(|_req| async {
105+
Ok::<_, std::convert::Infallible>(http::Response::new(
106+
http_body_util::Full::<Bytes>::new("hello from server".into()),
107+
))
108+
}),
109+
);
110+
tokio::spawn(fut);
111+
} else {
112+
let fut = hyper::server::conn::http1::Builder::new().serve_connection(
113+
hyper_util::rt::TokioIo::new(conn),
114+
hyper::service::service_fn(|_req| async {
115+
Ok::<_, std::convert::Infallible>(http::Response::new(
116+
http_body_util::Full::<Bytes>::new("hello from server".into()),
117+
))
118+
}),
119+
);
120+
tokio::spawn(fut);
121+
}
122+
}
123+
});
124+
125+
src_addr
126+
}
127+
128+
async fn create_http_proxy(src_addr: SocketAddr) -> SocketAddr {
129+
let prx_tcp = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
130+
let prx_addr = prx_tcp.local_addr().unwrap();
131+
132+
tokio::spawn(async move {
133+
while let Ok((mut sock, _)) = prx_tcp.accept().await {
134+
let fut = async move {
135+
let mut buf = [0u8; 4096];
136+
let _n = sock.read(&mut buf).await.unwrap();
137+
assert_eq!(&buf[..7], b"CONNECT");
138+
let mut dst_tcp =
139+
tokio::net::TcpStream::connect(src_addr).await.unwrap();
140+
sock.write_all(b"HTTP/1.1 200 OK\r\n\r\n").await.unwrap();
141+
tokio::io::copy_bidirectional(&mut sock, &mut dst_tcp)
142+
.await
143+
.unwrap();
144+
};
145+
tokio::spawn(fut);
146+
}
147+
});
148+
149+
prx_addr
150+
}
151+
152+
async fn create_https_proxy(src_addr: SocketAddr) -> SocketAddr {
153+
let mut tls_config = deno_tls::rustls::server::ServerConfig::builder()
154+
.with_no_client_auth()
155+
.with_single_cert(
156+
vec![EXAMPLE_CRT.into()],
157+
webpki::types::PrivateKeyDer::try_from(EXAMPLE_KEY).unwrap(),
158+
)
159+
.unwrap();
160+
// Set ALPN, to check our proxy connector. But we shouldn't receive anything.
161+
tls_config.alpn_protocols.push("h2".into());
162+
tls_config.alpn_protocols.push("http/1.1".into());
163+
let tls_acceptor = tokio_rustls::TlsAcceptor::from(Arc::from(tls_config));
164+
let prx_tcp = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
165+
let prx_addr = prx_tcp.local_addr().unwrap();
166+
167+
tokio::spawn(async move {
168+
while let Ok((sock, _)) = prx_tcp.accept().await {
169+
let mut sock = tls_acceptor.accept(sock).await.unwrap();
170+
assert_eq!(sock.get_ref().1.alpn_protocol(), None);
171+
172+
let fut = async move {
173+
let mut buf = [0u8; 4096];
174+
let _n = sock.read(&mut buf).await.unwrap();
175+
assert_eq!(&buf[..7], b"CONNECT");
176+
let mut dst_tcp =
177+
tokio::net::TcpStream::connect(src_addr).await.unwrap();
178+
sock.write_all(b"HTTP/1.1 200 OK\r\n\r\n").await.unwrap();
179+
tokio::io::copy_bidirectional(&mut sock, &mut dst_tcp)
180+
.await
181+
.unwrap();
182+
};
183+
tokio::spawn(fut);
184+
}
185+
});
186+
187+
prx_addr
188+
}

0 commit comments

Comments
 (0)