From a3704b971ebcfd5af6eb8067b1ebf9d7920839cf Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 22 Oct 2020 04:47:27 +0000 Subject: [PATCH] Support pre-flight CORS check (#1772) ## Issue Addressed - Resolves #1766 ## Proposed Changes - Use the `warp::filters::cors` filter instead of our work-around. ## Additional Info It's not trivial to enable/disable `cors` using `warp`, since using `routes.with(cors)` changes the type of `routes`. This makes it difficult to apply/not apply cors at runtime. My solution has been to *always* use the `warp::filters::cors` wrapper but when cors should be disabled, just pass the HTTP server listen address as the only permissible origin. --- Cargo.lock | 4 +- beacon_node/http_api/Cargo.toml | 2 +- beacon_node/http_api/src/lib.rs | 17 +++++-- beacon_node/http_metrics/Cargo.toml | 2 +- beacon_node/http_metrics/src/lib.rs | 17 +++++-- beacon_node/src/cli.rs | 13 +++-- common/warp_utils/Cargo.toml | 3 +- common/warp_utils/src/cors.rs | 76 ++++++++++++++++++++++++++++ common/warp_utils/src/lib.rs | 2 +- common/warp_utils/src/reply.rs | 15 ------ validator_client/Cargo.toml | 2 +- validator_client/src/cli.rs | 6 ++- validator_client/src/http_api/mod.rs | 17 +++++-- 13 files changed, 138 insertions(+), 38 deletions(-) create mode 100644 common/warp_utils/src/cors.rs delete mode 100644 common/warp_utils/src/reply.rs diff --git a/Cargo.lock b/Cargo.lock index 4a974b18b4..a1c923f770 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6534,8 +6534,7 @@ dependencies = [ [[package]] name = "warp" version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f41be6df54c97904af01aa23e613d4521eed7ab23537cede692d4058f6449407" +source = "git+https://github.com/paulhauner/warp?branch=cors-wildcard#a7685b76d70c3e5628e31d60aee510acec3c5c30" dependencies = [ "bytes 0.5.6", "futures 0.3.6", @@ -6565,6 +6564,7 @@ version = "0.1.0" dependencies = [ "beacon_chain", "eth2", + "headers", "safe_arith", "serde", "state_processing", diff --git a/beacon_node/http_api/Cargo.toml b/beacon_node/http_api/Cargo.toml index 608864a9d2..7b2cd83a78 100644 --- a/beacon_node/http_api/Cargo.toml +++ b/beacon_node/http_api/Cargo.toml @@ -5,7 +5,7 @@ authors = ["Paul Hauner "] edition = "2018" [dependencies] -warp = "0.2.5" +warp = { git = "https://github.com/paulhauner/warp", branch = "cors-wildcard" } serde = { version = "1.0.116", features = ["derive"] } tokio = { version = "0.2.22", features = ["macros"] } parking_lot = "0.11.0" diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index c608821b14..487880a938 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -211,7 +211,19 @@ pub fn serve( ) -> Result<(SocketAddr, impl Future), Error> { let config = ctx.config.clone(); let log = ctx.log.clone(); - let allow_origin = config.allow_origin.clone(); + + // Configure CORS. + let cors_builder = { + let builder = warp::cors() + .allow_methods(vec!["GET", "POST"]) + .allow_headers(vec!["Content-Type"]); + + warp_utils::cors::set_builder_origins( + builder, + config.allow_origin.as_deref(), + (config.listen_addr, config.listen_port), + )? + }; // Sanity check. if !config.enabled { @@ -1827,8 +1839,7 @@ pub fn serve( .with(prometheus_metrics()) // Add a `Server` header. .map(|reply| warp::reply::with_header(reply, "Server", &version_with_platform())) - // Maybe add some CORS headers. - .map(move |reply| warp_utils::reply::maybe_cors(reply, allow_origin.as_ref())); + .with(cors_builder.build()); let (listening_socket, server) = warp::serve(routes).try_bind_with_graceful_shutdown( SocketAddrV4::new(config.listen_addr, config.listen_port), diff --git a/beacon_node/http_metrics/Cargo.toml b/beacon_node/http_metrics/Cargo.toml index 1b9197917a..5b1715e689 100644 --- a/beacon_node/http_metrics/Cargo.toml +++ b/beacon_node/http_metrics/Cargo.toml @@ -8,7 +8,7 @@ edition = "2018" [dependencies] prometheus = "0.10.0" -warp = "0.2.5" +warp = { git = "https://github.com/paulhauner/warp", branch = "cors-wildcard" } serde = { version = "1.0.116", features = ["derive"] } slog = "2.5.2" beacon_chain = { path = "../beacon_chain" } diff --git a/beacon_node/http_metrics/src/lib.rs b/beacon_node/http_metrics/src/lib.rs index 37eac82bda..ce59578b9e 100644 --- a/beacon_node/http_metrics/src/lib.rs +++ b/beacon_node/http_metrics/src/lib.rs @@ -87,7 +87,19 @@ pub fn serve( ) -> Result<(SocketAddr, impl Future), Error> { let config = &ctx.config; let log = ctx.log.clone(); - let allow_origin = config.allow_origin.clone(); + + // Configure CORS. + let cors_builder = { + let builder = warp::cors() + .allow_method("GET") + .allow_headers(vec!["Content-Type"]); + + warp_utils::cors::set_builder_origins( + builder, + config.allow_origin.as_deref(), + (config.listen_addr, config.listen_port), + )? + }; // Sanity check. if !config.enabled { @@ -115,8 +127,7 @@ pub fn serve( }) // Add a `Server` header. .map(|reply| warp::reply::with_header(reply, "Server", &version_with_platform())) - // Maybe add some CORS headers. - .map(move |reply| warp_utils::reply::maybe_cors(reply, allow_origin.as_ref())); + .with(cors_builder.build()); let (listening_socket, server) = warp::serve(routes).try_bind_with_graceful_shutdown( SocketAddrV4::new(config.listen_addr, config.listen_port), diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index cb7ea121dc..525516687b 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -171,8 +171,10 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("http-allow-origin") .long("http-allow-origin") .value_name("ORIGIN") - .help("Set the value of the Access-Control-Allow-Origin response HTTP header. Use * to allow any origin (not recommended in production)") - .default_value("") + .help("Set the value of the Access-Control-Allow-Origin response HTTP header. \ + Use * to allow any origin (not recommended in production). \ + If no value is supplied, the CORS allowed origin is set to the listen \ + address of this server (e.g., http://localhost:5052).") .takes_value(true), ) /* Prometheus metrics HTTP server related arguments */ @@ -202,9 +204,10 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("metrics-allow-origin") .long("metrics-allow-origin") .value_name("ORIGIN") - .help("Set the value of the Access-Control-Allow-Origin response HTTP header for the Prometheus metrics HTTP server. \ - Use * to allow any origin (not recommended in production)") - .default_value("") + .help("Set the value of the Access-Control-Allow-Origin response HTTP header. \ + Use * to allow any origin (not recommended in production). \ + If no value is supplied, the CORS allowed origin is set to the listen \ + address of this server (e.g., http://localhost:5054).") .takes_value(true), ) /* Websocket related arguments */ diff --git a/common/warp_utils/Cargo.toml b/common/warp_utils/Cargo.toml index dd029e3c2a..1fc88ababd 100644 --- a/common/warp_utils/Cargo.toml +++ b/common/warp_utils/Cargo.toml @@ -7,7 +7,7 @@ edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -warp = "0.2.5" +warp = { git = "https://github.com/paulhauner/warp", branch = "cors-wildcard" } eth2 = { path = "../eth2" } types = { path = "../../consensus/types" } beacon_chain = { path = "../../beacon_node/beacon_chain" } @@ -15,3 +15,4 @@ state_processing = { path = "../../consensus/state_processing" } safe_arith = { path = "../../consensus/safe_arith" } serde = { version = "1.0.116", features = ["derive"] } tokio = { version = "0.2.22", features = ["sync"] } +headers = "0.3.2" diff --git a/common/warp_utils/src/cors.rs b/common/warp_utils/src/cors.rs new file mode 100644 index 0000000000..006424b505 --- /dev/null +++ b/common/warp_utils/src/cors.rs @@ -0,0 +1,76 @@ +use std::net::Ipv4Addr; +use warp::filters::cors::Builder; + +/// Configure a `cors::Builder`. +/// +/// If `allow_origin.is_none()` the `default_origin` is used. +pub fn set_builder_origins( + builder: Builder, + allow_origin: Option<&str>, + default_origin: (Ipv4Addr, u16), +) -> Result { + if let Some(allow_origin) = allow_origin { + let origins = allow_origin + .split(',') + .map(|s| verify_cors_origin_str(s).map(|_| s)) + .collect::, _>>()?; + Ok(builder.allow_origins(origins)) + } else { + let origin = format!("http://{}:{}", default_origin.0, default_origin.1); + verify_cors_origin_str(&origin)?; + + Ok(builder.allow_origin(origin.as_str())) + } +} + +/// Verify that `s` can be used as a CORS origin. +/// +/// ## Notes +/// +/// We need this function since `warp` will panic if provided an invalid origin. The verification +/// code is taken from here: +/// +/// https://github.com/seanmonstar/warp/blob/3d1760c6ca35ce2d03dee0562259d0320e9face3/src/filters/cors.rs#L616 +/// +/// Ideally we should make a PR to `warp` to expose this behaviour, however we defer this for a +/// later time. The impact of a false-positive on this function is fairly limited, since only +/// trusted users should be setting CORS origins. +fn verify_cors_origin_str(s: &str) -> Result<(), String> { + // Always the wildcard origin. + if s == "*" { + return Ok(()); + } + + let mut parts = s.splitn(2, "://"); + let scheme = parts + .next() + .ok_or_else(|| format!("{} is missing a scheme", s))?; + let rest = parts + .next() + .ok_or_else(|| format!("{} is missing the part following the scheme", s))?; + + headers::Origin::try_from_parts(scheme, rest, None) + .map_err(|e| format!("Unable to parse {}: {}", s, e)) + .map(|_| ()) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn valid_origins() { + verify_cors_origin_str("*").unwrap(); + verify_cors_origin_str("http://127.0.0.1").unwrap(); + verify_cors_origin_str("http://localhost").unwrap(); + verify_cors_origin_str("http://127.0.0.1:8000").unwrap(); + verify_cors_origin_str("http://localhost:8000").unwrap(); + } + + #[test] + fn invalid_origins() { + verify_cors_origin_str(".*").unwrap_err(); + verify_cors_origin_str("127.0.0.1").unwrap_err(); + verify_cors_origin_str("localhost").unwrap_err(); + } +} diff --git a/common/warp_utils/src/lib.rs b/common/warp_utils/src/lib.rs index ba02273e63..b645d85dc2 100644 --- a/common/warp_utils/src/lib.rs +++ b/common/warp_utils/src/lib.rs @@ -1,6 +1,6 @@ //! This crate contains functions that are common across multiple `warp` HTTP servers in the //! Lighthouse project. E.g., the `http_api` and `http_metrics` crates. +pub mod cors; pub mod reject; -pub mod reply; pub mod task; diff --git a/common/warp_utils/src/reply.rs b/common/warp_utils/src/reply.rs deleted file mode 100644 index dcec6214f0..0000000000 --- a/common/warp_utils/src/reply.rs +++ /dev/null @@ -1,15 +0,0 @@ -/// Add CORS headers to `reply` only if `allow_origin.is_some()`. -pub fn maybe_cors( - reply: T, - allow_origin: Option<&String>, -) -> Box { - if let Some(allow_origin) = allow_origin { - Box::new(warp::reply::with_header( - reply, - "Access-Control-Allow-Origin", - allow_origin, - )) - } else { - Box::new(reply) - } -} diff --git a/validator_client/Cargo.toml b/validator_client/Cargo.toml index 873faca162..055d0fe357 100644 --- a/validator_client/Cargo.toml +++ b/validator_client/Cargo.toml @@ -52,7 +52,7 @@ eth2_keystore = { path = "../crypto/eth2_keystore" } account_utils = { path = "../common/account_utils" } lighthouse_version = { path = "../common/lighthouse_version" } warp_utils = { path = "../common/warp_utils" } -warp = "0.2.5" +warp = { git = "https://github.com/paulhauner/warp", branch = "cors-wildcard" } hyper = "0.13.8" serde_utils = { path = "../consensus/serde_utils" } libsecp256k1 = "0.3.5" diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index c789798803..362149c2bf 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -131,8 +131,10 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("http-allow-origin") .long("http-allow-origin") .value_name("ORIGIN") - .help("Set the value of the Access-Control-Allow-Origin response HTTP header. Use * to allow any origin (not recommended in production)") - .default_value("") + .help("Set the value of the Access-Control-Allow-Origin response HTTP header. \ + Use * to allow any origin (not recommended in production). \ + If no value is supplied, the CORS allowed origin is set to the listen \ + address of this server (e.g., http://localhost:5062).") .takes_value(true), ) } diff --git a/validator_client/src/http_api/mod.rs b/validator_client/src/http_api/mod.rs index 7e0d387d26..6dde635c94 100644 --- a/validator_client/src/http_api/mod.rs +++ b/validator_client/src/http_api/mod.rs @@ -100,7 +100,19 @@ pub fn serve( ) -> Result<(SocketAddr, impl Future), Error> { let config = &ctx.config; let log = ctx.log.clone(); - let allow_origin = config.allow_origin.clone(); + + // Configure CORS. + let cors_builder = { + let builder = warp::cors() + .allow_methods(vec!["GET", "POST", "PATCH"]) + .allow_headers(vec!["Content-Type", "Authorization"]); + + warp_utils::cors::set_builder_origins( + builder, + config.allow_origin.as_deref(), + (config.listen_addr, config.listen_port), + )? + }; // Sanity check. if !config.enabled { @@ -428,8 +440,7 @@ pub fn serve( .recover(warp_utils::reject::handle_rejection) // Add a `Server` header. .map(|reply| warp::reply::with_header(reply, "Server", &version_with_platform())) - // Maybe add some CORS headers. - .map(move |reply| warp_utils::reply::maybe_cors(reply, allow_origin.as_ref())); + .with(cors_builder.build()); let (listening_socket, server) = warp::serve(routes).try_bind_with_graceful_shutdown( SocketAddrV4::new(config.listen_addr, config.listen_port),