diff --git a/src/cli/serve.rs b/src/cli/serve.rs index e33c3fc5..cbd327b3 100644 --- a/src/cli/serve.rs +++ b/src/cli/serve.rs @@ -87,6 +87,25 @@ fn show_start_message(bind_address: IpAddr, port: u16, color: ColorChoice) -> io writeln!(&mut buffer)?; + if !bind_address.is_loopback() { + let mut warning = ColorSpec::new(); + warning.set_fg(Some(Color::Yellow)).set_bold(true); + + buffer.set_color(&warning)?; + writeln!( + &mut buffer, + "WARNING: This server is bound to {address_string}, which is reachable from the \ + network.\n\ + The serve API is unauthenticated, so anyone who can reach {address_string}:{port} \ + can read\n\ + and modify your project's source. Prefer binding to localhost and tunneling (e.g. \ + SSH,\n\ + Tailscale, or WireGuard) when you need remote access." + )?; + buffer.set_color(&ColorSpec::new())?; + writeln!(&mut buffer)?; + } + buffer.set_color(&ColorSpec::new())?; write!(&mut buffer, "Visit ")?; diff --git a/src/web/api.rs b/src/web/api.rs index 38163bb9..532be770 100644 --- a/src/web/api.rs +++ b/src/web/api.rs @@ -1,7 +1,7 @@ //! Defines Rojo's HTTP API, all under /api. These endpoints generally return //! JSON. -use std::{collections::HashMap, fs, path::PathBuf, str::FromStr, sync::Arc}; +use std::{collections::HashMap, fs, net::SocketAddr, path::PathBuf, str::FromStr, sync::Arc}; use futures::{sink::SinkExt, stream::StreamExt}; use hyper::{body, Body, Method, Request, Response, StatusCode}; @@ -28,8 +28,12 @@ use crate::{ }, }; -pub async fn call(serve_session: Arc, mut request: Request) -> Response { - let service = ApiService::new(serve_session); +pub async fn call( + serve_session: Arc, + remote_addr: SocketAddr, + mut request: Request, +) -> Response { + let service = ApiService::new(serve_session, remote_addr); match (request.method(), request.uri().path()) { (&Method::GET, "/api/rojo") => service.handle_api_rojo().await, @@ -65,11 +69,15 @@ pub async fn call(serve_session: Arc, mut request: Request) pub struct ApiService { serve_session: Arc, + remote_addr: SocketAddr, } impl ApiService { - pub fn new(serve_session: Arc) -> Self { - ApiService { serve_session } + pub fn new(serve_session: Arc, remote_addr: SocketAddr) -> Self { + ApiService { + serve_session, + remote_addr, + } } /// Get a summary of information about the server @@ -348,6 +356,26 @@ impl ApiService { /// Open a script with the given ID in the user's default text editor. async fn handle_api_open(&self, request: Request) -> Response { + // Opening a file launches a local program, so it must never be reachable + // by a remote client even when the server is bound to an exposed address. + // + // `remote_addr` is the immediate peer, which is the strongest locality + // signal available: the legitimate caller is a sandboxed Roblox plugin + // whose only credential is being able to reach the port, so there is no + // secret to authenticate it with. A connection forwarded over loopback by + // an SSH/Tailscale tunnel or a local reverse proxy therefore appears local + // and is allowed. We treat that as delegated trust rather than a bypass: + // by deliberately standing up that tunnel/proxy the user has decided the + // remote end is trusted, and reachability is bounded by *that* hop's own + // authentication (SSH keys, Tailscale ACLs, ...). This gate's job is to + // stop direct, unauthenticated peers, which it does. + if !self.remote_addr.ip().is_loopback() { + return msgpack( + ErrorResponse::forbidden("/api/open is only available to local clients"), + StatusCode::FORBIDDEN, + ); + } + let argument = &request.uri().path()["/api/open/".len()..]; let requested_id = match Ref::from_str(argument) { Ok(id) => id, diff --git a/src/web/interface.rs b/src/web/interface.rs index 51307b67..8ec332ed 100644 --- a/src/web/interface.rs +++ b/src/web/interface.rs @@ -290,6 +290,13 @@ impl ErrorResponse { } } + pub fn forbidden>(details: S) -> Self { + Self { + kind: ErrorResponseKind::Forbidden, + details: details.into(), + } + } + pub fn internal_error>(details: S) -> Self { Self { kind: ErrorResponseKind::InternalError, @@ -302,5 +309,6 @@ impl ErrorResponse { pub enum ErrorResponseKind { NotFound, BadRequest, + Forbidden, InternalError, } diff --git a/src/web/mod.rs b/src/web/mod.rs index 8e33d9c5..780ed8c7 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -5,6 +5,7 @@ mod api; mod assets; pub mod interface; +mod origin; mod ui; mod util; @@ -14,7 +15,7 @@ use std::sync::Arc; use anyhow::Context; use hyper::{ - server::Server, + server::{conn::AddrStream, Server}, service::{make_service_fn, service_fn}, Body, Request, }; @@ -38,17 +39,29 @@ impl LiveServer { /// after binding can no longer fail (e.g. due to the port being in use). pub fn start(self, address: SocketAddr, on_listening: impl FnOnce()) -> anyhow::Result<()> { let serve_session = Arc::clone(&self.serve_session); + let allowed_hosts = origin::allowed_hosts(address.ip(), address.port()); - let make_service = make_service_fn(move |_conn| { + let make_service = make_service_fn(move |conn: &AddrStream| { let serve_session = Arc::clone(&serve_session); + let allowed_hosts = allowed_hosts.clone(); + let remote_addr = conn.remote_addr(); - async { + async move { let service = move |req: Request| { let serve_session = Arc::clone(&serve_session); + let allowed_hosts = allowed_hosts.clone(); async move { + // Reject cross-origin requests before doing any work, to + // defend the local server against DNS rebinding. + if let Some(response) = + origin::check_request_origin(&req, allowed_hosts.as_ref()) + { + return Ok::<_, Infallible>(response); + } + if req.uri().path().starts_with("/api") { - Ok::<_, Infallible>(api::call(serve_session, req).await) + Ok::<_, Infallible>(api::call(serve_session, remote_addr, req).await) } else { Ok::<_, Infallible>(ui::call(serve_session, req).await) } diff --git a/src/web/origin.rs b/src/web/origin.rs new file mode 100644 index 00000000..1c6ffad5 --- /dev/null +++ b/src/web/origin.rs @@ -0,0 +1,277 @@ +//! Host/Origin validation used to defend the local Rojo server against DNS +//! rebinding attacks. +//! +//! When Rojo is bound to a loopback address (the default), a malicious web page +//! the developer visits cannot read the API responses directly because of the +//! browser Same-Origin Policy. However, a DNS rebinding attack can defeat that: +//! the page points its own hostname at `127.0.0.1` after loading, so the browser +//! treats requests to the Rojo server as same-origin. Validating that the `Host` +//! (and, if present, `Origin`) header refers to a local address blocks this, +//! because the rebound request still carries the attacker's hostname. +//! +//! Validation is only enforced for loopback binds. If the user has explicitly +//! exposed the server on a non-loopback address we can't know which hostnames +//! legitimately resolve to it, so enforcement is disabled (they are warned at +//! startup instead). One consequence worth being explicit about: an exposed bind +//! therefore has *no* rebinding protection, even for a browser on the same +//! machine, because the `Host` check that would catch it is turned off. That is +//! an accepted trade-off: exposing the unauthenticated API to the network is +//! already the dominant risk, which the startup warning addresses. + +use std::net::IpAddr; + +use hyper::{ + header::{HOST, ORIGIN}, + http::uri::{Authority, Uri}, + Body, Request, Response, StatusCode, +}; + +use crate::web::{interface::ErrorResponse, util::msgpack}; + +/// The set of `Host`/`Origin` values accepted while the server is bound to a +/// loopback address. +#[derive(Debug, Clone)] +pub struct AllowedHosts { + port: u16, +} + +impl AllowedHosts { + /// Hostnames considered local, compared case-insensitively against the host + /// portion of the `Host`/`Origin` header (IPv6 brackets stripped first). + const LOCAL_HOSTS: [&'static str; 3] = ["localhost", "127.0.0.1", "::1"]; + + /// Returns whether the given host and optional port are allowed. A request + /// with no explicit port is accepted (the host already has to be local). + fn allows(&self, host: &str, port: Option) -> bool { + let host = normalize_host(host); + let host_is_local = Self::LOCAL_HOSTS + .iter() + .any(|local| host.eq_ignore_ascii_case(local)); + + host_is_local && port.is_none_or(|port| port == self.port) + } +} + +/// Builds the allowlist for a given bind address. Returns `None` (validation +/// disabled) when the server is bound to a non-loopback address. +pub fn allowed_hosts(bind: IpAddr, port: u16) -> Option { + if bind.is_loopback() { + Some(AllowedHosts { port }) + } else { + None + } +} + +/// Validates the `Host` and `Origin` headers of an incoming request against the +/// allowlist. Returns `Some` with a ready-to-send `403` response when the +/// request should be rejected, or `None` when it is allowed to proceed. When +/// `allowed` is `None` (non-loopback bind) every request is accepted. +pub fn check_request_origin( + request: &Request, + allowed: Option<&AllowedHosts>, +) -> Option> { + let allowed = allowed?; + + // The Host header is mandatory and must refer to a local address. + let host_ok = request + .headers() + .get(HOST) + .and_then(|value| value.to_str().ok()) + .and_then(parse_authority) + .is_some_and(|(host, port)| allowed.allows(&host, port)); + + if !host_ok { + return Some(reject()); + } + + // The Origin header is optional: non-browser clients such as the Roblox + // plugin never send it. When it is present (i.e. a browser made the request) + // it must also be local, which rejects a rebound page whose origin is still + // its own non-local hostname. + if let Some(origin) = request.headers().get(ORIGIN) { + let origin_ok = origin + .to_str() + .ok() + .and_then(parse_origin) + .is_some_and(|(host, port)| allowed.allows(&host, port)); + + if !origin_ok { + return Some(reject()); + } + } + + None +} + +/// Strips the surrounding brackets from an IPv6 host literal (e.g. `[::1]`), so +/// it can be compared against the unbracketed names in `LOCAL_HOSTS`. +fn normalize_host(host: &str) -> &str { + host.strip_prefix('[') + .and_then(|host| host.strip_suffix(']')) + .unwrap_or(host) +} + +/// Parses a `Host` header value (an authority such as `localhost:34872`) into +/// its host and optional port. Returns `None` if the authority carries userinfo +/// (e.g. `evil.com@localhost`), so a value whose host looks local only after the +/// userinfo is stripped can never sneak past the allowlist. +fn parse_authority(value: &str) -> Option<(String, Option)> { + let authority: Authority = value.parse().ok()?; + reject_userinfo(&authority)?; + Some((authority.host().to_owned(), authority.port_u16())) +} + +/// Parses an `Origin` header value (an absolute URI such as +/// `http://localhost:34872`) into its host and optional port. Returns `None` for +/// origins without a host, such as the opaque `null` origin, or for origins whose +/// authority carries userinfo (see [`parse_authority`]). +fn parse_origin(value: &str) -> Option<(String, Option)> { + let uri: Uri = value.parse().ok()?; + reject_userinfo(uri.authority()?)?; + Some((uri.host()?.to_owned(), uri.port_u16())) +} + +/// Returns `None` (rejecting the value) when an authority contains a userinfo +/// component, identified by the `@` separator. A bare host or `host:port` never +/// contains `@`, so this only fires on `userinfo@host` forms. +fn reject_userinfo(authority: &Authority) -> Option<()> { + if authority.as_str().contains('@') { + None + } else { + Some(()) + } +} + +fn reject() -> Response { + msgpack( + ErrorResponse::forbidden( + "Request rejected because its Host or Origin header is not an allowed local \ + address. This protects the Rojo server from DNS rebinding attacks.", + ), + StatusCode::FORBIDDEN, + ) +} + +#[cfg(test)] +mod test { + use super::*; + + use std::net::{Ipv4Addr, Ipv6Addr}; + + const PORT: u16 = 34872; + + fn loopback_allowlist() -> Option { + allowed_hosts(IpAddr::V4(Ipv4Addr::LOCALHOST), PORT) + } + + fn request_with(headers: &[(&'static str, &str)]) -> Request { + let mut builder = Request::builder().uri("/api/rojo"); + for (name, value) in headers { + builder = builder.header(*name, *value); + } + builder.body(Body::empty()).unwrap() + } + + #[test] + fn loopback_bind_enables_enforcement() { + assert!(allowed_hosts(IpAddr::V4(Ipv4Addr::LOCALHOST), PORT).is_some()); + assert!(allowed_hosts(IpAddr::V6(Ipv6Addr::LOCALHOST), PORT).is_some()); + } + + #[test] + fn non_loopback_bind_disables_enforcement() { + assert!(allowed_hosts(IpAddr::V4(Ipv4Addr::UNSPECIFIED), PORT).is_none()); + assert!(allowed_hosts(IpAddr::V4(Ipv4Addr::new(192, 168, 1, 5)), PORT).is_none()); + } + + #[test] + fn accepts_local_hosts() { + let allowed = loopback_allowlist(); + for host in [ + format!("localhost:{PORT}"), + format!("127.0.0.1:{PORT}"), + format!("[::1]:{PORT}"), + "localhost".to_owned(), + ] { + let request = request_with(&[("host", &host)]); + assert!( + check_request_origin(&request, allowed.as_ref()).is_none(), + "host {host} should be allowed" + ); + } + } + + #[test] + fn rejects_foreign_host() { + let allowed = loopback_allowlist(); + let request = request_with(&[("host", "evil.com")]); + assert!(check_request_origin(&request, allowed.as_ref()).is_some()); + } + + #[test] + fn rejects_wrong_port() { + let allowed = loopback_allowlist(); + let request = request_with(&[("host", "localhost:1234")]); + assert!(check_request_origin(&request, allowed.as_ref()).is_some()); + } + + #[test] + fn rejects_missing_host() { + let allowed = loopback_allowlist(); + let request = request_with(&[]); + assert!(check_request_origin(&request, allowed.as_ref()).is_some()); + } + + #[test] + fn rejects_host_with_userinfo() { + let allowed = loopback_allowlist(); + // The host parses to `localhost`, but the userinfo prefix must keep it + // from being treated as a local address. + let request = request_with(&[("host", &format!("evil.com@localhost:{PORT}"))]); + assert!(check_request_origin(&request, allowed.as_ref()).is_some()); + } + + #[test] + fn rejects_origin_with_userinfo() { + let allowed = loopback_allowlist(); + let request = request_with(&[ + ("host", &format!("localhost:{PORT}")), + ("origin", &format!("http://evil.com@localhost:{PORT}")), + ]); + assert!(check_request_origin(&request, allowed.as_ref()).is_some()); + } + + #[test] + fn rejects_foreign_origin_even_with_local_host() { + let allowed = loopback_allowlist(); + let request = request_with(&[ + ("host", &format!("localhost:{PORT}")), + ("origin", &format!("http://evil.com:{PORT}")), + ]); + assert!(check_request_origin(&request, allowed.as_ref()).is_some()); + } + + #[test] + fn rejects_null_origin() { + let allowed = loopback_allowlist(); + let request = request_with(&[("host", &format!("localhost:{PORT}")), ("origin", "null")]); + assert!(check_request_origin(&request, allowed.as_ref()).is_some()); + } + + #[test] + fn accepts_local_origin() { + let allowed = loopback_allowlist(); + let request = request_with(&[ + ("host", &format!("localhost:{PORT}")), + ("origin", &format!("http://localhost:{PORT}")), + ]); + assert!(check_request_origin(&request, allowed.as_ref()).is_none()); + } + + #[test] + fn disabled_allowlist_accepts_foreign_host() { + let allowed = allowed_hosts(IpAddr::V4(Ipv4Addr::UNSPECIFIED), PORT); + let request = request_with(&[("host", "evil.com")]); + assert!(check_request_origin(&request, allowed.as_ref()).is_none()); + } +} diff --git a/tests/rojo_test/serve_util.rs b/tests/rojo_test/serve_util.rs index 4b67a883..b7853abe 100644 --- a/tests/rojo_test/serve_util.rs +++ b/tests/rojo_test/serve_util.rs @@ -126,6 +126,10 @@ impl TestServeSession { &self.project_path } + pub fn port(&self) -> usize { + self.port + } + /// Waits for the `rojo serve` server to come online with expontential /// backoff. pub fn wait_to_come_online(&mut self) -> ServerInfoResponse { @@ -241,6 +245,35 @@ impl TestServeSession { Ok(deserialize_msgpack(&body).expect("Server returned malformed response")) } + + /// Sends a GET to `/api/rojo` with the given extra request headers and + /// returns the response status code. Used to exercise the Host/Origin + /// allowlist that guards against DNS rebinding. + pub fn api_rojo_status_with_headers(&self, headers: &[(&str, &str)]) -> reqwest::StatusCode { + let client = reqwest::blocking::Client::new(); + let url = format!("http://localhost:{}/api/rojo", self.port); + + let mut request = client.get(url); + for (name, value) in headers { + request = request.header(*name, *value); + } + + request.send().expect("Failed to send request").status() + } + + /// Sends a POST to `/api/open/` and returns the response status code. + /// Used to verify that the local-only gate on `/api/open` admits loopback + /// peers (the test harness always connects over loopback). + pub fn api_open_status(&self, id: &str) -> reqwest::StatusCode { + let client = reqwest::blocking::Client::new(); + let url = format!("http://localhost:{}/api/open/{}", self.port, id); + + client + .post(url) + .send() + .expect("Failed to send request") + .status() + } } fn serialize_msgpack(value: T) -> Result, rmp_serde::encode::Error> { diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index 748f1687..ef9ec464 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -10,6 +10,51 @@ use crate::rojo_test::{ use librojo::web_api::SocketPacketType; +#[test] +fn rejects_dns_rebinding_requests() { + run_serve_test("empty", |session, _redactions| { + let port = session.port(); + let local_host = format!("localhost:{port}"); + + // A request carrying a local Host header is served normally. + assert_eq!( + session.api_rojo_status_with_headers(&[("host", &local_host)]), + reqwest::StatusCode::OK, + ); + + // A request whose Host is a foreign hostname, as a DNS-rebound page + // would send, is rejected. + assert_eq!( + session.api_rojo_status_with_headers(&[("host", "evil.com")]), + reqwest::StatusCode::FORBIDDEN, + ); + + // Even with a local Host, a present-but-foreign Origin is rejected. + let foreign_origin = format!("http://evil.com:{port}"); + assert_eq!( + session.api_rojo_status_with_headers(&[ + ("host", &local_host), + ("origin", &foreign_origin), + ]), + reqwest::StatusCode::FORBIDDEN, + ); + }); +} + +#[test] +fn allows_api_open_from_loopback_peer() { + run_serve_test("empty", |session, _redactions| { + // The harness always connects over loopback, so the local-only gate on + // /api/open must let the request through. A bogus instance id then fails + // id parsing with 400, which confirms we got past the gate rather than + // being rejected with 403. + assert_eq!( + session.api_open_status("not-a-real-ref"), + reqwest::StatusCode::BAD_REQUEST, + ); + }); +} + #[test] fn empty() { run_serve_test("empty", |session, mut redactions| {