Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/http/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ pub enum Error {
InvalidCRL,
#[error("UnsupportedProxyProtocol")]
UnsupportedProxyProtocol,
#[error("CONNECT tunnel failed, response {0}")]
ProxyConnectFailed(u32),
#[error(transparent)]
Cert(#[from] CertError),
#[error(transparent)]
Expand Down Expand Up @@ -308,6 +310,7 @@ impl Error {
Self::FailedToOpenSocket => "FailedToOpenSocket",
Self::InvalidCRL => "InvalidCRL",
Self::UnsupportedProxyProtocol => "UnsupportedProxyProtocol",
Self::ProxyConnectFailed(_) => "ProxyConnectFailed",
Self::Cert(e) => <&'static str>::from(e),
Self::Alloc(_) => "OutOfMemory",
Self::Hpack(e) => <&'static str>::from(e),
Expand Down
66 changes: 19 additions & 47 deletions src/http/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4764,23 +4764,29 @@ impl<'a> HTTPClient<'a> {
&mut self,
response: &mut picohttp::Response,
) -> crate::Result<ShouldContinue> {
if self.verbose != HTTPVerboseLevel::None {
print_response(response);
}

// RFC 9110 §9.3.6: any 2xx to CONNECT establishes the tunnel; a
// non-2xx CONNECT reply travelled over the plaintext client→proxy
// hop so it must reject the fetch, never surface as an https-origin
// Response (CVE-2009-2062). Dispatched before the header loop so no
// CONNECT-leg header or status writes `self.state` — ProxyTunnel
// does not reset it between the CONNECT leg and the origin leg.
Comment thread
robobun marked this conversation as resolved.
if self.flags.proxy_tunneling && self.proxy_tunnel.is_none() {
if response.status_code >= 200 && response.status_code < 300 {
return Ok(ShouldContinue::ContinueStreaming);
}
return Err(crate::Error::ProxyConnectFailed(response.status_code));
}

let mut location: &[u8] = b"";
let mut pretend_304 = false;
let mut is_server_sent_events = false;
for (header_i, header) in response.headers.list.iter().enumerate() {
match hash_header_name(header.name()) {
h if h == hash_header_const(b"Content-Length") => {
// RFC 9110 section 9.3.6: a client MUST ignore
// Content-Length in a successful response to CONNECT —
// the connection becomes an opaque tunnel and is never
// pooled, so the framing-desync concern below does not
// apply.
if self.flags.proxy_tunneling
&& self.proxy_tunnel.is_none()
&& response.status_code == 200
{
continue;
}
// byte-level parse — header.value() is network bytes, not &str
//
// RFC 9112 section 6.3: an invalid or conflicting
Expand Down Expand Up @@ -4835,15 +4841,6 @@ impl<'a> HTTPClient<'a> {
}
}
h if h == hash_header_const(b"Transfer-Encoding") => {
// RFC 9110 section 9.3.6: as with Content-Length above, a
// client MUST ignore Transfer-Encoding in a successful
// response to CONNECT.
if self.flags.proxy_tunneling
&& self.proxy_tunnel.is_none()
&& response.status_code == 200
{
continue;
}
// RFC 9112 §7: transfer-coding names are case-insensitive.
let value = header.value();
if strings::eql_case_insensitive_ascii_check_length(value, b"gzip")
Expand Down Expand Up @@ -4917,10 +4914,6 @@ impl<'a> HTTPClient<'a> {
}
}

if self.verbose != HTTPVerboseLevel::None {
print_response(response);
}

if pretend_304 {
response.status_code = 304;
}
Expand All @@ -4931,8 +4924,6 @@ impl<'a> HTTPClient<'a> {
// [...] cannot contain a message body or trailer section.
// Therefore in these cases set content-length to 0, so the response body is always ignored
// and is not waited for (which could cause a timeout).
// This applies regardless of whether we're using a proxy tunnel or not,
// since these status codes NEVER have a body per the HTTP spec.
if (response.status_code >= 100 && response.status_code < 200)
|| response.status_code == 204
|| response.status_code == 304
Expand All @@ -4958,24 +4949,6 @@ impl<'a> HTTPClient<'a> {
}
}

// RFC 9110 §9.3.6: a non-200 response to CONNECT means the tunnel was
// not established. Surface the proxy's response to the caller, but
// never follow a Location header from it — a malicious proxy could
// otherwise redirect the request (body and custom headers included)
// to an attacker-chosen plaintext origin.
let mut is_proxy_connect_failure = false;
if self.flags.proxy_tunneling && self.proxy_tunnel.is_none() {
if response.status_code == 200 {
// signal to continue the proxing
return Ok(ShouldContinue::ContinueStreaming);
}

// proxy denied connection so return proxy result (407, 403 etc)
self.flags.proxy_tunneling = false;
self.flags.disable_keepalive = true;
is_proxy_connect_failure = true;
}

let status_code = response.status_code;

if status_code == 407 {
Expand All @@ -4986,8 +4959,7 @@ impl<'a> HTTPClient<'a> {
// if is no redirect or if is redirect == "manual" just proceed
let is_redirect = status_code >= 300 && status_code <= 399;
if is_redirect {
if !is_proxy_connect_failure
&& self.redirect_type == FetchRedirect::Follow
if self.redirect_type == FetchRedirect::Follow
&& !location.is_empty()
&& self.remaining_redirect_count > 0
{
Expand Down Expand Up @@ -5246,7 +5218,7 @@ impl<'a> HTTPClient<'a> {
}
_ => {}
}
} else if !is_proxy_connect_failure && self.redirect_type == FetchRedirect::Error {
} else if self.redirect_type == FetchRedirect::Error {
// error out if redirect is not allowed
return Err(crate::Error::UnexpectedRedirect);
}
Expand Down
29 changes: 8 additions & 21 deletions src/http_jsc/websocket_client/WebSocketUpgradeClient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -985,19 +985,6 @@ impl<const SSL: bool> HTTPClient<SSL> {
body = &me.body;
}

// Check for HTTP 200 response from proxy
let is_first = me.body.is_empty();
const HTTP_200: &[u8] = b"HTTP/1.1 200 ";
const HTTP_200_ALT: &[u8] = b"HTTP/1.0 200 ";
if is_first && body.len() > HTTP_200.len() {
if !body.starts_with(HTTP_200) && !body.starts_with(HTTP_200_ALT) {
// Proxy connection failed
// SAFETY: `me`'s last use is above; no `&mut Self` spans this call.
unsafe { Self::terminate(this.as_ptr(), ErrorCode::ProxyConnectFailed) };
return;
}
}

// Parse the response to find the end of headers
let response = match picohttp::Response::parse(body, &mut me.headers_buf) {
Ok(r) => r,
Expand All @@ -1021,15 +1008,15 @@ impl<const SSL: bool> HTTPClient<SSL> {
}
};

// Proxy returned non-200 status
if response.status_code != 200 {
if response.status_code == 407 {
// SAFETY: `me`'s last use is above; no `&mut Self` spans this call.
unsafe { Self::terminate(this.as_ptr(), ErrorCode::ProxyAuthenticationRequired) };
// RFC 9110 §9.3.6: any 2xx to CONNECT establishes the tunnel.
if !(200..300).contains(&response.status_code) {
let code = if response.status_code == 407 {
ErrorCode::ProxyAuthenticationRequired
} else {
// SAFETY: `me`'s last use is above; no `&mut Self` spans this call.
unsafe { Self::terminate(this.as_ptr(), ErrorCode::ProxyConnectFailed) };
}
ErrorCode::ProxyConnectFailed
};
// SAFETY: `me`'s last use is above; no `&mut Self` spans this call.
unsafe { Self::terminate(this.as_ptr(), code) };
return;
}

Expand Down
3 changes: 3 additions & 0 deletions src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,6 +1348,9 @@ impl FetchTasklet {
http::Error::RedirectURLInvalid => {
BunString::static_("Redirect URL in Location header is invalid.")
}
http::Error::ProxyConnectFailed(status) => BunString::create_format(format_args!(
"CONNECT tunnel failed, proxy responded with status {status}",
)),

http::Error::Cert(http::CertError::UNABLE_TO_GET_ISSUER_CERT) => {
BunString::static_("unable to get issuer certificate")
Expand Down
Loading
Loading