Skip to content

Commit 72ba8ae

Browse files
committed
Deny unallowed hosts
Signed-off-by: Ryan Levick <[email protected]>
1 parent 0cc5c0f commit 72ba8ae

File tree

12 files changed

+147
-102
lines changed

12 files changed

+147
-102
lines changed

Cargo.lock

Lines changed: 39 additions & 38 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,14 @@ members = ["crates/*", "sdk/rust", "sdk/rust/macro"]
111111
[workspace.dependencies]
112112
anyhow = "1.0.75"
113113
tracing = { version = "0.1", features = ["log"] }
114-
wasmtime-wasi = { git = "https://github.com/bytecodealliance/wasmtime", rev = "2ffbc36c377b98e4eabe89ae37bb334605d904cc", features = [
114+
wasmtime-wasi = { git = "https://github.com/bytecodealliance/wasmtime", rev = "2da78ca5c4c1e5a8cbc88a8ad3accf24bb4e6cfb", features = [
115115
"tokio",
116116
] }
117-
wasi-common-preview1 = { git = "https://github.com/bytecodealliance/wasmtime", rev = "2ffbc36c377b98e4eabe89ae37bb334605d904cc", package = "wasi-common" }
118-
wasmtime = { git = "https://github.com/bytecodealliance/wasmtime", rev = "2ffbc36c377b98e4eabe89ae37bb334605d904cc", features = [
117+
wasi-common-preview1 = { git = "https://github.com/bytecodealliance/wasmtime", rev = "2da78ca5c4c1e5a8cbc88a8ad3accf24bb4e6cfb", package = "wasi-common" }
118+
wasmtime = { git = "https://github.com/bytecodealliance/wasmtime", rev = "2da78ca5c4c1e5a8cbc88a8ad3accf24bb4e6cfb", features = [
119119
"component-model",
120120
] }
121-
wasmtime-wasi-http = { git = "https://github.com/bytecodealliance/wasmtime", rev = "2ffbc36c377b98e4eabe89ae37bb334605d904cc" }
121+
wasmtime-wasi-http = { git = "https://github.com/bytecodealliance/wasmtime", rev = "2da78ca5c4c1e5a8cbc88a8ad3accf24bb4e6cfb" }
122122
spin-componentize = { git = "https://github.com/fermyon/spin-componentize", rev = "191789170abde10cd55590466c0660dd6c7d472a" }
123123
hyper = { version = "=1.0.0-rc.3", features = ["full"] }
124124
http-body-util = "=0.1.0-rc.2"

crates/core/tests/integration_test.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,9 @@ fn test_config() -> Config {
178178
fn test_engine() -> Engine<()> {
179179
let mut builder = Engine::builder(&test_config()).unwrap();
180180
builder.add_host_component(MultiplierHostComponent).unwrap();
181+
builder
182+
.link_import(|l, _| wasmtime_wasi::preview2::command::add_to_linker(l))
183+
.unwrap();
181184
builder.build()
182185
}
183186

crates/outbound-http/src/allowed_http_hosts.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@ impl Default for AllowedHttpHosts {
2020

2121
impl AllowedHttpHosts {
2222
/// Tests whether the given URL is allowed according to the allow-list.
23-
pub fn allow(&self, url: &url::Url) -> bool {
23+
pub fn allows(&self, url: &url::Url) -> bool {
2424
match self {
2525
Self::AllowAll => true,
2626
Self::AllowSpecific(hosts) => hosts.iter().any(|h| h.allow(url)),
2727
}
2828
}
2929

30-
pub fn allow_relative_url(&self) -> bool {
30+
pub fn allows_relative_url(&self) -> bool {
3131
match self {
3232
Self::AllowAll => true,
3333
Self::AllowSpecific(hosts) => hosts.contains(&AllowedHttpHost::host("self")),
@@ -295,21 +295,21 @@ mod test {
295295
fn test_allowed_hosts_can_be_specific() {
296296
let allowed =
297297
parse_allowed_http_hosts(&["spin.fermyon.dev", "http://example.com:8383"]).unwrap();
298-
assert!(allowed.allow(&Url::parse("http://example.com:8383/foo/bar").unwrap()));
299-
assert!(allowed.allow(&Url::parse("https://spin.fermyon.dev/").unwrap()));
300-
assert!(!allowed.allow(&Url::parse("http://example.com/").unwrap()));
301-
assert!(!allowed.allow(&Url::parse("http://google.com/").unwrap()));
298+
assert!(allowed.allows(&Url::parse("http://example.com:8383/foo/bar").unwrap()));
299+
assert!(allowed.allows(&Url::parse("https://spin.fermyon.dev/").unwrap()));
300+
assert!(!allowed.allows(&Url::parse("http://example.com/").unwrap()));
301+
assert!(!allowed.allows(&Url::parse("http://google.com/").unwrap()));
302302
}
303303

304304
#[test]
305305
fn test_allowed_hosts_allow_relative_url() {
306306
let allowed = parse_allowed_http_hosts(&["self", "http://example.com:8383"]).unwrap();
307-
assert!(allowed.allow_relative_url());
307+
assert!(allowed.allows_relative_url());
308308

309309
let not_allowed = parse_allowed_http_hosts(&["http://example.com:8383"]).unwrap();
310-
assert!(!not_allowed.allow_relative_url());
310+
assert!(!not_allowed.allows_relative_url());
311311

312312
let allow_all = parse_allowed_http_hosts(&["insecure:allow-all"]).unwrap();
313-
assert!(allow_all.allow_relative_url());
313+
assert!(allow_all.allows_relative_url());
314314
}
315315
}

crates/outbound-http/src/host_impl.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ impl OutboundHttp {
2828
/// If `None` is passed, the guest module is not allowed to send the request.
2929
fn is_allowed(&mut self, url: &str) -> Result<bool, HttpError> {
3030
if url.starts_with('/') {
31-
return Ok(self.allowed_hosts.allow_relative_url());
31+
return Ok(self.allowed_hosts.allows_relative_url());
3232
}
3333

3434
let url = Url::parse(url).map_err(|_| HttpError::InvalidUrl)?;
35-
Ok(self.allowed_hosts.allow(&url))
35+
Ok(self.allowed_hosts.allows(&url))
3636
}
3737
}
3838

crates/trigger-http/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ tls-listener = { version = "0.4.0", features = [
3535
] }
3636
tokio = { version = "1.23", features = ["full"] }
3737
tokio-rustls = { version = "0.23.2" }
38+
url = "2.4.1"
3839
tracing = { workspace = true }
3940
wasmtime = { workspace = true }
4041
wasmtime-wasi = { workspace = true }

crates/trigger-http/src/handler.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::{net::SocketAddr, str, str::FromStr};
33
use crate::{Body, HttpExecutor, HttpTrigger, Store};
44
use anyhow::bail;
55
use anyhow::{anyhow, Context, Result};
6+
use http::{HeaderName, HeaderValue};
67
use http_body_util::BodyExt;
78
use hyper::{Request, Response};
89
use outbound_http::OutboundHttpComponent;
@@ -42,7 +43,7 @@ impl HttpExecutor for HttpHandlerExecutor {
4243
set_http_origin_from_request(&mut store, engine, &req);
4344

4445
let resp = match HandlerType::from_exports(instance.exports(&mut store)) {
45-
Some(HandlerType::Wasi) => Self::execute_wasi(store, instance, req).await?,
46+
Some(HandlerType::Wasi) => Self::execute_wasi(store, instance, base, raw_route, req, client_addr).await?,
4647
Some(HandlerType::Spin) => {
4748
Self::execute_spin(store, instance, base, raw_route, req, client_addr)
4849
.await
@@ -68,11 +69,7 @@ impl HttpHandlerExecutor {
6869
req: Request<Body>,
6970
client_addr: SocketAddr,
7071
) -> Result<Response<Body>> {
71-
let headers;
72-
let mut req = req;
73-
{
74-
headers = Self::headers(&mut req, raw_route, base, client_addr)?;
75-
}
72+
let headers = Self::headers(&req, raw_route, base, client_addr)?;
7673
let func = instance
7774
.exports(&mut store)
7875
.instance("fermyon:spin/inbound-http")
@@ -147,8 +144,23 @@ impl HttpHandlerExecutor {
147144
async fn execute_wasi(
148145
mut store: Store,
149146
instance: Instance,
150-
req: Request<Body>,
147+
base: &str,
148+
raw_route: &str,
149+
mut req: Request<Body>,
150+
client_addr: SocketAddr,
151151
) -> anyhow::Result<Response<Body>> {
152+
let headers = Self::headers(&req, raw_route, base, client_addr)?;
153+
req.headers_mut().clear();
154+
req.headers_mut()
155+
.extend(headers.into_iter().filter_map(|(n, v)| {
156+
let Ok(name) = n.parse::<HeaderName>() else {
157+
return None;
158+
};
159+
let Ok(value) = HeaderValue::from_bytes(v.as_bytes()) else {
160+
return None;
161+
};
162+
Some((name, value))
163+
}));
152164
let request = store.as_mut().data_mut().new_incoming_request(req)?;
153165

154166
let (response_tx, response_rx) = oneshot::channel();
@@ -190,7 +202,7 @@ impl HttpHandlerExecutor {
190202
}
191203

192204
fn headers(
193-
req: &mut Request<Body>,
205+
req: &Request<Body>,
194206
raw: &str,
195207
base: &str,
196208
client_addr: SocketAddr,
@@ -280,6 +292,8 @@ fn set_http_origin_from_request(
280292
.get_or_insert(outbound_http_handle);
281293

282294
outbound_http_data.origin = origin.clone();
295+
store.as_mut().data_mut().as_mut().allowed_hosts =
296+
outbound_http_data.allowed_hosts.clone();
283297
}
284298
store.as_mut().data_mut().as_mut().origin = Some(origin);
285299
}

crates/trigger-http/src/lib.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use hyper::{
2222
service::service_fn,
2323
Request, Response,
2424
};
25+
use outbound_http::allowed_http_hosts::AllowedHttpHosts;
2526
use spin_app::{AppComponent, APP_DESCRIPTION_KEY};
2627
use spin_core::{Engine, OutboundWasiHttpHandler};
2728
use spin_http::{
@@ -446,6 +447,7 @@ pub(crate) trait HttpExecutor: Clone + Send + Sync + 'static {
446447
#[derive(Default)]
447448
pub struct HttpRuntimeData {
448449
origin: Option<String>,
450+
allowed_hosts: AllowedHttpHosts,
449451
}
450452

451453
impl OutboundWasiHttpHandler for HttpRuntimeData {
@@ -458,6 +460,7 @@ impl OutboundWasiHttpHandler for HttpRuntimeData {
458460
where
459461
Self: Sized,
460462
{
463+
let this = data.as_ref();
461464
let is_relative_url = request
462465
.request
463466
.uri()
@@ -466,7 +469,7 @@ impl OutboundWasiHttpHandler for HttpRuntimeData {
466469
.unwrap_or_default();
467470
if is_relative_url {
468471
// Origin must be set in the incoming http handler
469-
let origin = data.as_ref().origin.clone().unwrap();
472+
let origin = this.origin.clone().unwrap();
470473
let path_and_query = request
471474
.request
472475
.uri()
@@ -487,6 +490,28 @@ impl OutboundWasiHttpHandler for HttpRuntimeData {
487490
*request.request.uri_mut() = uri;
488491
}
489492

493+
let unallowed_relative = is_relative_url && !this.allowed_hosts.allows_relative_url();
494+
let unallowed_absolute = !is_relative_url
495+
&& !this
496+
.allowed_hosts
497+
.allows(&url::Url::parse(&request.request.uri().to_string()).unwrap());
498+
if unallowed_relative || unallowed_absolute {
499+
tracing::log::error!("Destination not allowed: {}", request.request.uri());
500+
let host = if unallowed_absolute {
501+
// Safe to unwrap because absolute urls have a host by definition.
502+
let host = request.request.uri().authority().map(|a| a.host()).unwrap();
503+
terminal::warn!(
504+
"A component tried to make a HTTP request to non-allowed host '{host}'."
505+
);
506+
host
507+
} else {
508+
terminal::warn!("A component tried to make a HTTP request to the same component but it does not have permission.");
509+
"self"
510+
};
511+
eprintln!("To allow requests, add 'allowed_http_hosts = [\"{}\"]' to the manifest component section.", host);
512+
anyhow::bail!("destination-not-allowed (error 1)")
513+
}
514+
490515
wasmtime_wasi_http::types::default_send_request(data, request)
491516
}
492517
}

0 commit comments

Comments
 (0)