diff --git a/Cargo.lock b/Cargo.lock index 07867c5bd5..49f847a407 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -151,7 +151,7 @@ dependencies = [ "tokio-util 0.7.1", "tonic", "tower", - "tower-http 0.2.5", + "tower-http", "tower-service", "tower-test", "tracing", @@ -210,6 +210,7 @@ dependencies = [ "lazy_static", "lru", "miette", + "mime", "mockall", "moka", "multimap", @@ -445,7 +446,7 @@ dependencies = [ "sync_wrapper", "tokio", "tower", - "tower-http 0.3.0", + "tower-http", "tower-layer", "tower-service", ] @@ -4860,28 +4861,9 @@ dependencies = [ [[package]] name = "tower-http" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aba3f3efabf7fb41fae8534fc20a817013dd1c12cb45441efb6c82e6556b4cd8" -dependencies = [ - "bitflags", - "bytes", - "futures-core", - "futures-util", - "http", - "http-body", - "http-range-header", - "pin-project-lite", - "tower-layer", - "tower-service", - "tracing", -] - -[[package]] -name = "tower-http" -version = "0.3.0" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "79dd37121c38240c4b4fe6520332406218bbf876f2f690fe9e406020189366fd" +checksum = "7d342c6d58709c0a6d48d48dabbb62d4ef955cf5f0f3bbfd845838e7ae88dbae" dependencies = [ "bitflags", "bytes", @@ -4894,6 +4876,7 @@ dependencies = [ "tower", "tower-layer", "tower-service", + "tracing", ] [[package]] diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 14095f2d5f..c435c365e0 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -51,8 +51,26 @@ telemetry: endpoint: default ``` +### CSRF Protection is enabled by default [PR #1006](https://github.com/apollographql/router/pull/1006) +A [Cross-Site Request Forgery protection plugin](https://developer.mozilla.org/en-US/docs/Glossary/CSRF) is enabled by default. + +This means [simple requests](https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests) will be rejected from now on (they represent a security risk). + +The plugin can be customized as explained in the [CORS and CSRF example](https://github.com/apollographql/router/tree/main/examples/cors-and-csrf/custom-headers.router.yaml) + +### CORS default behavior update [PR #1006](https://github.com/apollographql/router/pull/1006) +The CORS allow_headers default behavior changes from: + - allow only `Content-Type`, `apollographql-client-name` and `apollographql-client-version` +to: + - mirror the received `access-control-request-headers` + +This change loosens the CORS related headers restrictions, so it shouldn't have any impact on your setup. + ## 🚀 Features ( :rocket: ) +### CSRF Protection [PR #1006](https://github.com/apollographql/router/pull/1006) +The router now embeds a CSRF protection plugin, which is enabled by default. Have a look at the [CORS and CSRF example](https://github.com/apollographql/router/tree/main/examples/cors-and-csrf/custom-headers.router.yaml) to learn how to customize it. [Documentation](https://www.apollographql.com/docs/router/configuration/cors/) will be updated soon! + ### helm chart now supports prometheus metrics [PR #1005](https://github.com/apollographql/router/pull/1005) The router has supported exporting prometheus metrics for a while. This change updates our helm chart to enable router deployment prometheus metrics. diff --git a/apollo-router-core/Cargo.toml b/apollo-router-core/Cargo.toml index c513b9ac89..9d58c2608a 100644 --- a/apollo-router-core/Cargo.toml +++ b/apollo-router-core/Cargo.toml @@ -60,6 +60,7 @@ tracing = "0.1.34" tracing-opentelemetry = "0.17.2" typed-builder = "0.10.0" urlencoding = "2.1.0" +mime = "0.3.16" [dev-dependencies] insta = "1.12.0" diff --git a/apollo-router-core/src/plugins/csrf.rs b/apollo-router-core/src/plugins/csrf.rs new file mode 100644 index 0000000000..82a5c863b3 --- /dev/null +++ b/apollo-router-core/src/plugins/csrf.rs @@ -0,0 +1,335 @@ +use crate::{register_plugin, Plugin, RouterRequest, RouterResponse, ServiceBuilderExt}; +use http::header; +use http::{HeaderMap, StatusCode}; +use schemars::JsonSchema; +use serde::Deserialize; +use std::ops::ControlFlow; +use tower::util::BoxService; +use tower::{BoxError, ServiceBuilder, ServiceExt}; + +#[derive(Deserialize, Debug, Clone, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct CSRFConfig { + /// The CSRF plugin is enabled by default; + /// set unsafe_disabled = true to disable the plugin behavior + /// Note that setting this to true is deemed unsafe https://developer.mozilla.org/en-US/docs/Glossary/CSRF + #[serde(default)] + unsafe_disabled: bool, + /// Override the headers to check for by setting + /// custom_headers + /// Note that if you set required_headers here, + /// you may also want to have a look at your `CORS` configuration, + /// and make sure you either: + /// - did not set any `allow_headers` list (so it defaults to `mirror_request`) + /// - added your required headers to the allow_headers list, as shown in the + /// `examples/cors-and-csrf/custom-headers.router.yaml` files. + #[serde(default = "apollo_custom_preflight_headers")] + required_headers: Vec, +} + +fn apollo_custom_preflight_headers() -> Vec { + vec![ + "x-apollo-operation-name".to_string(), + "apollo-require-preflight".to_string(), + ] +} + +impl Default for CSRFConfig { + fn default() -> Self { + Self { + unsafe_disabled: false, + required_headers: apollo_custom_preflight_headers(), + } + } +} + +static NON_PREFLIGHTED_CONTENT_TYPES: &[&str] = &[ + "application/x-www-form-urlencoded", + "multipart/form-data", + "text/plain", +]; + +/// The Csrf plugin makes sure any request received would have been preflighted if it was sent by a browser. +/// +/// Quoting the great apollo server comment: +/// https://github.com/apollographql/apollo-server/blob/12bf5fc8ef305caa6a8848e37f862d32dae5957f/packages/server/src/preventCsrf.ts#L26 +/// +/// We don't want random websites to be able to execute actual GraphQL operations +/// from a user's browser unless our CORS policy supports it. It's not good +/// enough just to ensure that the browser can't read the response from the +/// operation; we also want to prevent CSRF, where the attacker can cause side +/// effects with an operation or can measure the timing of a read operation. Our +/// goal is to ensure that we don't run the context function or execute the +/// GraphQL operation until the browser has evaluated the CORS policy, which +/// means we want all operations to be pre-flighted. We can do that by only +/// processing operations that have at least one header set that appears to be +/// manually set by the JS code rather than by the browser automatically. +/// +/// POST requests generally have a content-type `application/json`, which is +/// sufficient to trigger preflighting. So we take extra care with requests that +/// specify no content-type or that specify one of the three non-preflighted +/// content types. For those operations, we require (if this feature is enabled) +/// one of a set of specific headers to be set. By ensuring that every operation +/// either has a custom content-type or sets one of these headers, we know we +/// won't execute operations at the request of origins who our CORS policy will +/// block. +#[derive(Debug, Clone)] +pub struct Csrf { + config: CSRFConfig, +} + +#[async_trait::async_trait] +impl Plugin for Csrf { + type Config = CSRFConfig; + + async fn new(config: Self::Config) -> Result { + Ok(Csrf { config }) + } + + fn router_service( + &mut self, + service: BoxService, + ) -> BoxService { + if !self.config.unsafe_disabled { + let required_headers = self.config.required_headers.clone(); + ServiceBuilder::new() + .checkpoint(move |req: RouterRequest| { + if is_preflighted(&req, required_headers.as_slice()) { + tracing::warn!("request is preflighted"); + Ok(ControlFlow::Continue(req)) + } else { + tracing::warn!("request is not preflighted"); + let error = crate::Error::builder().message( + format!( + "This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). \ + Please either specify a 'content-type' header (with a mime-type that is not one of {}) \ + or provide one of the following headers: {}", + NON_PREFLIGHTED_CONTENT_TYPES.join(", "), + required_headers.join(", ") + )).build(); + let res = RouterResponse::builder() + .error(error) + .status_code(StatusCode::BAD_REQUEST) + .context(req.context) + .build()?; + Ok(ControlFlow::Break(res)) + } + }) + .service(service) + .boxed() + } else { + service + } + } +} + +// A `preflighted` request is the opposite of a `simple` request. +// +// A simple request is a request that satisfies the three predicates below: +// - Has method `GET` `POST` or `HEAD` (which turns out to be the three methods our web server allows) +// - If content-type is set, it must be with a mime type that is application/x-www-form-urlencoded OR multipart/form-data OR text/plain +// - The only headers added by javascript code are part of the cors safelisted request headers (Accept,Accept-Language,Content-Language,Content-Type, and simple Range +// +// Given the first step is covered in our web browser, we'll take care of the two other steps below: +fn is_preflighted(req: &RouterRequest, required_headers: &[String]) -> bool { + let headers = req.originating_request.headers(); + content_type_requires_preflight(headers) + || recommended_header_is_provided(headers, required_headers) +} + +// Part two of the algorithm above: +// If content-type is set, it must be with a mime type that is application/x-www-form-urlencoded OR multipart/form-data OR text/plain +// The details of the algorithm are covered in the fetch specification https://fetch.spec.whatwg.org/#cors-safelisted-request-header +// +// content_type_requires_preflight will thus return true if +// the header value is !(`application/x-www-form-urlencoded` || `multipart/form-data` || `text/plain`) +fn content_type_requires_preflight(headers: &HeaderMap) -> bool { + let joined_content_type_header_value = if let Ok(combined_headers) = headers + .get_all(header::CONTENT_TYPE) + .iter() + .map(|header_value| { + // The mime type parser we're using is a bit askew, + // so we're going to perform a bit of trimming, and character replacement + // before we combine the header values + // https://github.com/apollographql/router/pull/1006#discussion_r869777439 + header_value + .to_str() + .map(|as_str| as_str.trim().replace('\u{0009}', "\u{0020}")) // replace tab with space + }) + .collect::, _>>() + { + // https://fetch.spec.whatwg.org/#concept-header-list-combine + combined_headers.join("\u{002C}\u{0020}") // ', ' + } else { + // We couldn't parse a header value, let's err on the side of caution here + return false; + }; + + if let Ok(mime_type) = joined_content_type_header_value.parse::() { + !NON_PREFLIGHTED_CONTENT_TYPES.contains(&mime_type.essence_str()) + } else { + // If we get here, this means that we couldn't parse the content-type value into + // a valid mime type... which would be safe enough for us to assume preflight was triggered if the `mime` + // crate followed the fetch specification, but it unfortunately doesn't (see comment above). + // + // Better safe than sorry, we will claim we don't have solid enough reasons + // to believe the request will have triggered preflight + false + } +} + +// Part three of the algorithm described above: +// The only headers added by javascript code are part of the cors safelisted request headers (Accept,Accept-Language,Content-Language,Content-Type, and simple Range +// +// It would be pretty hard for us to keep track of the headers browser send themselves, +// and the ones that were explicitely added by a javascript client (and have thus triggered preflight). +// so we will do the oposite: +// We hereby challenge any client to provide one of the required_headers. +// Browsers definitely will not add any "x-apollo-operation-name" or "apollo-require-preflight" to every request anytime soon, +// which means if the header is present, javascript has added it, and the browser will have triggered preflight. +fn recommended_header_is_provided(headers: &HeaderMap, required_headers: &[String]) -> bool { + required_headers + .iter() + .any(|header| headers.get(header).is_some()) +} + +register_plugin!("apollo", "csrf", Csrf); + +#[cfg(test)] +mod csrf_tests { + #[tokio::test] + async fn plugin_registered() { + crate::plugins() + .get("apollo.csrf") + .expect("Plugin not found") + .create_instance(&serde_json::json!({ "unsafe_disabled": true })) + .await + .unwrap(); + + crate::plugins() + .get("apollo.csrf") + .expect("Plugin not found") + .create_instance(&serde_json::json!({})) + .await + .unwrap(); + } + + use super::*; + use crate::{plugin::utils::test::MockRouterService, ResponseBody}; + use serde_json_bytes::json; + use tower::ServiceExt; + + #[tokio::test] + async fn it_lets_preflighted_request_pass_through() { + let config = CSRFConfig::default(); + let with_preflight_content_type = RouterRequest::fake_builder() + .headers( + [("content-type".into(), "application/json".into())] + .into_iter() + .collect(), + ) + .build() + .unwrap(); + assert_accepted(config.clone(), with_preflight_content_type).await; + + let with_preflight_header = RouterRequest::fake_builder() + .headers( + [("apollo-require-preflight".into(), "this-is-a-test".into())] + .into_iter() + .collect(), + ) + .build() + .unwrap(); + assert_accepted(config, with_preflight_header).await; + } + + #[tokio::test] + async fn it_rejects_non_preflighted_headers_request() { + let config = CSRFConfig::default(); + let non_preflighted_request = RouterRequest::fake_builder().build().unwrap(); + assert_rejected(config, non_preflighted_request).await + } + + #[tokio::test] + async fn it_rejects_non_preflighted_content_type_request() { + let config = CSRFConfig::default(); + let non_preflighted_request = RouterRequest::fake_builder() + .headers( + [("content-type".into(), "text/plain".into())] + .into_iter() + .collect(), + ) + .build() + .unwrap(); + assert_rejected(config.clone(), non_preflighted_request).await; + + let non_preflighted_request = RouterRequest::fake_builder() + .headers( + [("content-type".into(), "text/plain; charset=utf8".into())] + .into_iter() + .collect(), + ) + .build() + .unwrap(); + assert_rejected(config, non_preflighted_request).await; + } + + #[tokio::test] + async fn it_accepts_non_preflighted_headers_request_when_plugin_is_disabled() { + let config = CSRFConfig { + unsafe_disabled: true, + ..Default::default() + }; + let non_preflighted_request = RouterRequest::fake_builder().build().unwrap(); + assert_accepted(config, non_preflighted_request).await + } + + async fn assert_accepted(config: CSRFConfig, request: RouterRequest) { + let mut mock_service = MockRouterService::new(); + mock_service.expect_call().times(1).returning(move |_| { + RouterResponse::fake_builder() + .data(json!({ "test": 1234_u32 })) + .build() + }); + + let mock = mock_service.build(); + let service_stack = Csrf::new(config) + .await + .unwrap() + .router_service(mock.boxed()); + let res = service_stack.oneshot(request).await.unwrap(); + + match res.response.into_body() { + ResponseBody::GraphQL(res) => { + assert_eq!(res.data.unwrap(), json!({ "test": 1234_u32 })); + } + other => panic!("expected graphql response, found {:?}", other), + } + } + + async fn assert_rejected(config: CSRFConfig, request: RouterRequest) { + let mock = MockRouterService::new().build(); + let service_stack = Csrf::new(config) + .await + .unwrap() + .router_service(mock.boxed()); + let res = service_stack.oneshot(request).await.unwrap(); + + match res.response.into_body() { + ResponseBody::GraphQL(res) => { + assert_eq!( + 1, + res.errors.len(), + "expected one(1) error in the RouterResponse, found {}\n{:?}", + res.errors.len(), + res.errors + ); + assert_eq!(res.errors[0].message, "This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). \ + Please either specify a 'content-type' header \ + (with a mime-type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) \ + or provide one of the following headers: x-apollo-operation-name, apollo-require-preflight"); + } + other => panic!("expected graphql response, found {:?}", other), + } + } +} diff --git a/apollo-router-core/src/plugins/mod.rs b/apollo-router-core/src/plugins/mod.rs index 5dd14fbf83..7949b9810f 100644 --- a/apollo-router-core/src/plugins/mod.rs +++ b/apollo-router-core/src/plugins/mod.rs @@ -2,6 +2,7 @@ //! //! These plugins are compiled into the router and configured via YAML configuration. +pub mod csrf; mod forbid_mutations; mod headers; mod include_subgraph_errors; diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index c34c68653e..5d5811c4a8 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -82,7 +82,7 @@ tokio = { version = "1.17.0", features = ["full"] } tokio-util = { version = "0.7.1", features = ["net", "codec"] } tonic = { version = "0.6.2", features = ["transport", "tls"] } tower = { version = "0.4.12", features = ["full"] } -tower-http = { version = "0.2.5", features = ["trace", "cors"] } +tower-http = { version = "0.3.3", features = ["trace", "cors"] } tower-service = "0.3.1" tracing = "0.1.34" tracing-core = "0.1.26" diff --git a/apollo-router/src/axum_http_server_factory.rs b/apollo-router/src/axum_http_server_factory.rs index a1e89ffb1c..3447a1f5f8 100644 --- a/apollo-router/src/axum_http_server_factory.rs +++ b/apollo-router/src/axum_http_server_factory.rs @@ -1284,7 +1284,10 @@ mod tests { .header(ACCEPT, "text/html") .header(ORIGIN, "http://studio") .header(ACCESS_CONTROL_REQUEST_METHOD, "POST") - .header(ACCESS_CONTROL_REQUEST_HEADERS, "Content-type") + .header( + ACCESS_CONTROL_REQUEST_HEADERS, + "Content-type, x-an-other-test-header, apollo-require-preflight", + ) .send() .await .unwrap(); @@ -1295,11 +1298,12 @@ mod tests { vec!["http://studio"], "Incorrect access control allow origin header" ); + let headers = response.headers().get_all(ACCESS_CONTROL_ALLOW_HEADERS); assert_header_contains!( &response, ACCESS_CONTROL_ALLOW_HEADERS, - &["content-type"], - "Incorrect access control allow header header" + &["Content-type, x-an-other-test-header, apollo-require-preflight"], + "Incorrect access control allow header header {headers:?}" ); assert_header_contains!( &response, diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index dc096f5795..ea071a433f 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -20,7 +20,7 @@ use std::fmt; use std::net::SocketAddr; use std::str::FromStr; use thiserror::Error; -use tower_http::cors::{Any, CorsLayer, Origin}; +use tower_http::cors::{self, CorsLayer}; use typed_builder::TypedBuilder; /// Configuration error. @@ -85,6 +85,10 @@ pub struct Configuration { const APOLLO_PLUGIN_PREFIX: &str = "apollo."; +// Add your plugin to this list so it gets automatically set up if its not been provided a custom configuration. +// ! requires the plugin configuration to implement Default +const MANDATORY_APOLLO_PLUGINS: &[&str] = &["csrf"]; + fn default_listen() -> ListenAddr { SocketAddr::from_str("127.0.0.1:4000").unwrap().into() } @@ -105,12 +109,27 @@ impl Configuration { // Add all the apollo plugins for (plugin, config) in &self.apollo_plugins.plugins { - plugins.push(( - format!("{}{}", APOLLO_PLUGIN_PREFIX, plugin), - config.clone(), - )); + let plugin_full_name = format!("{}{}", APOLLO_PLUGIN_PREFIX, plugin); + tracing::debug!( + "adding plugin {} with user provided configuration", + plugin_full_name.as_str() + ); + plugins.push((plugin_full_name, config.clone())); } + // Add the mandatory apollo plugins with defaults, + // if a custom configuration hasn't been provided by the user + MANDATORY_APOLLO_PLUGINS.iter().for_each(|plugin_name| { + let plugin_full_name = format!("{}{}", APOLLO_PLUGIN_PREFIX, plugin_name); + if !plugins.iter().any(|p| p.0 == plugin_full_name) { + tracing::debug!( + "adding plugin {} with default configuration", + plugin_full_name.as_str() + ); + plugins.push((plugin_full_name, Value::Object(Map::new()))); + } + }); + // Add all the user plugins if let Some(config_map) = self.plugins.plugins.as_ref() { for (plugin, config) in config_map { @@ -321,10 +340,19 @@ pub struct Cors { pub allow_credentials: Option, /// The headers to allow. - /// Defaults to the required request header for Apollo Studio - #[serde(default = "default_cors_headers")] - #[builder(default_code = "default_cors_headers()")] - pub allow_headers: Vec, + /// If this is not set, we will default to + /// the `mirror_request` mode, which mirrors the received + /// `access-control-request-headers` preflight has sent. + /// + /// Note that if you set headers here, + /// you also want to have a look at your `CSRF` plugins configuration, + /// and make sure you either: + /// - accept `x-apollo-operation-name` AND / OR `apollo-require-preflight` + /// - defined `csrf` required headers in your yml configuration, as shown in the + /// `examples/cors-and-csrf/custom-headers.router.yaml` files. + #[serde(default)] + #[builder(default)] + pub allow_headers: Option>, #[serde(default)] #[builder(default)] @@ -348,14 +376,6 @@ fn default_origins() -> Vec { vec!["https://studio.apollographql.com".into()] } -fn default_cors_headers() -> Vec { - vec![ - "Content-Type".into(), - "apollographql-client-name".into(), - "apollographql-client-version".into(), - ] -} - fn default_cors_methods() -> Vec { vec!["GET".into(), "POST".into(), "OPTIONS".into()] } @@ -380,41 +400,50 @@ impl Default for Server { impl Cors { pub fn into_layer(self) -> CorsLayer { - let cors = - CorsLayer::new() - .allow_credentials(self.allow_credentials.unwrap_or_default()) - .allow_headers(self.allow_headers.iter().filter_map(|header| { - header - .parse() - .map_err(|_| tracing::error!("header name '{header}' is not valid")) - .ok() - })) - .expose_headers(self.expose_headers.unwrap_or_default().iter().filter_map( - |header| { + let allow_headers = if let Some(headers_to_allow) = self.allow_headers { + cors::AllowHeaders::list(headers_to_allow.iter().filter_map(|header| { + header + .parse() + .map_err(|_| tracing::error!("header name '{header}' is not valid")) + .ok() + })) + } else { + cors::AllowHeaders::mirror_request() + }; + let cors = CorsLayer::new() + .allow_credentials(self.allow_credentials.unwrap_or_default()) + .allow_headers(allow_headers) + .expose_headers(cors::ExposeHeaders::list( + self.expose_headers + .unwrap_or_default() + .iter() + .filter_map(|header| { header .parse() .map_err(|_| tracing::error!("header name '{header}' is not valid")) .ok() - }, - )) - .allow_methods(self.methods.iter().filter_map(|method| { + }), + )) + .allow_methods(cors::AllowMethods::list(self.methods.iter().filter_map( + |method| { method .parse() .map_err(|_| tracing::error!("method '{method}' is not valid")) .ok() - })); + }, + ))); if self.allow_any_origin.unwrap_or_default() { - cors.allow_origin(Any) + cors.allow_origin(cors::Any) } else { - cors.allow_origin(Origin::list(self.origins.into_iter().filter_map( - |origin| { + cors.allow_origin(cors::AllowOrigin::list( + self.origins.into_iter().filter_map(|origin| { origin .parse() .map_err(|_| tracing::error!("origin '{origin}' is not valid")) .ok() - }, - ))) + }), + )) } } } @@ -759,6 +788,11 @@ mod tests { !cors.allow_any_origin.unwrap_or_default(), "Allow any origin should be disabled by default" ); + + assert!( + cors.allow_headers.is_none(), + "No allow_headers list should be present by default" + ); } #[test] diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index ba54f0e503..481c3ea502 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -9,6 +9,28 @@ expression: "&schema" "description": "The configuration for the router. Currently maintains a mapping of subgraphs.", "type": "object", "properties": { + "csrf": { + "type": "object", + "properties": { + "required_headers": { + "description": "Override the headers to check for by setting custom_headers Note that if you set required_headers here, you may also want to have a look at your `CORS` configuration, and make sure you either: - did not set any `allow_headers` list (so it defaults to `mirror_request`) - added your required headers to the allow_headers list, as shown in the `examples/cors-and-csrf/custom-headers.router.yaml` files.", + "default": [ + "x-apollo-operation-name", + "apollo-require-preflight" + ], + "type": "array", + "items": { + "type": "string" + } + }, + "unsafe_disabled": { + "description": "The CSRF plugin is enabled by default; set unsafe_disabled = true to disable the plugin behavior Note that setting this to true is deemed unsafe https://developer.mozilla.org/en-US/docs/Glossary/CSRF", + "default": false, + "type": "boolean" + } + }, + "additionalProperties": false + }, "forbid_mutations": { "type": "boolean" }, @@ -348,16 +370,13 @@ expression: "&schema" "nullable": true }, "allow_headers": { - "description": "The headers to allow. Defaults to the required request header for Apollo Studio", - "default": [ - "Content-Type", - "apollographql-client-name", - "apollographql-client-version" - ], + "description": "The headers to allow. If this is not set, we will default to the `mirror_request` mode, which mirrors the received `access-control-request-headers` preflight has sent.\n\nNote that if you set headers here, you also want to have a look at your `CSRF` plugins configuration, and make sure you either: - accept `x-apollo-operation-name` AND / OR `apollo-require-preflight` - defined `csrf` required headers in your yml configuration, as shown in the `examples/cors-and-csrf/custom-headers.router.yaml` files.", + "default": null, "type": "array", "items": { "type": "string" - } + }, + "nullable": true }, "expose_headers": { "description": "Which response headers should be made available to scripts running in the browser, in response to a cross-origin request.", diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index a06ebff9a5..7e4faa996b 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -5,8 +5,9 @@ use apollo_router::plugins::telemetry::config::Tracing; use apollo_router::plugins::telemetry::{self, apollo, Telemetry}; use apollo_router_core::{ - http_compat, prelude::*, Object, PluggableRouterServiceBuilder, Plugin, ResponseBody, - RouterRequest, RouterResponse, Schema, SubgraphRequest, TowerSubgraphService, ValueExt, + http_compat, plugins::csrf, prelude::*, Object, PluggableRouterServiceBuilder, Plugin, + ResponseBody, RouterRequest, RouterResponse, Schema, SubgraphRequest, TowerSubgraphService, + ValueExt, }; use http::Method; use maplit::hashmap; @@ -44,12 +45,14 @@ macro_rules! assert_federated_response { }; let originating_request = http_compat::Request::fake_builder().method(Method::POST) + // otherwise the query would be a simple one, + // and CSRF protection would reject it + .header("content-type", "application/json") .body(request) .build().expect("expecting valid originating request"); let (actual, registry) = query_rust(originating_request.into()).await; - tracing::debug!("query:\n{}\n", $query); assert!( @@ -110,6 +113,7 @@ async fn api_schema_hides_field() { let originating_request = http_compat::Request::fake_builder() .method(Method::POST) + .header("content-type", "application/json") .body(request) .build() .expect("expecting valid request"); @@ -198,6 +202,7 @@ async fn queries_should_work_over_get() { let originating_request = http_compat::Request::fake_builder() .body(request) + .header("content-type", "application/json") .build() .expect("expecting valid request"); @@ -207,6 +212,47 @@ async fn queries_should_work_over_get() { assert_eq!(registry.totals(), expected_service_hits); } +#[tokio::test(flavor = "multi_thread")] +async fn simple_queries_should_not_work() { + let expected_error =apollo_router_core::Error { + message :"This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). \ + Please either specify a 'content-type' header \ + (with a mime-type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) \ + or provide one of the following headers: x-apollo-operation-name, apollo-require-preflight".to_string(), + ..Default::default() + }; + + let request = graphql::Request::builder() + .query(Some( + r#"{ topProducts { upc name reviews {id product { name } author { id name } } } }"# + .to_string(), + )) + .variables(Arc::new( + vec![ + ("topProductsFirst".into(), 2.into()), + ("reviewsForAuthorAuthorId".into(), 1.into()), + ] + .into_iter() + .collect(), + )) + .build(); + + let originating_request = http_compat::Request::fake_builder() + .body(request) + .build() + .expect("expecting valid request"); + + let (actual, registry) = query_rust(originating_request.into()).await; + + assert_eq!( + 1, + actual.errors.len(), + "CSRF should have rejected this query" + ); + assert_eq!(expected_error, actual.errors[0]); + assert_eq!(registry.totals(), hashmap! {}); +} + #[tokio::test(flavor = "multi_thread")] async fn queries_should_work_over_post() { let request = graphql::Request::builder() @@ -232,6 +278,7 @@ async fn queries_should_work_over_post() { let http_request = http_compat::Request::fake_builder() .method(Method::POST) + .header("content-type", "application/json") .body(request) .build() .expect("expecting valid request"); @@ -263,6 +310,7 @@ async fn service_errors_should_be_propagated() { let originating_request = http_compat::Request::fake_builder() .body(request) + .header("content-type", "application/json") .build() .expect("expecting valid request"); @@ -306,6 +354,7 @@ async fn mutation_should_not_work_over_get() { let originating_request = http_compat::Request::fake_builder() .body(request) + .header("content-type", "application/json") .build() .expect("expecting valid request"); @@ -351,6 +400,7 @@ async fn mutation_should_work_over_post() { let http_request = http_compat::Request::fake_builder() .method(Method::POST) + .header("content-type", "application/json") .body(request) .build() .expect("expecting valid request"); @@ -402,6 +452,7 @@ async fn automated_persisted_queries() { let originating_request = http_compat::Request::fake_builder() .body(apq_only_request) + .header("content-type", "application/json") .build() .expect("expecting valid request"); @@ -425,6 +476,7 @@ async fn automated_persisted_queries() { let originating_request = http_compat::Request::fake_builder() .body(apq_request_with_query) + .header("content-type", "application/json") .build() .expect("expecting valid request"); @@ -443,6 +495,7 @@ async fn automated_persisted_queries() { let originating_request = http_compat::Request::fake_builder() .body(apq_only_request) + .header("content-type", "application/json") .build() .expect("expecting valid request"); @@ -501,6 +554,7 @@ async fn missing_variables() { let originating_request = http_compat::Request::fake_builder() .method(Method::POST) + .header("content-type", "application/json") .body(request) .build() .expect("expecting valid request"); @@ -568,7 +622,10 @@ async fn setup_router_and_registry() -> ( }) .await .unwrap(); - builder = builder.with_dyn_plugin("apollo.telemetry".to_string(), Box::new(telemetry_plugin)); + let csrf_plugin = csrf::Csrf::new(Default::default()).await.unwrap(); + builder = builder + .with_dyn_plugin("apollo.telemetry".to_string(), Box::new(telemetry_plugin)) + .with_dyn_plugin("apollo.csrf".to_string(), Box::new(csrf_plugin)); for (name, _url) in subgraphs { let cloned_counter = counting_registry.clone(); let cloned_name = name.clone(); diff --git a/examples/cors-and-csrf/custom-headers.router.yaml b/examples/cors-and-csrf/custom-headers.router.yaml new file mode 100644 index 0000000000..55c8dd234f --- /dev/null +++ b/examples/cors-and-csrf/custom-headers.router.yaml @@ -0,0 +1,25 @@ +server: + cors: + # adding the headers in the CSRF section + # and not here would fail the OPTIONS preflight + # you can: + # - either add them here to be explicit + # - or not set the `allow_headers` key at all. + # the default behavior is `mirror_request`, + # which mirrors the received + # `access-control-request-headers` preflight has sent + allow_headers: + - x-my-custom-required-header + - x-and-an-other-required-header + - content-type # and many more! +csrf: + # you could set unsafe_disabled: true, but That wouldn't be safe https://developer.mozilla.org/en-US/docs/Glossary/CSRF + + # pick required headers that are custom enough + # to make sure browsers would need to have added them with javascript, + # which would trigger a preflight https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#what_requests_use_cors + # `content-type` is not a safe idea for example. + required_headers: + - x-my-custom-required-header # we will look for presence of this header... + - x-and-an-other-required-header # ...or that one if a request was made with a `simple` content type https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS#simple_requests + diff --git a/licenses.html b/licenses.html index 5fa2d7573f..35b85b1ef6 100644 --- a/licenses.html +++ b/licenses.html @@ -11307,7 +11307,6 @@

MIT License

Used by:

Copyright (c) 2019-2021 Tower Contributors