Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions crates/rmcp/src/transport/streamable_http_server/tower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ pub struct StreamableHttpServerConfig {
/// Defaults to an empty list, which disables Origin validation for backward
/// compatibility. A non-empty list enables validation. Requests carrying
/// an `Origin` header must match per RFC 6454 `(scheme, host, port)`;
/// missing-`Origin` requests still pass. Entries must include a scheme;
/// missing-`Origin` requests still pass. An entry that omits the port
/// permits any port; an entry with an explicit port matches only that

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it really permit any port, or if it's not listed should it permit only the default port for the scheme?

People can always configure it with a specific known port if they need one...

If we need the unconditional match, perhaps it would be better to introduce a syntax for specifying that like:

http://localhost:*

/// port, resolving an origin's omitted port to the scheme default
/// (443 for `https`, 80 for `http`). Entries must include a scheme;
/// `"null"` matches the browser's `Origin: null`.
///
/// Call [`StreamableHttpServerConfig::enforce_origin_validation`] to enable
Expand Down Expand Up @@ -853,6 +856,15 @@ fn parse_origin_value(value: &str) -> Option<NormalizedOrigin> {
})
}

/// The port a scheme implies when an origin serialization omits it (RFC 6454 §4).
fn default_port(scheme: &str) -> Option<u16> {
match scheme {
"http" => Some(80),
"https" => Some(443),
_ => None,
}
}

fn origin_is_allowed(origin: &NormalizedOrigin, allowed_origins: &[String]) -> bool {
allowed_origins
.iter()
Expand All @@ -870,7 +882,17 @@ fn origin_is_allowed(origin: &NormalizedOrigin, allowed_origins: &[String]) -> b
host: o_host,
port: o_port,
},
) => a_scheme == o_scheme && a_host == o_host && (a_port.is_none() || a_port == o_port),
) => {
a_scheme == o_scheme
&& a_host == o_host
&& match a_port {
// An omitted configured port permits any port.
None => true,
// RFC 6454 §6.2 omits the default port when serializing an
// origin, so an absent incoming port means the scheme default.
Some(a_port) => o_port.or_else(|| default_port(o_scheme)) == Some(*a_port),
}
}
_ => false,
})
}
Expand Down
54 changes: 54 additions & 0 deletions crates/rmcp/tests/test_custom_headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1311,4 +1311,58 @@ mod origin_validation {
let response = service.handle(init_request(Some("null"))).await;
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
}

#[tokio::test]
async fn explicit_https_default_port_allows_port_less_origin() {
let service = service_with_allowed_origins(&["https://client.example:443"]);
let response = service
.handle(init_request(Some("https://client.example")))
.await;
assert_eq!(response.status(), http::StatusCode::OK);
}

#[tokio::test]
async fn explicit_http_default_port_allows_port_less_origin() {
let service = service_with_allowed_origins(&["http://client.example:80"]);
let response = service
.handle(init_request(Some("http://client.example")))
.await;
assert_eq!(response.status(), http::StatusCode::OK);
}

#[tokio::test]
async fn explicit_default_port_allows_matching_explicit_origin_port() {
let service = service_with_allowed_origins(&["https://client.example:443"]);
let response = service
.handle(init_request(Some("https://client.example:443")))
.await;
assert_eq!(response.status(), http::StatusCode::OK);
}

#[tokio::test]
async fn explicit_default_port_forbids_non_default_origin_port() {
let service = service_with_allowed_origins(&["https://client.example:443"]);
let response = service
.handle(init_request(Some("https://client.example:8443")))
.await;
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
}

#[tokio::test]
async fn explicit_non_default_port_forbids_port_less_origin() {
let service = service_with_allowed_origins(&["https://client.example:8443"]);
let response = service
.handle(init_request(Some("https://client.example")))
.await;
assert_eq!(response.status(), http::StatusCode::FORBIDDEN);
}

#[tokio::test]
async fn omitted_configured_port_allows_any_origin_port() {
let service = service_with_allowed_origins(&["https://client.example"]);
let response = service
.handle(init_request(Some("https://client.example:8443")))
.await;
assert_eq!(response.status(), http::StatusCode::OK);
}
}