From e72b7493c6b7478579ca2da1bec9320ea4b456c8 Mon Sep 17 00:00:00 2001 From: Trevor Elliott Date: Tue, 14 Nov 2023 14:06:14 -0800 Subject: [PATCH] Filter out forbidden headers on incoming request and response resources (#7538) --- crates/test-programs/src/bin/api_proxy.rs | 9 ++-- crates/wasi-http/src/types.rs | 59 ++++++++++++++++++++--- crates/wasi-http/src/types_impl.rs | 28 +++-------- 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/crates/test-programs/src/bin/api_proxy.rs b/crates/test-programs/src/bin/api_proxy.rs index c9f292330dc6..d9ca3936fab0 100644 --- a/crates/test-programs/src/bin/api_proxy.rs +++ b/crates/test-programs/src/bin/api_proxy.rs @@ -20,15 +20,16 @@ impl bindings::exports::wasi::http::incoming_handler::Guest for T { let req_hdrs = request.headers(); assert!( - !req_hdrs.get(&header).is_empty(), - "missing `custom-forbidden-header` from request" + req_hdrs.get(&header).is_empty(), + "forbidden `custom-forbidden-header` found in request" ); assert!(req_hdrs.delete(&header).is_err()); + assert!(req_hdrs.append(&header, &b"no".to_vec()).is_err()); assert!( - !req_hdrs.get(&header).is_empty(), - "delete of forbidden header succeeded" + req_hdrs.get(&header).is_empty(), + "append of forbidden header succeeded" ); let hdrs = bindings::wasi::http::types::Headers::new(); diff --git a/crates/wasi-http/src/types.rs b/crates/wasi-http/src/types.rs index 1a44b7910869..4473b3912574 100644 --- a/crates/wasi-http/src/types.rs +++ b/crates/wasi-http/src/types.rs @@ -35,17 +35,18 @@ pub trait WasiHttpView: Send { fn new_incoming_request( &mut self, req: hyper::Request, - ) -> wasmtime::Result> { + ) -> wasmtime::Result> + where + Self: Sized, + { let (parts, body) = req.into_parts(); let body = HostIncomingBody::new( body, // TODO: this needs to be plumbed through std::time::Duration::from_millis(600 * 1000), ); - Ok(self.table().push(HostIncomingRequest { - parts, - body: Some(body), - })?) + let incoming_req = HostIncomingRequest::new(self, parts, Some(body)); + Ok(self.table().push(incoming_req)?) } fn new_response_outparam( @@ -73,6 +74,41 @@ pub trait WasiHttpView: Send { } } +/// Returns `true` when the header is forbidden according to this [`WasiHttpView`] implementation. +pub(crate) fn is_forbidden_header(view: &mut dyn WasiHttpView, name: &HeaderName) -> bool { + static FORBIDDEN_HEADERS: [HeaderName; 9] = [ + hyper::header::CONNECTION, + HeaderName::from_static("keep-alive"), + hyper::header::PROXY_AUTHENTICATE, + hyper::header::PROXY_AUTHORIZATION, + HeaderName::from_static("proxy-connection"), + hyper::header::TE, + hyper::header::TRANSFER_ENCODING, + hyper::header::UPGRADE, + HeaderName::from_static("http2-settings"), + ]; + + FORBIDDEN_HEADERS.contains(name) || view.is_forbidden_header(name) +} + +/// Removes forbidden headers from a [`hyper::HeaderMap`]. +pub(crate) fn remove_forbidden_headers( + view: &mut dyn WasiHttpView, + headers: &mut hyper::HeaderMap, +) { + let forbidden_keys = Vec::from_iter(headers.keys().filter_map(|name| { + if is_forbidden_header(view, name) { + Some(name.clone()) + } else { + None + } + })); + + for name in forbidden_keys { + headers.remove(name); + } +} + pub fn default_send_request( view: &mut dyn WasiHttpView, OutgoingRequest { @@ -263,10 +299,21 @@ impl TryInto for types::Method { } pub struct HostIncomingRequest { - pub parts: http::request::Parts, + pub(crate) parts: http::request::Parts, pub body: Option, } +impl HostIncomingRequest { + pub fn new( + view: &mut dyn WasiHttpView, + mut parts: http::request::Parts, + body: Option, + ) -> Self { + remove_forbidden_headers(view, &mut parts.headers); + Self { parts, body } + } +} + pub struct HostResponseOutparam { pub result: tokio::sync::oneshot::Sender, types::ErrorCode>>, diff --git a/crates/wasi-http/src/types_impl.rs b/crates/wasi-http/src/types_impl.rs index 6529381c3fb8..ca11e87fe0ea 100644 --- a/crates/wasi-http/src/types_impl.rs +++ b/crates/wasi-http/src/types_impl.rs @@ -2,13 +2,13 @@ use crate::{ bindings::http::types::{self, Headers, Method, Scheme, StatusCode, Trailers}, body::{HostFutureTrailers, HostIncomingBody, HostOutgoingBody}, types::{ - FieldMap, HostFields, HostFutureIncomingResponse, HostIncomingRequest, - HostIncomingResponse, HostOutgoingRequest, HostOutgoingResponse, HostResponseOutparam, + is_forbidden_header, remove_forbidden_headers, FieldMap, HostFields, + HostFutureIncomingResponse, HostIncomingRequest, HostIncomingResponse, HostOutgoingRequest, + HostOutgoingResponse, HostResponseOutparam, }, WasiHttpView, }; use anyhow::Context; -use hyper::header::HeaderName; use std::any::Any; use std::str::FromStr; use wasmtime::component::Resource; @@ -89,22 +89,6 @@ fn get_fields_mut<'a>( } } -fn is_forbidden_header(view: &mut T, name: &HeaderName) -> bool { - static FORBIDDEN_HEADERS: [HeaderName; 9] = [ - hyper::header::CONNECTION, - HeaderName::from_static("keep-alive"), - hyper::header::PROXY_AUTHENTICATE, - hyper::header::PROXY_AUTHORIZATION, - HeaderName::from_static("proxy-connection"), - hyper::header::TE, - hyper::header::TRANSFER_ENCODING, - hyper::header::UPGRADE, - HeaderName::from_static("http2-settings"), - ]; - - FORBIDDEN_HEADERS.contains(name) || view.is_forbidden_header(name) -} - impl crate::bindings::http::types::HostFields for T { fn new(&mut self) -> wasmtime::Result> { let id = self @@ -834,11 +818,13 @@ impl crate::bindings::http::types::HostFutureIncomingResponse f Ok(Err(e)) => return Ok(Some(Ok(Err(e)))), }; - let (parts, body) = resp.resp.into_parts(); + let (mut parts, body) = resp.resp.into_parts(); + + remove_forbidden_headers(self, &mut parts.headers); let resp = self.table().push(HostIncomingResponse { status: parts.status.as_u16(), - headers: FieldMap::from(parts.headers), + headers: parts.headers, body: Some({ let mut body = HostIncomingBody::new(body, resp.between_bytes_timeout); body.retain_worker(&resp.worker);