Skip to content

Commit

Permalink
Merge pull request #1710 from michelleN/outboundhttp
Browse files Browse the repository at this point in the history
  • Loading branch information
michelleN authored Sep 5, 2023
2 parents 4d178a1 + c233253 commit 089e135
Show file tree
Hide file tree
Showing 33 changed files with 406 additions and 520 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 29 additions & 0 deletions crates/outbound-http/src/allowed_http_hosts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@ impl AllowedHttpHosts {
Self::AllowSpecific(hosts) => hosts.iter().any(|h| h.allow(url)),
}
}

pub fn allow_relative_url(&self) -> bool {
match self {
Self::AllowAll => true,
Self::AllowSpecific(hosts) => hosts.contains(&AllowedHttpHost::host("self")),
}
}
}

/// An HTTP host allow-list entry.
Expand Down Expand Up @@ -214,6 +221,14 @@ mod test {
);
}

#[test]
fn test_allowed_hosts_accepts_self() {
assert_eq!(
AllowedHttpHost::host("self"),
parse_allowed_http_host("self").unwrap()
);
}

#[test]
fn test_allowed_hosts_accepts_localhost_addresses() {
assert_eq!(
Expand Down Expand Up @@ -303,4 +318,18 @@ mod test {
assert!(!allowed.allow(&Url::parse("http://example.com/").unwrap()));
assert!(!allowed.allow(&Url::parse("http://google.com/").unwrap()));
}

#[test]
fn test_allowed_hosts_allow_relative_url() {
let allowed =
parse_allowed_http_hosts(&to_vec_owned(&["self", "http://example.com:8383"])).unwrap();
assert!(allowed.allow_relative_url());

let not_allowed =
parse_allowed_http_hosts(&to_vec_owned(&["http://example.com:8383"])).unwrap();
assert!(!not_allowed.allow_relative_url());

let allow_all = parse_allowed_http_hosts(&to_vec_owned(&["insecure:allow-all"])).unwrap();
assert!(allow_all.allow_relative_url());
}
}
24 changes: 20 additions & 4 deletions crates/outbound-http/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,23 @@ pub const ALLOWED_HTTP_HOSTS_KEY: MetadataKey<Vec<String>> = MetadataKey::new("a
pub struct OutboundHttp {
/// List of hosts guest modules are allowed to make requests to.
pub allowed_hosts: AllowedHttpHosts,
/// During an incoming HTTP request, origin is set to the host of that incoming HTTP request.
/// This is used to direct outbound requests to the same host when allowed.
pub origin: String,
client: Option<Client>,
}

impl OutboundHttp {
/// Check if guest module is allowed to send request to URL, based on the list of
/// allowed hosts defined by the runtime. If the list of allowed hosts contains
/// allowed hosts defined by the runtime. If the url passed in is a relative path,
/// only allow if allowed_hosts contains `self`. If the list of allowed hosts contains
/// `insecure:allow-all`, then all hosts are allowed.
/// If `None` is passed, the guest module is not allowed to send the request.
fn is_allowed(&self, url: &str) -> Result<bool, HttpError> {
fn is_allowed(&mut self, url: &str) -> Result<bool, HttpError> {
if url.starts_with('/') {
return Ok(self.allowed_hosts.allow_relative_url());
}

let url = Url::parse(url).map_err(|_| HttpError::InvalidUrl)?;
Ok(self.allowed_hosts.allow(&url))
}
Expand All @@ -49,7 +57,15 @@ impl outbound_http::Host for OutboundHttp {
}

let method = method_from(req.method);
let url = Url::parse(&req.uri).map_err(|_| HttpError::InvalidUrl)?;

let abs_url = if req.uri.starts_with('/') {
format!("{}{}", self.origin, req.uri)
} else {
req.uri.clone()
};

let req_url = Url::parse(&abs_url).map_err(|_| HttpError::InvalidUrl)?;

let headers = request_headers(req.headers).map_err(|_| HttpError::RuntimeError)?;
let body = req.body.unwrap_or_default().to_vec();

Expand All @@ -62,7 +78,7 @@ impl outbound_http::Host for OutboundHttp {
let client = self.client.get_or_insert_with(Default::default);

let resp = client
.request(method, url)
.request(method, req_url)
.headers(headers)
.body(body)
.send()
Expand Down
1 change: 1 addition & 0 deletions crates/trigger-http/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ futures-util = "0.3.8"
http = "0.2"
hyper = { version = "0.14", features = ["full"] }
indexmap = "1"
outbound-http = { path = "../outbound-http" }
percent-encoding = "2"
rustls-pemfile = "0.3.0"
serde = { version = "1.0", features = ["derive"] }
Expand Down
27 changes: 26 additions & 1 deletion crates/trigger-http/src/spin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ use crate::{HttpExecutor, HttpTrigger, Store};
use anyhow::{anyhow, Result};
use async_trait::async_trait;
use hyper::{Body, Request, Response};
use outbound_http::OutboundHttpComponent;
use spin_core::Instance;
use spin_trigger::{EitherInstance, TriggerAppEngine};
use spin_world::http_types;
use std::sync::Arc;

#[derive(Clone)]
pub struct SpinHttpExecutor;
Expand All @@ -27,11 +29,13 @@ impl HttpExecutor for SpinHttpExecutor {
component_id
);

let (instance, store) = engine.prepare_instance(component_id).await?;
let (instance, mut store) = engine.prepare_instance(component_id).await?;
let EitherInstance::Component(instance) = instance else {
unreachable!()
};

set_http_origin_from_request(&mut store, engine, &req);

let resp = Self::execute_impl(store, instance, base, raw_route, req, client_addr)
.await
.map_err(contextualise_err)?;
Expand Down Expand Up @@ -182,6 +186,27 @@ impl SpinHttpExecutor {
}
}

fn set_http_origin_from_request(
store: &mut Store,
engine: &TriggerAppEngine<HttpTrigger>,
req: &Request<Body>,
) {
if let Some(authority) = req.uri().authority() {
if let Some(scheme) = req.uri().scheme_str() {
if let Some(outbound_http_handle) = engine
.engine
.find_host_component_handle::<Arc<OutboundHttpComponent>>()
{
let mut outbound_http_data = store
.host_components_data()
.get_or_insert(outbound_http_handle);

outbound_http_data.origin = format!("{}://{}", scheme, authority);
}
}
}
}

fn contextualise_err(e: anyhow::Error) -> anyhow::Error {
if e.to_string()
.contains("failed to find function export `canonical_abi_free`")
Expand Down
Loading

0 comments on commit 089e135

Please sign in to comment.