From d2365a911e7b07703cf8a16fc40f967bd68815ec Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Tue, 19 Nov 2024 11:30:22 +0200 Subject: [PATCH] Configuration Options for HTTP/1 Max Headers and Buffer Limits (#6194) Co-authored-by: Jesse Rosenberger Co-authored-by: Ivan Goncharov Co-authored-by: Simon Sapin --- .changesets/feat_max_headers.md | 19 +++ .circleci/config.yml | 8 +- Cargo.lock | 6 +- Cargo.toml | 3 + apollo-router/Cargo.toml | 6 +- .../axum_factory/axum_http_server_factory.rs | 16 ++ apollo-router/src/axum_factory/listeners.rs | 15 +- apollo-router/src/configuration/mod.rs | 8 + ...nfiguration__tests__schema_generation.snap | 15 ++ apollo-router/src/configuration/tests.rs | 5 + apollo-router/src/plugins/limits/mod.rs | 16 ++ apollo-router/tests/integration/mod.rs | 1 + apollo-router/tests/integration/supergraph.rs | 140 ++++++++++++++++++ .../source/reference/router/configuration.mdx | 20 +++ .../routing/security/request-limits.mdx | 20 +++ xtask/src/commands/dist.rs | 16 +- 16 files changed, 294 insertions(+), 20 deletions(-) create mode 100644 .changesets/feat_max_headers.md create mode 100644 apollo-router/tests/integration/supergraph.rs diff --git a/.changesets/feat_max_headers.md b/.changesets/feat_max_headers.md new file mode 100644 index 0000000000..30939d802a --- /dev/null +++ b/.changesets/feat_max_headers.md @@ -0,0 +1,19 @@ +### Configuration Options for HTTP/1 Max Headers and Buffer Limits ([PR #6194](https://github.com/apollographql/router/pull/6194)) + +This update introduces configuration options that allow you to adjust the maximum number of HTTP/1 request headers and the maximum buffer size allocated for headers. + +By default, the Router accepts HTTP/1 requests with up to 100 headers and allocates ~400kib of buffer space to store them. If you need to handle requests with more headers or require a different buffer size, you can now configure these limits in the Router's configuration file: +```yaml +limits: + http1_request_max_headers: 200 + http1_request_max_buf_size: 200kib +``` + +Note for Rust Crate Users: If you are using the Router as a Rust crate, the `http1_request_max_buf_size` option requires the `hyper_header_limits` feature and also necessitates using Apollo's fork of the Hyper crate until the [changes are merged upstream](https://github.com/hyperium/hyper/pull/3523). +You can include this fork by adding the following patch to your Cargo.toml file: +```toml +[patch.crates-io] +"hyper" = { git = "https://github.com/apollographql/hyper.git", tag = "header-customizations-20241108" } +``` + +By [@IvanGoncharov](https://github.com/IvanGoncharov) in https://github.com/apollographql/router/pull/6194 diff --git a/.circleci/config.yml b/.circleci/config.yml index 91bce18b05..7b932c7fc3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -507,7 +507,7 @@ commands: # TODO: remove this workaround once we update to Xcode >= 15.1.0 # See: https://github.com/apollographql/router/pull/5462 RUST_LIB_BACKTRACE: 0 - command: xtask test --workspace --locked --features ci + command: xtask test --workspace --locked --features ci,hyper_header_limits - run: name: Delete large files from cache command: | @@ -655,10 +655,10 @@ jobs: - run: cargo xtask release prepare nightly - run: command: > - cargo xtask dist --target aarch64-apple-darwin + cargo xtask dist --target aarch64-apple-darwin --features hyper_header_limits - run: command: > - cargo xtask dist --target x86_64-apple-darwin + cargo xtask dist --target x86_64-apple-darwin --features hyper_header_limits - run: command: > mkdir -p artifacts @@ -718,7 +718,7 @@ jobs: - run: cargo xtask release prepare nightly - run: command: > - cargo xtask dist + cargo xtask dist --features hyper_header_limits - run: command: > mkdir -p artifacts diff --git a/Cargo.lock b/Cargo.lock index 192ac8fc00..b8d2d105bd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3382,9 +3382,8 @@ dependencies = [ [[package]] name = "hyper" -version = "0.14.30" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a152ddd61dfaec7273fe8419ab357f33aee0d914c5f4efbf0d96fa749eea5ec9" +version = "0.14.31" +source = "git+https://github.com/apollographql/hyper.git?tag=header-customizations-20241108#c42aec785394b40645a283384838b856beace011" dependencies = [ "bytes", "futures-channel", @@ -3397,6 +3396,7 @@ dependencies = [ "httpdate", "itoa", "pin-project-lite", + "smallvec", "socket2 0.5.7", "tokio", "tower-service", diff --git a/Cargo.toml b/Cargo.toml index d8514547c6..c492b05480 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,3 +76,6 @@ sha1 = "0.10.6" tempfile = "3.10.1" tokio = { version = "1.36.0", features = ["full"] } tower = { version = "0.4.13", features = ["full"] } + +[patch.crates-io] +"hyper" = { git = "https://github.com/apollographql/hyper.git", tag = "header-customizations-20241108" } diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index ced84ad29f..6c7e06a6d0 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -52,6 +52,10 @@ docs_rs = ["router-bridge/docs_rs"] # and not yet ready for production use. telemetry_next = [] +# Allow Router to use feature from custom fork of Hyper until it is merged: +# https://github.com/hyperium/hyper/pull/3523 +hyper_header_limits = [] + # is set when ci builds take place. It allows us to disable some tests when CI is running on certain platforms. ci = [] @@ -105,7 +109,7 @@ http-body = "0.4.6" heck = "0.5.0" humantime = "2.1.0" humantime-serde = "1.1.1" -hyper = { version = "0.14.28", features = ["server", "client", "stream"] } +hyper = { version = "0.14.31", features = ["server", "client", "stream"] } hyper-rustls = { version = "0.24.2", features = ["http1", "http2"] } indexmap = { version = "2.2.6", features = ["serde"] } itertools = "0.13.0" diff --git a/apollo-router/src/axum_factory/axum_http_server_factory.rs b/apollo-router/src/axum_factory/axum_http_server_factory.rs index 08df933dc6..391424f0b1 100644 --- a/apollo-router/src/axum_factory/axum_http_server_factory.rs +++ b/apollo-router/src/axum_factory/axum_http_server_factory.rs @@ -5,6 +5,7 @@ use std::sync::atomic::AtomicBool; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; use std::sync::Arc; +use std::time::Duration; use std::time::Instant; use axum::error_handling::HandleErrorLayer; @@ -24,6 +25,7 @@ use http::header::CONTENT_ENCODING; use http::HeaderValue; use http::Request; use http_body::combinators::UnsyncBoxBody; +use hyper::server::conn::Http; use hyper::Body; use itertools::Itertools; use multimap::MultiMap; @@ -298,12 +300,25 @@ impl HttpServerFactory for AxumHttpServerFactory { let actual_main_listen_address = main_listener .local_addr() .map_err(ApolloRouterError::ServerCreationError)?; + let mut http_config = Http::new(); + http_config.http1_keep_alive(true); + http_config.http1_header_read_timeout(Duration::from_secs(10)); + + #[cfg(feature = "hyper_header_limits")] + if let Some(max_headers) = configuration.limits.http1_max_request_headers { + http_config.http1_max_headers(max_headers); + } + + if let Some(max_buf_size) = configuration.limits.http1_max_request_buf_size { + http_config.max_buf_size(max_buf_size.as_u64() as usize); + } let (main_server, main_shutdown_sender) = serve_router_on_listen_addr( main_listener, actual_main_listen_address.clone(), all_routers.main.1, true, + http_config.clone(), all_connections_stopped_sender.clone(), ); @@ -343,6 +358,7 @@ impl HttpServerFactory for AxumHttpServerFactory { listen_addr.clone(), router, false, + http_config.clone(), all_connections_stopped_sender.clone(), ); ( diff --git a/apollo-router/src/axum_factory/listeners.rs b/apollo-router/src/axum_factory/listeners.rs index dad439317c..52ea352979 100644 --- a/apollo-router/src/axum_factory/listeners.rs +++ b/apollo-router/src/axum_factory/listeners.rs @@ -202,6 +202,7 @@ pub(super) fn serve_router_on_listen_addr( address: ListenAddr, router: axum::Router, main_graphql_port: bool, + http_config: Http, all_connections_stopped_sender: mpsc::Sender<()>, ) -> (impl Future, oneshot::Sender<()>) { let (shutdown_sender, shutdown_receiver) = oneshot::channel::<()>(); @@ -243,6 +244,7 @@ pub(super) fn serve_router_on_listen_addr( } let address = address.clone(); + let mut http_config = http_config.clone(); tokio::task::spawn(async move { // this sender must be moved into the session to track that it is still running let _connection_stop_signal = connection_stop_signal; @@ -261,11 +263,8 @@ pub(super) fn serve_router_on_listen_addr( .expect( "this should not fail unless the socket is invalid", ); - let connection = Http::new() - .http1_keep_alive(true) - .http1_header_read_timeout(Duration::from_secs(10)) - .serve_connection(stream, app); + let connection = http_config.serve_connection(stream, app); tokio::pin!(connection); tokio::select! { // the connection finished first @@ -291,9 +290,7 @@ pub(super) fn serve_router_on_listen_addr( NetworkStream::Unix(stream) => { let received_first_request = Arc::new(AtomicBool::new(false)); let app = IdleConnectionChecker::new(received_first_request.clone(), app); - let connection = Http::new() - .http1_keep_alive(true) - .serve_connection(stream, app); + let connection = http_config.serve_connection(stream, app); tokio::pin!(connection); tokio::select! { @@ -329,9 +326,7 @@ pub(super) fn serve_router_on_listen_addr( let protocol = stream.get_ref().1.alpn_protocol(); let http2 = protocol == Some(&b"h2"[..]); - let connection = Http::new() - .http1_keep_alive(true) - .http1_header_read_timeout(Duration::from_secs(10)) + let connection = http_config .http2_only(http2) .serve_connection(stream, app); diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index c561a131f9..2df5ad8c62 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -492,6 +492,14 @@ impl Configuration { impl Configuration { pub(crate) fn validate(self) -> Result { + #[cfg(not(feature = "hyper_header_limits"))] + if self.limits.http1_max_request_headers.is_some() { + return Err(ConfigurationError::InvalidConfiguration { + message: "'limits.http1_max_request_headers' requires 'hyper_header_limits' feature", + error: "enable 'hyper_header_limits' feature in order to use 'limits.http1_max_request_headers'".to_string(), + }); + } + // Sandbox and Homepage cannot be both enabled if self.sandbox.enabled && self.homepage.enabled { return Err(ConfigurationError::InvalidConfiguration { 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 35913d1ce2..3616c90414 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 @@ -1,6 +1,7 @@ --- source: apollo-router/src/configuration/tests.rs expression: "&schema" +snapshot_kind: text --- { "$schema": "http://json-schema.org/draft-07/schema#", @@ -1317,6 +1318,20 @@ expression: "&schema" "additionalProperties": false, "description": "Configuration for operation limits, parser limits, HTTP limits, etc.", "properties": { + "http1_max_request_buf_size": { + "default": null, + "description": "Limit the maximum buffer size for the HTTP1 connection.\n\nDefault is ~400kib.", + "nullable": true, + "type": "string" + }, + "http1_max_request_headers": { + "default": null, + "description": "Limit the maximum number of headers of incoming HTTP1 requests. Default is 100.\n\nIf router receives more headers than the buffer size, it responds to the client with \"431 Request Header Fields Too Large\".", + "format": "uint", + "minimum": 0.0, + "nullable": true, + "type": "integer" + }, "http_max_request_bytes": { "default": 2000000, "description": "Limit the size of incoming HTTP requests read from the network, to protect against running out of memory. Default: 2000000 (2 MB)", diff --git a/apollo-router/src/configuration/tests.rs b/apollo-router/src/configuration/tests.rs index fb7acabf04..21cb5fdb50 100644 --- a/apollo-router/src/configuration/tests.rs +++ b/apollo-router/src/configuration/tests.rs @@ -413,6 +413,11 @@ fn validate_project_config_files() { }; for yaml in yamls { + #[cfg(not(feature = "hyper_header_limits"))] + if yaml.contains("http1_max_request_headers") { + continue; + } + if let Err(e) = validate_yaml_configuration( &yaml, Expansion::default().unwrap(), diff --git a/apollo-router/src/plugins/limits/mod.rs b/apollo-router/src/plugins/limits/mod.rs index ea743c6d2b..ec041a1c02 100644 --- a/apollo-router/src/plugins/limits/mod.rs +++ b/apollo-router/src/plugins/limits/mod.rs @@ -4,6 +4,7 @@ mod limited; use std::error::Error; use async_trait::async_trait; +use bytesize::ByteSize; use http::StatusCode; use schemars::JsonSchema; use serde::Deserialize; @@ -101,6 +102,19 @@ pub(crate) struct Config { /// Limit the size of incoming HTTP requests read from the network, /// to protect against running out of memory. Default: 2000000 (2 MB) pub(crate) http_max_request_bytes: usize, + + /// Limit the maximum number of headers of incoming HTTP1 requests. Default is 100. + /// + /// If router receives more headers than the buffer size, it responds to the client with + /// "431 Request Header Fields Too Large". + /// + pub(crate) http1_max_request_headers: Option, + + /// Limit the maximum buffer size for the HTTP1 connection. + /// + /// Default is ~400kib. + #[schemars(with = "Option", default)] + pub(crate) http1_max_request_buf_size: Option, } impl Default for Config { @@ -113,6 +127,8 @@ impl Default for Config { max_aliases: None, warn_only: false, http_max_request_bytes: 2_000_000, + http1_max_request_headers: None, + http1_max_request_buf_size: None, parser_max_tokens: 15_000, // This is `apollo-parser`’s default, which protects against stack overflow diff --git a/apollo-router/tests/integration/mod.rs b/apollo-router/tests/integration/mod.rs index c383b5348f..7e775a21a9 100644 --- a/apollo-router/tests/integration/mod.rs +++ b/apollo-router/tests/integration/mod.rs @@ -12,6 +12,7 @@ mod operation_limits; mod operation_name; mod query_planner; mod subgraph_response; +mod supergraph; mod traffic_shaping; mod typename; diff --git a/apollo-router/tests/integration/supergraph.rs b/apollo-router/tests/integration/supergraph.rs new file mode 100644 index 0000000000..97d5131d84 --- /dev/null +++ b/apollo-router/tests/integration/supergraph.rs @@ -0,0 +1,140 @@ +use std::collections::HashMap; + +use serde_json::json; +use tower::BoxError; + +use crate::integration::IntegrationTest; + +#[cfg(not(feature = "hyper_header_limits"))] +#[tokio::test(flavor = "multi_thread")] +async fn test_supergraph_error_http1_max_headers_config() -> Result<(), BoxError> { + let mut router = IntegrationTest::builder() + .config( + r#" + limits: + http1_max_request_headers: 100 + "#, + ) + .build() + .await; + + router.start().await; + router.assert_log_contains("'limits.http1_max_request_headers' requires 'hyper_header_limits' feature: enable 'hyper_header_limits' feature in order to use 'limits.http1_max_request_headers'").await; + router.assert_not_started().await; + Ok(()) +} + +#[cfg(feature = "hyper_header_limits")] +#[tokio::test(flavor = "multi_thread")] +async fn test_supergraph_errors_on_http1_max_headers() -> Result<(), BoxError> { + let mut router = IntegrationTest::builder() + .config( + r#" + limits: + http1_max_request_headers: 100 + "#, + ) + .build() + .await; + + router.start().await; + router.assert_started().await; + + let mut headers = HashMap::new(); + for i in 0..100 { + headers.insert(format!("test-header-{i}"), format!("value_{i}")); + } + + let (_trace_id, response) = router + .execute_query_with_headers(&json!({ "query": "{ __typename }"}), headers) + .await; + assert_eq!(response.status(), 431); + Ok(()) +} + +#[cfg(feature = "hyper_header_limits")] +#[tokio::test(flavor = "multi_thread")] +async fn test_supergraph_allow_to_change_http1_max_headers() -> Result<(), BoxError> { + let mut router = IntegrationTest::builder() + .config( + r#" + limits: + http1_max_request_headers: 200 + "#, + ) + .build() + .await; + + router.start().await; + router.assert_started().await; + + let mut headers = HashMap::new(); + for i in 0..100 { + headers.insert(format!("test-header-{i}"), format!("value_{i}")); + } + + let (_trace_id, response) = router + .execute_query_with_headers(&json!({ "query": "{ __typename }"}), headers) + .await; + assert_eq!(response.status(), 200); + assert_eq!( + response.json::().await?, + json!({ "data": { "__typename": "Query" } }) + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_supergraph_errors_on_http1_header_that_does_not_fit_inside_buffer( +) -> Result<(), BoxError> { + let mut router = IntegrationTest::builder() + .config( + r#" + limits: + http1_max_request_buf_size: 100kib + "#, + ) + .build() + .await; + + router.start().await; + router.assert_started().await; + + let mut headers = HashMap::new(); + headers.insert("test-header".to_string(), "x".repeat(1048576 + 1)); + + let (_trace_id, response) = router + .execute_query_with_headers(&json!({ "query": "{ __typename }"}), headers) + .await; + assert_eq!(response.status(), 431); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_supergraph_allow_to_change_http1_max_buf_size() -> Result<(), BoxError> { + let mut router = IntegrationTest::builder() + .config( + r#" + limits: + http1_max_request_buf_size: 2mib + "#, + ) + .build() + .await; + + router.start().await; + router.assert_started().await; + + let mut headers = HashMap::new(); + headers.insert("test-header".to_string(), "x".repeat(1048576 + 1)); + + let (_trace_id, response) = router + .execute_query_with_headers(&json!({ "query": "{ __typename }"}), headers) + .await; + assert_eq!(response.status(), 200); + assert_eq!( + response.json::().await?, + json!({ "data": { "__typename": "Query" } }) + ); + Ok(()) +} diff --git a/docs/source/reference/router/configuration.mdx b/docs/source/reference/router/configuration.mdx index 42597a9e57..9438bb331b 100644 --- a/docs/source/reference/router/configuration.mdx +++ b/docs/source/reference/router/configuration.mdx @@ -1114,6 +1114,8 @@ The router rejects any request that violates at least one of these limits. limits: # Network-based limits http_max_request_bytes: 2000000 # Default value: 2 MB + http1_max_request_headers: 200 # Default value: 100 + http1_max_request_buf_size: 800kib # Default value: 400kib # Parser-based limits parser_max_tokens: 15000 # Default value @@ -1145,6 +1147,24 @@ Before increasing this limit significantly consider testing performance in an environment similar to your production, especially if some clients are untrusted. Many concurrent large requests could cause the router to run out of memory. +##### `http1_max_request_headers` + +Limit the maximum number of headers of incoming HTTP1 requests. +The default value is 100 headers. + +If router receives more headers than the buffer size, it responds to the client with `431 Request Header Fields Too Large`. + +##### `http1_max_request_buf_size` + +Limit the maximum buffer size for the HTTP1 connection. Default is ~400kib. + +Note for Rust Crate Users: If you are using the Router as a Rust crate, the `http1_request_max_buf_size` option requires the `hyper_header_limits` feature and also necessitates using Apollo's fork of the Hyper crate until the [changes are merged upstream](https://github.com/hyperium/hyper/pull/3523). +You can include this fork by adding the following patch to your Cargo.toml file: +```toml +[patch.crates-io] +"hyper" = { git = "https://github.com/apollographql/hyper.git", tag = "header-customizations-20241108" } +``` + #### Parser-based limits ##### `parser_max_tokens` diff --git a/docs/source/routing/security/request-limits.mdx b/docs/source/routing/security/request-limits.mdx index 3cbe0ff552..3feb97f84f 100644 --- a/docs/source/routing/security/request-limits.mdx +++ b/docs/source/routing/security/request-limits.mdx @@ -15,6 +15,8 @@ For enhanced security, the GraphOS Router can reject requests that violate any o limits: # Network-based limits http_max_request_bytes: 2000000 # Default value: 2 MB + http1_max_request_headers: 200 # Default value: 100 + http1_max_request_buf_size: 800kb # Default value: 400kib # Parser-based limits parser_max_tokens: 15000 # Default value @@ -273,6 +275,24 @@ Before increasing this limit significantly consider testing performance in an environment similar to your production, especially if some clients are untrusted. Many concurrent large requests could cause the router to run out of memory. +### `http1_max_request_headers` + +Limit the maximum number of headers of incoming HTTP1 requests. +The default value is 100 headers. + +If router receives more headers than the buffer size, it responds to the client with `431 Request Header Fields Too Large`. + +### `http1_max_request_buf_size` + +Limit the maximum buffer size for the HTTP1 connection. Default is ~400kib. + +Note for Rust Crate Users: If you are using the Router as a Rust crate, the `http1_request_max_buf_size` option requires the `hyper_header_limits` feature and also necessitates using Apollo's fork of the Hyper crate until the [changes are merged upstream](https://github.com/hyperium/hyper/pull/3523). +You can include this fork by adding the following patch to your Cargo.toml file: +```toml +[patch.crates-io] +"hyper" = { git = "https://github.com/apollographql/hyper.git", tag = "header-customizations-20241108" } +``` + ## Parser-based limits ### `parser_max_tokens` diff --git a/xtask/src/commands/dist.rs b/xtask/src/commands/dist.rs index 544b8e9cb7..a28dbe234c 100644 --- a/xtask/src/commands/dist.rs +++ b/xtask/src/commands/dist.rs @@ -5,13 +5,25 @@ use xtask::*; pub struct Dist { #[clap(long)] target: Option, + + /// Pass --features to cargo test + #[clap(long)] + features: Option, } impl Dist { pub fn run(&self) -> Result<()> { + let mut args = vec!["build", "--release"]; + if let Some(features) = &self.features { + args.push("--features"); + args.push(features); + } + match &self.target { Some(target) => { - cargo!(["build", "--release", "--target", target]); + args.push("--target"); + args.push(target); + cargo!(args); let bin_path = TARGET_DIR .join(target.to_string()) @@ -21,7 +33,7 @@ impl Dist { eprintln!("successfully compiled to: {}", &bin_path); } None => { - cargo!(["build", "--release"]); + cargo!(args); let bin_path = TARGET_DIR.join("release").join(RELEASE_BIN);