fix: Prevent malicious websites from connecting to http / websocket server (#1157)

* fix: Prevent malicious websites from connecting to http / websocket server

Browsers allow any website to connect to websockets on `127.0.0.1` and,
therefore, to spy on users of tinymist. Disallow this by checking the
`Origin` header. Note: This does not protect against malicious users
that share the same `127.0.0.1` as us (e.g. multi-user systems where
the users don't trust each other). That requires additional changes
that may be added in the future.

* Add VSCode exception

* Also prevent malicious connections to trace/profiling http server

Allow VSCode only for now.
This commit is contained in:
tmistele 2025-01-27 00:56:05 -05:00 committed by GitHub
parent 1b80d8c31d
commit ea331a5aa6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 73 additions and 21 deletions

View file

@ -245,14 +245,25 @@ pub async fn make_http_server(
let timings = timings.clone();
let _ = alive_tx.send(());
async move {
// Make sure VSCode can connect to this http server but no malicious website a user
// might open in a browser. We recognize VSCode by an `Origin` header that starts
// with `vscode-webview://`. Malicious websites can (hopefully) not trick browsers
// into sending an `Origin` header that starts with `vscode-webview://`
//
// See comment in `make_http_server` in `crates/tinymist/src/tool/preview.rs` for more
// details. In particular, note that this does _not_ protect against malicious users
// that share the same computer as us.
let Some(allowed_origin) = req
.headers()
.get("Origin")
.filter(|h| h.as_bytes().starts_with(b"vscode-webview://"))
else {
anyhow::bail!("Origin must start with vscode-webview://");
};
let b = hyper::Response::builder()
.header(hyper::header::ACCESS_CONTROL_ALLOW_ORIGIN, allowed_origin);
if req.uri().path() == "/" {
let b = hyper::Response::builder()
.header(hyper::header::ACCESS_CONTROL_ALLOW_ORIGIN, "*")
.header(hyper::header::ACCESS_CONTROL_ALLOW_METHODS, "GET, HEAD")
.header(
hyper::header::ACCESS_CONTROL_ALLOW_HEADERS,
"Origin, X-Requested-With, Content-Type, Accept",
);
let res = if req.method() == hyper::Method::HEAD {
b.body(Full::<Bytes>::default()).unwrap()
} else {
@ -261,10 +272,10 @@ pub async fn make_http_server(
.unwrap()
};
Ok::<_, std::convert::Infallible>(res)
Ok::<_, anyhow::Error>(res)
} else {
// jump to /
let res = hyper::Response::builder()
let res = b
.status(hyper::StatusCode::FOUND)
.header(hyper::header::LOCATION, "/")
.body(Full::<Bytes>::default())

View file

@ -7,6 +7,7 @@ use std::ops::DerefMut;
use std::{collections::HashMap, net::SocketAddr, path::Path, sync::Arc};
use futures::{SinkExt, StreamExt, TryStreamExt};
use hyper::header::HeaderValue;
use hyper::service::service_fn;
use hyper_tungstenite::{tungstenite::Message, HyperWebsocket, HyperWebsocketStream};
use hyper_util::rt::TokioIo;
@ -444,33 +445,51 @@ pub async fn make_http_server(
type Server = hyper_util::server::conn::auto::Builder<hyper_util::rt::TokioExecutor>;
let frontend_html = hyper::body::Bytes::from(frontend_html);
let expected_origin = format!("http://{static_file_addr}");
let make_service = move || {
let frontend_html = frontend_html.clone();
let websocket_tx = websocket_tx.clone();
let expected_origin = expected_origin.clone();
service_fn(move |mut req: hyper::Request<Incoming>| {
let frontend_html = frontend_html.clone();
let websocket_tx = websocket_tx.clone();
let expected_origin = expected_origin.clone();
async move {
// Check if the request is a websocket upgrade request.
if hyper_tungstenite::is_upgrade_request(&req) {
let (response, websocket) = hyper_tungstenite::upgrade(&mut req, None)
.map_err(|e| {
log::error!("Error in websocket upgrade: {e}");
// let e = Error::new(e);
})
.unwrap();
let response = if hyper_tungstenite::is_upgrade_request(&req) {
// Any website a user visits in a browser can connect to our websocket server on
// 127.0.0.1 because CORS does not work for websockets. Thus, check the `Origin`
// header ourselves. See the comment on CORS below for more details.
//
// The VSCode webview panel needs an exception: It doesn't send `http://{static_file_addr}`
// as `Origin`. Instead it sends `vscode-webview://<random>`. Thus, we allow any `Origin`
// starting with `vscode-webview://` as well. I think that's okay from a security point
// of view, because I think malicious websites can't trick browsers into sending
// `vscode-webview://...` as `Origin`.
if req.headers().get("Origin").map_or(false, |h| {
*h == expected_origin || h.as_bytes().starts_with(b"vscode-webview://")
}) {
let (response, websocket) = hyper_tungstenite::upgrade(&mut req, None)
.map_err(|e| {
log::error!("Error in websocket upgrade: {e}");
// let e = Error::new(e);
})
.unwrap();
let _ = websocket_tx.send(websocket);
let _ = websocket_tx.send(websocket);
// Return the response so the spawned future can continue.
Ok(response)
// Return the response so the spawned future can continue.
Ok(response)
} else {
anyhow::bail!("Websocket connection with unexpected `Origin` header. Closing connection");
}
} else if req.uri().path() == "/" {
// log::debug!("Serve frontend: {mode:?}");
let res = hyper::Response::builder()
.header(hyper::header::CONTENT_TYPE, "text/html")
.body(Full::<Bytes>::from(frontend_html))
.unwrap();
Ok::<_, std::convert::Infallible>(res)
Ok::<_, anyhow::Error>(res)
} else {
// jump to /
let res = hyper::Response::builder()
@ -479,7 +498,29 @@ pub async fn make_http_server(
.body(Full::<Bytes>::default())
.unwrap();
Ok(res)
}
};
// When a user visits a website in a browser, that website can try to connect to our http / websocket
// server on `127.0.0.1` which may leak sensitive information. Thus, use CORS to explicitly disallow this.
//
// However, for Websockets, CORS does not work. Thus, we have additional checks of the `Origin` header
// above in the websocket upgrade path.
//
// Stricly speaking, setting the `Acess-Control-Allow-Origin` header is not required here since browsers
// disallow cross origin access also when that header is missing. But I think it's better to be explicit.
//
// Important: This does _not_ protect against malicious users that share the same computer as us (i.e. multi-
// user systems where the users don't trust each other). In this case, malicious attackers can _still_ connect
// to our http / websocket servers (using a browser and otherwise). And additionally they can impersonate
// a tinymist http / websocket server towards a legitimate frontend/html client.
// This requires additional protection that may be added in the future.
response.map(|mut response| {
response.headers_mut().insert(
hyper::header::ACCESS_CONTROL_ALLOW_ORIGIN,
HeaderValue::from_str(&expected_origin).unwrap(),
);
response
})
}
})
};