Skip to content

Commit e27c5d1

Browse files
authored
fix(http): enforce Origin validation semantics (#1192)
Return HTTP 403 for malformed and non-UTF-8 Origin headers when validation is enabled. Add enforce_origin_validation() for strict empty-allowlist validation while preserving the legacy default and non-empty allowlist behavior. Configure the conformance server to opt in explicitly. Signed-off-by: lucarlig <luca.carlig@ibm.com>
1 parent 3e636ca commit e27c5d1

3 files changed

Lines changed: 117 additions & 20 deletions

File tree

conformance/src/bin/server.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1744,7 +1744,9 @@ async fn main() -> anyhow::Result<()> {
17441744
tracing::info!("Starting conformance server on {}", bind_addr);
17451745

17461746
let server = ConformanceServer::new();
1747-
let config = StreamableHttpServerConfig::default();
1747+
let config = StreamableHttpServerConfig::default()
1748+
.with_allowed_origins([format!("http://{bind_addr}")])
1749+
.enforce_origin_validation();
17481750
let service = StreamableHttpService::new(
17491751
move || Ok(server.clone()),
17501752
LocalSessionManager::default().into(),

crates/rmcp/src/transport/streamable_http_server/tower.rs

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,18 @@ pub struct StreamableHttpServerConfig {
110110
pub allowed_hosts: Vec<String>,
111111
/// Allowed browser origins for inbound `Origin` validation.
112112
///
113-
/// Defaults to an empty list, which disables Origin validation. When
114-
/// non-empty, requests carrying an `Origin` header must match per RFC 6454
115-
/// `(scheme, host, port)`; missing-`Origin` requests still pass. Entries
116-
/// must include a scheme; `"null"` matches the browser's `Origin: null`.
113+
/// Defaults to an empty list, which disables Origin validation for backward
114+
/// compatibility. A non-empty list enables validation. Requests carrying
115+
/// an `Origin` header must match per RFC 6454 `(scheme, host, port)`;
116+
/// missing-`Origin` requests still pass. Entries must include a scheme;
117+
/// `"null"` matches the browser's `Origin: null`.
118+
///
119+
/// Call [`StreamableHttpServerConfig::enforce_origin_validation`] to enable
120+
/// validation with an empty list, rejecting every present Origin value.
117121
/// examples:
118122
/// allowed_origins = ["https://app.example.com", "http://localhost:8080"]
119123
pub allowed_origins: Vec<String>,
124+
validate_empty_origin_allowlist: bool,
120125
/// Optional external session store for cross-instance recovery.
121126
///
122127
/// When set, [`SessionState`] (the client's `initialize` parameters) is
@@ -189,6 +194,7 @@ impl Default for StreamableHttpServerConfig {
189194
cancellation_token: CancellationToken::new(),
190195
allowed_hosts: vec!["localhost".into(), "127.0.0.1".into(), "::1".into()],
191196
allowed_origins: vec![],
197+
validate_empty_origin_allowlist: false,
192198
session_store: None,
193199
max_request_body_bytes: DEFAULT_MAX_REQUEST_BODY_BYTES,
194200
stateless_protocol_metadata_required: false,
@@ -216,9 +222,15 @@ impl StreamableHttpServerConfig {
216222
self.allowed_origins = allowed_origins.into_iter().map(Into::into).collect();
217223
self
218224
}
219-
/// Disable Origin validation, reverting to the default ignore-Origin behavior.
225+
/// Enable Origin validation, including when the allowed Origins list is empty.
226+
pub fn enforce_origin_validation(mut self) -> Self {
227+
self.validate_empty_origin_allowlist = true;
228+
self
229+
}
230+
/// Disable Origin validation, allowing requests with any `Origin` header.
220231
pub fn disable_allowed_origins(mut self) -> Self {
221232
self.allowed_origins.clear();
233+
self.validate_empty_origin_allowlist = false;
222234
self
223235
}
224236
pub fn with_sse_keep_alive(mut self, duration: Option<Duration>) -> Self {
@@ -802,9 +814,6 @@ fn parse_origin_value(value: &str) -> Option<NormalizedOrigin> {
802814
}
803815

804816
fn origin_is_allowed(origin: &NormalizedOrigin, allowed_origins: &[String]) -> bool {
805-
if allowed_origins.is_empty() {
806-
return true;
807-
}
808817
allowed_origins
809818
.iter()
810819
.filter_map(|raw| parse_origin_value(raw))
@@ -876,12 +885,15 @@ fn validate_dns_rebinding_headers(
876885
);
877886
return Err(forbidden_response("Forbidden: Host header is not allowed").into());
878887
}
879-
validate_origin_header(headers, &config.allowed_origins)?;
888+
validate_origin_header(headers, config)?;
880889
Ok(())
881890
}
882891

883-
fn validate_origin_header(headers: &HeaderMap, allowed_origins: &[String]) -> HttpResult<()> {
884-
if allowed_origins.is_empty() {
892+
fn validate_origin_header(
893+
headers: &HeaderMap,
894+
config: &StreamableHttpServerConfig,
895+
) -> HttpResult<()> {
896+
if !config.validate_empty_origin_allowlist && config.allowed_origins.is_empty() {
885897
return Ok(());
886898
}
887899
let Some(origin_header) = headers.get(http::header::ORIGIN) else {
@@ -892,15 +904,15 @@ fn validate_origin_header(headers: &HeaderMap, allowed_origins: &[String]) -> Ht
892904
.inspect_err(|_| {
893905
tracing::warn!(origin = ?origin_header, "rejected request with non-UTF-8 Origin header");
894906
})
895-
.map_err(|_| bad_request_response("Bad Request: Invalid Origin header encoding"))?;
907+
.map_err(|_| forbidden_response("Forbidden: Invalid Origin header encoding"))?;
896908
let origin = parse_origin_value(origin_str).ok_or_else(|| {
897909
tracing::warn!(
898910
origin = origin_str,
899911
"rejected request with malformed Origin header",
900912
);
901-
bad_request_response("Bad Request: Invalid Origin header")
913+
forbidden_response("Forbidden: Invalid Origin header")
902914
})?;
903-
if !origin_is_allowed(&origin, allowed_origins) {
915+
if !origin_is_allowed(&origin, &config.allowed_origins) {
904916
tracing::warn!(
905917
origin = ?origin,
906918
"rejected request with disallowed Origin header (possible cross-origin attack)",

crates/rmcp/tests/test_custom_headers.rs

Lines changed: 88 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -880,10 +880,11 @@ fn test_protocol_version_utilities() {
880880
assert!(ProtocolVersion::KNOWN_VERSIONS.contains(&ProtocolVersion::V_2026_07_28));
881881
}
882882

883-
/// Integration test: Verify server validates only the Host header for DNS rebinding protection
883+
/// Integration test: Verify Host validation remains enabled when Origin validation is
884+
/// disabled by default
884885
#[tokio::test]
885886
#[cfg(all(feature = "transport-streamable-http-server", feature = "server",))]
886-
async fn test_server_validates_host_header_for_dns_rebinding_protection() {
887+
async fn test_server_validates_host_when_origin_validation_is_disabled_by_default() {
887888
use std::sync::Arc;
888889

889890
use bytes::Bytes;
@@ -1127,7 +1128,7 @@ mod origin_validation {
11271128
use std::sync::Arc;
11281129

11291130
use bytes::Bytes;
1130-
use http::{Method, Request, header::CONTENT_TYPE};
1131+
use http::{HeaderValue, Method, Request, header::CONTENT_TYPE};
11311132
use http_body_util::Full;
11321133
use rmcp::{
11331134
handler::server::ServerHandler,
@@ -1147,12 +1148,20 @@ mod origin_validation {
11471148
}
11481149
}
11491150

1150-
fn service_with_allowed_origins(
1151-
origins: &[&str],
1151+
fn service_with_config(
1152+
config: StreamableHttpServerConfig,
11521153
) -> StreamableHttpService<TestHandler, LocalSessionManager> {
11531154
StreamableHttpService::new(
11541155
|| Ok(TestHandler),
11551156
Arc::new(LocalSessionManager::default()),
1157+
config,
1158+
)
1159+
}
1160+
1161+
fn service_with_allowed_origins(
1162+
origins: &[&str],
1163+
) -> StreamableHttpService<TestHandler, LocalSessionManager> {
1164+
service_with_config(
11561165
StreamableHttpServerConfig::default().with_allowed_origins(origins.iter().copied()),
11571166
)
11581167
}
@@ -1199,6 +1208,80 @@ mod origin_validation {
11991208
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
12001209
}
12011210

1211+
#[tokio::test]
1212+
async fn malformed_origin_is_forbidden() {
1213+
let service = service_with_allowed_origins(&["http://localhost:8080"]);
1214+
let response = service.handle(init_request(Some("not-an-origin"))).await;
1215+
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
1216+
}
1217+
1218+
#[tokio::test]
1219+
async fn non_utf8_origin_is_forbidden() {
1220+
let service = service_with_allowed_origins(&["http://localhost:8080"]);
1221+
let mut request = init_request(None);
1222+
request.headers_mut().insert(
1223+
http::header::ORIGIN,
1224+
HeaderValue::from_bytes(b"\xff").unwrap(),
1225+
);
1226+
let response = service.handle(request).await;
1227+
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
1228+
}
1229+
1230+
#[tokio::test]
1231+
async fn enabled_empty_allowlist_forbids_present_origin() {
1232+
let service =
1233+
service_with_config(StreamableHttpServerConfig::default().enforce_origin_validation());
1234+
let response = service
1235+
.handle(init_request(Some("http://localhost:8080")))
1236+
.await;
1237+
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
1238+
}
1239+
1240+
#[tokio::test]
1241+
async fn enabled_empty_allowlist_allows_missing_origin() {
1242+
let service =
1243+
service_with_config(StreamableHttpServerConfig::default().enforce_origin_validation());
1244+
let response = service.handle(init_request(None)).await;
1245+
assert_eq!(response.status(), http::StatusCode::OK);
1246+
}
1247+
1248+
#[tokio::test]
1249+
async fn empty_allowed_origins_preserves_disabled_validation() {
1250+
let service = service_with_config(
1251+
StreamableHttpServerConfig::default().with_allowed_origins(std::iter::empty::<&str>()),
1252+
);
1253+
let response = service
1254+
.handle(init_request(Some("http://attacker.example")))
1255+
.await;
1256+
assert_eq!(response.status(), http::StatusCode::OK);
1257+
}
1258+
1259+
#[tokio::test]
1260+
async fn nonempty_public_allowed_origins_field_enables_validation() {
1261+
let mut config = StreamableHttpServerConfig::default();
1262+
config
1263+
.allowed_origins
1264+
.push("http://localhost:8080".to_string());
1265+
let service = service_with_config(config);
1266+
let response = service
1267+
.handle(init_request(Some("http://attacker.example")))
1268+
.await;
1269+
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
1270+
}
1271+
1272+
#[tokio::test]
1273+
async fn explicitly_disabled_validation_allows_present_origin() {
1274+
let service = service_with_config(
1275+
StreamableHttpServerConfig::default()
1276+
.enforce_origin_validation()
1277+
.disable_allowed_origins(),
1278+
);
1279+
let response = service
1280+
.handle(init_request(Some("http://attacker.example")))
1281+
.await;
1282+
assert_eq!(response.status(), http::StatusCode::OK);
1283+
}
1284+
12021285
#[tokio::test]
12031286
async fn missing_origin_passes_through() {
12041287
let service = service_with_allowed_origins(&["http://localhost:8080"]);

0 commit comments

Comments
 (0)