diff --git a/conformance/src/bin/server.rs b/conformance/src/bin/server.rs index 0d2baf4e7..0550fbd04 100644 --- a/conformance/src/bin/server.rs +++ b/conformance/src/bin/server.rs @@ -1744,7 +1744,8 @@ async fn main() -> anyhow::Result<()> { tracing::info!("Starting conformance server on {}", bind_addr); let server = ConformanceServer::new(); - let config = StreamableHttpServerConfig::default(); + let config = + StreamableHttpServerConfig::default().with_allowed_origins([format!("http://{bind_addr}")]); let service = StreamableHttpService::new( move || Ok(server.clone()), LocalSessionManager::default().into(), diff --git a/crates/rmcp/src/transport/streamable_http_server/tower.rs b/crates/rmcp/src/transport/streamable_http_server/tower.rs index f1fef585f..bc6f95b18 100644 --- a/crates/rmcp/src/transport/streamable_http_server/tower.rs +++ b/crates/rmcp/src/transport/streamable_http_server/tower.rs @@ -92,13 +92,18 @@ pub struct StreamableHttpServerConfig { pub allowed_hosts: Vec, /// Allowed browser origins for inbound `Origin` validation. /// - /// Defaults to an empty list, which disables Origin validation. When - /// non-empty, requests carrying an `Origin` header must match per RFC 6454 - /// `(scheme, host, port)`; missing-`Origin` requests still pass. Entries - /// must include a scheme; `"null"` matches the browser's `Origin: null`. + /// Validation is enabled by default. Requests carrying an `Origin` header + /// must match per RFC 6454 + /// `(scheme, host, port)`; missing-`Origin` requests still pass. An empty + /// list allows no present Origin values. Entries must include a scheme; + /// `"null"` matches the browser's `Origin: null`. + /// + /// Call [`StreamableHttpServerConfig::disable_allowed_origins`] to + /// explicitly disable Origin validation. /// examples: /// allowed_origins = ["https://app.example.com", "http://localhost:8080"] pub allowed_origins: Vec, + origin_validation_enabled: bool, /// Optional external session store for cross-instance recovery. /// /// When set, [`SessionState`] (the client's `initialize` parameters) is @@ -171,6 +176,7 @@ impl Default for StreamableHttpServerConfig { cancellation_token: CancellationToken::new(), allowed_hosts: vec!["localhost".into(), "127.0.0.1".into(), "::1".into()], allowed_origins: vec![], + origin_validation_enabled: true, session_store: None, max_request_body_bytes: DEFAULT_MAX_REQUEST_BODY_BYTES, stateless_protocol_metadata_required: false, @@ -196,11 +202,13 @@ impl StreamableHttpServerConfig { allowed_origins: impl IntoIterator>, ) -> Self { self.allowed_origins = allowed_origins.into_iter().map(Into::into).collect(); + self.origin_validation_enabled = true; self } - /// Disable Origin validation, reverting to the default ignore-Origin behavior. + /// Disable Origin validation, allowing requests with any `Origin` header. pub fn disable_allowed_origins(mut self) -> Self { self.allowed_origins.clear(); + self.origin_validation_enabled = false; self } pub fn with_sse_keep_alive(mut self, duration: Option) -> Self { @@ -797,9 +805,6 @@ fn parse_origin_value(value: &str) -> Option { } fn origin_is_allowed(origin: &NormalizedOrigin, allowed_origins: &[String]) -> bool { - if allowed_origins.is_empty() { - return true; - } allowed_origins .iter() .filter_map(|raw| parse_origin_value(raw)) @@ -874,15 +879,15 @@ fn validate_dns_rebinding_headers( ); return Err(forbidden_response("Forbidden: Host header is not allowed")); } - validate_origin_header(headers, &config.allowed_origins)?; + validate_origin_header(headers, config)?; Ok(()) } fn validate_origin_header( headers: &HeaderMap, - allowed_origins: &[String], + config: &StreamableHttpServerConfig, ) -> Result<(), BoxResponse> { - if allowed_origins.is_empty() { + if !config.origin_validation_enabled { return Ok(()); } let Some(origin_header) = headers.get(http::header::ORIGIN) else { @@ -893,15 +898,15 @@ fn validate_origin_header( .inspect_err(|_| { tracing::warn!(origin = ?origin_header, "rejected request with non-UTF-8 Origin header"); }) - .map_err(|_| bad_request_response("Bad Request: Invalid Origin header encoding"))?; + .map_err(|_| forbidden_response("Forbidden: Invalid Origin header encoding"))?; let origin = parse_origin_value(origin_str).ok_or_else(|| { tracing::warn!( origin = origin_str, "rejected request with malformed Origin header", ); - bad_request_response("Bad Request: Invalid Origin header") + forbidden_response("Forbidden: Invalid Origin header") })?; - if !origin_is_allowed(&origin, allowed_origins) { + if !origin_is_allowed(&origin, &config.allowed_origins) { tracing::warn!( origin = ?origin, "rejected request with disallowed Origin header (possible cross-origin attack)", diff --git a/crates/rmcp/tests/test_custom_headers.rs b/crates/rmcp/tests/test_custom_headers.rs index 736dce18e..8dc7dd356 100644 --- a/crates/rmcp/tests/test_custom_headers.rs +++ b/crates/rmcp/tests/test_custom_headers.rs @@ -880,10 +880,10 @@ fn test_protocol_version_utilities() { assert!(ProtocolVersion::KNOWN_VERSIONS.contains(&ProtocolVersion::V_2026_07_28)); } -/// Integration test: Verify server validates only the Host header for DNS rebinding protection +/// Integration test: Verify Host validation remains enabled when Origin validation is disabled #[tokio::test] #[cfg(all(feature = "transport-streamable-http-server", feature = "server",))] -async fn test_server_validates_host_header_for_dns_rebinding_protection() { +async fn test_server_validates_host_when_origin_validation_is_disabled() { use std::sync::Arc; use bytes::Bytes; @@ -910,7 +910,7 @@ async fn test_server_validates_host_header_for_dns_rebinding_protection() { let service = StreamableHttpService::new( || Ok(TestHandler), Arc::new(LocalSessionManager::default()), - StreamableHttpServerConfig::default(), + StreamableHttpServerConfig::default().disable_allowed_origins(), ); let init_body = json!({ @@ -1127,7 +1127,7 @@ mod origin_validation { use std::sync::Arc; use bytes::Bytes; - use http::{Method, Request, header::CONTENT_TYPE}; + use http::{HeaderValue, Method, Request, header::CONTENT_TYPE}; use http_body_util::Full; use rmcp::{ handler::server::ServerHandler, @@ -1147,12 +1147,20 @@ mod origin_validation { } } - fn service_with_allowed_origins( - origins: &[&str], + fn service_with_config( + config: StreamableHttpServerConfig, ) -> StreamableHttpService { StreamableHttpService::new( || Ok(TestHandler), Arc::new(LocalSessionManager::default()), + config, + ) + } + + fn service_with_allowed_origins( + origins: &[&str], + ) -> StreamableHttpService { + service_with_config( StreamableHttpServerConfig::default().with_allowed_origins(origins.iter().copied()), ) } @@ -1199,6 +1207,51 @@ mod origin_validation { assert_eq!(response.status(), http::StatusCode::FORBIDDEN); } + #[tokio::test] + async fn malformed_origin_is_forbidden() { + let service = service_with_allowed_origins(&["http://localhost:8080"]); + let response = service.handle(init_request(Some("not-an-origin"))).await; + assert_eq!(response.status(), http::StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn non_utf8_origin_is_forbidden() { + let service = service_with_allowed_origins(&["http://localhost:8080"]); + let mut request = init_request(None); + request.headers_mut().insert( + http::header::ORIGIN, + HeaderValue::from_bytes(b"\xff").unwrap(), + ); + let response = service.handle(request).await; + assert_eq!(response.status(), http::StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn empty_allowlist_forbids_present_origin() { + let service = service_with_config(StreamableHttpServerConfig::default()); + let response = service + .handle(init_request(Some("http://localhost:8080"))) + .await; + assert_eq!(response.status(), http::StatusCode::FORBIDDEN); + } + + #[tokio::test] + async fn empty_allowlist_allows_missing_origin() { + let service = service_with_config(StreamableHttpServerConfig::default()); + let response = service.handle(init_request(None)).await; + assert_eq!(response.status(), http::StatusCode::OK); + } + + #[tokio::test] + async fn explicitly_disabled_validation_allows_present_origin() { + let service = + service_with_config(StreamableHttpServerConfig::default().disable_allowed_origins()); + let response = service + .handle(init_request(Some("http://attacker.example"))) + .await; + assert_eq!(response.status(), http::StatusCode::OK); + } + #[tokio::test] async fn missing_origin_passes_through() { let service = service_with_allowed_origins(&["http://localhost:8080"]);