Merge remote-tracking branch 'gh/main' into wave3/protocol-session

crates/improvise-ws-server/src/lib.rs

@@ -2,9 +2,11 @@
 //! Native only; no auth, no TLS, loopback only. `context/code/clients.md`.
 
 pub mod host;
+pub mod origin;
 pub mod registry;
 pub mod transport;
 
 pub use host::{Addressed, Host};
 pub use improvise_protocol::{ClientFrame, ServerFrame};
+pub use origin::OriginPolicy;
 pub use registry::{ClientId, DocumentId, Registry, Routed};

crates/improvise-ws-server/src/main.rs

@@ -10,6 +10,7 @@ use tokio::net::TcpListener;
 use improvise_command::ui::app::App;
 use improvise_core::workbook::Workbook;
 use improvise_io::persistence;
+use improvise_ws_server::origin::OriginPolicy;
 use improvise_ws_server::registry::{DocumentId, Registry};
 use improvise_ws_server::transport;
 
@@ -30,10 +31,22 @@ struct Args {
     /// unauthenticated, unencrypted, fully writable documents.
     #[arg(long, default_value = "127.0.0.1")]
     bind: String,
+
+    /// An origin a browser page may open a socket from, e.g.
+    /// `https://example.github.io`. Repeatable; loopback is always allowed.
+    #[arg(long = "allow-origin", value_name = "ORIGIN")]
+    allow_origin: Vec<String>,
+}
+
+impl Args {
+    fn origin_policy(&self) -> OriginPolicy {
+        OriginPolicy::new(self.allow_origin.iter().cloned())
+    }
 }
 
 fn main() -> Result<()> {
     let args = Args::parse();
+    let origins = args.origin_policy();
     let registry = open_all(&args.files)?;
     let names: Vec<String> = registry.ids().iter().map(DocumentId::to_string).collect();
 
@@ -47,15 +60,25 @@ fn main() -> Result<()> {
             .await
             .with_context(|| format!("failed to bind {}:{}", args.bind, args.port))?;
         eprintln!(
-            "improvise-ws-server listening on ws://{}:{} (no auth, no TLS)\n  documents: {}",
+            "improvise-ws-server listening on ws://{}:{} (no auth, no TLS)\n  documents: {}\n  \
+             origins: loopback pages{}",
             args.bind,
             args.port,
-            names.join(", ")
+            names.join(", "),
+            allowed_suffix(&args.allow_origin)
         );
-        transport::serve(registry, listener).await
+        transport::serve(registry, listener, origins).await
     }))
 }
 
+/// What the startup banner adds after "loopback pages".
+fn allowed_suffix(named: &[String]) -> String {
+    match named.is_empty() {
+        true => String::new(),
+        false => format!(", {}", named.join(", ")),
+    }
+}
+
 /// Open every named document, or the scratch document if none was named. A
 /// stem collision is fatal: a client could not tell the two documents apart.
 fn open_all(files: &[PathBuf]) -> Result<Registry> {
@@ -140,6 +163,24 @@ mod tests {
         );
     }
 
+    /// The flag is the only way to widen the policy, and with none given the
+    /// deployed https page is refused while the local recipe still works.
+    #[test]
+    fn allow_origin_widens_the_policy_and_nothing_else_does() {
+        let bare = Args::parse_from(["improvise-ws-server"]).origin_policy();
+        assert!(bare.permits(Some("http://localhost:8080")));
+        assert!(!bare.permits(Some("https://fiddlerwoaroof.github.io")));
+
+        let widened = Args::parse_from([
+            "improvise-ws-server",
+            "--allow-origin",
+            "https://fiddlerwoaroof.github.io",
+        ])
+        .origin_policy();
+        assert!(widened.permits(Some("https://fiddlerwoaroof.github.io")));
+        assert!(!widened.permits(Some("https://evil.example")));
+    }
+
     #[test]
     fn no_files_serves_one_scratch_document() {
         let registry = open_all(&[]).unwrap();

crates/improvise-ws-server/src/origin.rs

@@ -0,0 +1,132 @@
+//! Which origins may open a socket. A handshake is exempt from the same-origin
+//! policy, so this is the only thing that can see a hijack: improvise-kc7n.
+
+use std::net::Ipv4Addr;
+
+/// The origins a browser page may open a socket from. Loopback pages are
+/// always allowed; anywhere else has to be named.
+#[derive(Debug, Clone, Default)]
+pub struct OriginPolicy {
+    named: Vec<String>,
+}
+
+impl OriginPolicy {
+    /// `named` is what `--allow-origin` collected: a scheme and an authority,
+    /// with no path, exactly as a browser spells it.
+    pub fn new(named: impl IntoIterator<Item = String>) -> Self {
+        Self {
+            named: named
+                .into_iter()
+                .map(|origin| origin.trim_end_matches('/').to_ascii_lowercase())
+                .collect(),
+        }
+    }
+
+    /// Whether a handshake carrying `origin` may proceed. Absent is a decision
+    /// and not a fall-through: `context/code/clients.md` §Origin argues it.
+    pub fn permits(&self, origin: Option<&str>) -> bool {
+        match origin {
+            // A client that sends no `Origin` is not a browser page: curl, a
+            // native client, this crate's own tests.
+            None => true,
+            Some(origin) => self.permits_present(&origin.to_ascii_lowercase()),
+        }
+    }
+
+    fn permits_present(&self, origin: &str) -> bool {
+        is_loopback(origin) || self.named.iter().any(|allowed| allowed == origin)
+    }
+}
+
+/// Whether `origin` names a page served from this machine. `null` — a
+/// `file://` page or a sandboxed frame — is not one, and that is the point.
+fn is_loopback(origin: &str) -> bool {
+    match origin.split_once("://") {
+        Some(("http" | "https", authority)) => is_loopback_host(host_of(authority)),
+        _ => false,
+    }
+}
+
+/// The host an authority names, without its port. An IPv6 literal keeps its
+/// brackets, which is how a browser spells it anyway.
+fn host_of(authority: &str) -> &str {
+    match authority.rsplit_once(':') {
+        Some((host, port)) if !port.is_empty() && port.bytes().all(|b| b.is_ascii_digit()) => host,
+        _ => authority,
+    }
+}
+
+fn is_loopback_host(host: &str) -> bool {
+    host == "localhost"
+        || host == "[::1]"
+        || host.parse::<Ipv4Addr>().is_ok_and(|ip| ip.is_loopback())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn allowing(origins: &[&str]) -> OriginPolicy {
+        OriginPolicy::new(origins.iter().map(|o| o.to_string()))
+    }
+
+    /// The hole itself: a page the user did not open, driving the socket from
+    /// the user's own allowed address.
+    #[test]
+    fn a_foreign_origin_is_refused() {
+        let policy = OriginPolicy::default();
+        assert!(!policy.permits(Some("https://evil.example")));
+        assert!(!policy.permits(Some("http://improvise.example.com")));
+    }
+
+    /// The shipped local recipe. Every spelling of loopback a browser can put
+    /// in the address bar, on any port, over either scheme.
+    #[test]
+    fn a_loopback_page_is_allowed_by_default() {
+        let policy = OriginPolicy::default();
+        for origin in [
+            "http://localhost:8080",
+            "http://127.0.0.1:8080",
+            "https://localhost",
+            "http://[::1]:8080",
+            "http://127.0.0.2:3000",
+        ] {
+            assert!(policy.permits(Some(origin)), "{origin} must be allowed");
+        }
+    }
+
+    /// `null` is what a `file://` page and a sandboxed frame send, and both of
+    /// those are exactly what the check exists to keep out.
+    #[test]
+    fn the_null_origin_is_not_loopback() {
+        assert!(!OriginPolicy::default().permits(Some("null")));
+    }
+
+    /// A host that merely *contains* a loopback spelling is a different host.
+    #[test]
+    fn a_hostname_that_looks_like_loopback_is_not() {
+        let policy = OriginPolicy::default();
+        assert!(!policy.permits(Some("http://localhost.evil.example")));
+        assert!(!policy.permits(Some("http://127.0.0.1.evil.example")));
+        assert!(!policy.permits(Some("ws://localhost:8080")));
+    }
+
+    /// The deployed page reaches the server through a TLS proxy, so its origin
+    /// is named rather than guessed.
+    #[test]
+    fn a_named_origin_is_allowed_and_its_neighbours_are_not() {
+        let policy = allowing(&["https://fiddlerwoaroof.github.io"]);
+        assert!(policy.permits(Some("https://fiddlerwoaroof.github.io")));
+        assert!(policy.permits(Some("https://FiddlerWoaroof.GitHub.io")));
+        assert!(!policy.permits(Some("http://fiddlerwoaroof.github.io")));
+        assert!(!policy.permits(Some("https://fiddlerwoaroof.github.io.evil.example")));
+    }
+
+    /// Decided rather than defaulted: no browser omits `Origin` on a handshake,
+    /// so an absent one is a client an address check already answers.
+    #[test]
+    fn an_absent_origin_is_allowed() {
+        assert!(OriginPolicy::default().permits(None));
+        assert!(allowing(&["https://example.com"]).permits(None));
+    }
+}

crates/improvise-ws-server/src/transport.rs

@@ -10,12 +10,14 @@ use futures_util::{SinkExt, StreamExt};
 use tokio::net::{TcpListener, TcpStream};
 use tokio::sync::{mpsc, oneshot};
 use tokio_tungstenite::tungstenite::Message;
-use tokio_tungstenite::tungstenite::handshake::server::{Request, Response};
+use tokio_tungstenite::tungstenite::handshake::server::{ErrorResponse, Request, Response};
+use tokio_tungstenite::tungstenite::http::{StatusCode, header};
 use tokio_tungstenite::tungstenite::protocol::CloseFrame;
 use tokio_tungstenite::tungstenite::protocol::frame::coding::CloseCode;
 
 #[cfg(doc)]
 use crate::host::Host;
+use crate::origin::OriginPolicy;
 use crate::registry::{ClientId, ConnectError, Registry, Routed};
 use improvise_protocol::{ClientFrame, ServerFrame};
 
@@ -36,15 +38,17 @@ enum Inbound {
 
 /// Accept connections on `listener` until it fails, serving every document in
 /// `registry`, on the current thread in a `LocalSet` the caller supplies.
-pub async fn serve(registry: Registry, listener: TcpListener) -> Result<()> {
+pub async fn serve(registry: Registry, listener: TcpListener, origins: OriginPolicy) -> Result<()> {
     let (tx, rx) = mpsc::unbounded_channel();
     let documents = tokio::task::spawn_local(registry_loop(registry, rx));
+    let origins = Rc::new(origins);
 
     while let Ok((stream, _peer)) = listener.accept().await {
         let tx = tx.clone();
+        let origins = Rc::clone(&origins);
         tokio::task::spawn_local(async move {
             // A connection that goes wrong is that connection's problem.
-            let _ = connection(stream, tx).await;
+            let _ = connection(stream, tx, origins).await;
         });
     }
     drop(tx);
@@ -114,14 +118,23 @@ fn attach(
 /// One websocket connection, from handshake to close. The large error type is
 /// tungstenite's and is never returned: refusal is the registry's answer.
 #[allow(clippy::result_large_err)]
-async fn connection(stream: TcpStream, tx: mpsc::UnboundedSender<Inbound>) -> Result<()> {
+async fn connection(
+    stream: TcpStream,
+    tx: mpsc::UnboundedSender<Inbound>,
+    origins: Rc<OriginPolicy>,
+) -> Result<()> {
     // The document is named in the URL, which is readable only during the
     // handshake, so it is captured out of the callback.
     let requested: Rc<RefCell<Option<String>>> = Rc::new(RefCell::new(None));
     let seen = Rc::clone(&requested);
     let ws = tokio_tungstenite::accept_hdr_async(stream, move |req: &Request, res: Response| {
-        *seen.borrow_mut() = document_of(req.uri().path());
-        Ok(res)
+        match admitted(&origins, req) {
+            true => {
+                *seen.borrow_mut() = document_of(req.uri().path());
+                Ok(res)
+            }
+            false => Err(forbidden()),
+        }
     })
     .await
     .context("websocket handshake failed")?;
@@ -185,6 +198,29 @@ async fn connection(stream: TcpStream, tx: mpsc::UnboundedSender<Inbound>) -> Re
     Ok(())
 }
 
+/// Whether `origins` permits this handshake. Two `Origin` headers, or one that
+/// is not text, name no origin a policy could answer for and are refused.
+fn admitted(origins: &OriginPolicy, req: &Request) -> bool {
+    let mut stated = req.headers().get_all(header::ORIGIN).iter();
+    match (stated.next(), stated.next()) {
+        (None, _) => origins.permits(None),
+        (Some(value), None) => value
+            .to_str()
+            .is_ok_and(|origin| origins.permits(Some(origin))),
+        (Some(_), Some(_)) => false,
+    }
+}
+
+/// What a refused handshake is answered with. A browser surfaces none of it to
+/// the page, so the wording is for whoever is reading a proxy log.
+fn forbidden() -> ErrorResponse {
+    let mut response = ErrorResponse::new(Some(
+        "origin not allowed; name it with --allow-origin".to_string(),
+    ));
+    *response.status_mut() = StatusCode::FORBIDDEN;
+    response
+}
+
 /// The document a request path names; `/` and `` are "whichever one there is".
 /// No unescaping: a `DocumentId` is a name a host chose, not a path.
 fn document_of(path: &str) -> Option<String> {
@@ -230,4 +266,59 @@ mod tests {
         // name, so a nested-looking path is simply a name nothing serves.
         assert_eq!(document_of("/a/b"), Some("a/b".to_string()));
     }
+
+    fn handshake(origin: Option<&[u8]>) -> Request {
+        let mut req = Request::builder().uri("/").body(()).unwrap();
+        if let Some(origin) = origin {
+            req.headers_mut().insert(
+                header::ORIGIN,
+                header::HeaderValue::from_bytes(origin).unwrap(),
+            );
+        }
+        req
+    }
+
+    /// A proxy that appends rather than sets `Origin` leaves two, and reading
+    /// the first would let the second be anything at all.
+    #[test]
+    fn two_origins_state_none_this_policy_can_answer_for() {
+        let mut req = handshake(Some(b"http://localhost:8080"));
+        req.headers_mut().append(
+            header::ORIGIN,
+            header::HeaderValue::from_static("https://evil.example"),
+        );
+        assert_eq!(req.headers().get_all(header::ORIGIN).iter().count(), 2);
+        assert!(!admitted(&OriginPolicy::default(), &req));
+
+        // And the allowed one alone is still allowed, so the refusal above is
+        // the second header and not the first.
+        assert!(admitted(
+            &OriginPolicy::default(),
+            &handshake(Some(b"http://localhost:8080"))
+        ));
+    }
+
+    /// The seam between the policy and the handshake, including the one case
+    /// the policy cannot be asked about: an `Origin` that is not text.
+    #[test]
+    fn the_handshake_refuses_what_the_policy_refuses() {
+        let origins = OriginPolicy::default();
+        assert!(admitted(
+            &origins,
+            &handshake(Some(b"http://localhost:8080"))
+        ));
+        assert!(admitted(&origins, &handshake(None)));
+        assert!(!admitted(
+            &origins,
+            &handshake(Some(b"https://evil.example"))
+        ));
+        assert!(!admitted(&origins, &handshake(Some(b"http://\xffevil"))));
+    }
+
+    /// A refusal has to be a refused upgrade rather than an accepted socket
+    /// the server then ignores.
+    #[test]
+    fn a_refusal_is_a_forbidden_response() {
+        assert_eq!(forbidden().status(), StatusCode::FORBIDDEN);
+    }
 }

crates/improvise-ws-server/tests/socket.rs

@@ -4,6 +4,7 @@
 use futures_util::{SinkExt, StreamExt};
 use tokio::net::TcpListener;
 use tokio_tungstenite::tungstenite::Message;
+use tokio_tungstenite::tungstenite::http::StatusCode;
 
 use improvise_command::ui::app::App;
 use improvise_core::model::cell::CellValue;
@@ -13,6 +14,7 @@ use improvise_protocol::{
     BasisCache, CellId, Command, GridCache, GridUpdate, ModelCommand, Projection, WidgetCache,
 };
 use improvise_protocol::{ClientFrame, ServerFrame};
+use improvise_ws_server::origin::OriginPolicy;
 use improvise_ws_server::registry::{DocumentId, Registry};
 
 /// One document, served as `quarterly`.
@@ -50,7 +52,12 @@ fn a_client_on_a_socket_is_seeded_and_gets_its_edit_back() {
         let listener = TcpListener::bind(("127.0.0.1", 0)).await.unwrap();
         let addr = listener.local_addr().unwrap();
         tokio::task::spawn_local(async move {
-            let _ = improvise_ws_server::transport::serve(registry(), listener).await;
+            let _ = improvise_ws_server::transport::serve(
+                registry(),
+                listener,
+                OriginPolicy::default(),
+            )
+            .await;
         });
 
         // No path: with one document served, `/` reaches it, exactly as it
@@ -198,7 +205,9 @@ fn the_url_path_picks_the_document() {
             registry
                 .open(DocumentId::new("beta"), named_app("Beta", "Bison"))
                 .unwrap();
-            let _ = improvise_ws_server::transport::serve(registry, listener).await;
+            let _ =
+                improvise_ws_server::transport::serve(registry, listener, OriginPolicy::default())
+                    .await;
         });
 
         for (path, row) in [("alpha", "Aardvark"), ("beta", "Bison")] {
@@ -262,3 +271,111 @@ fn named_app(name: &str, row: &str) -> App {
     app.rebuild_layout();
     app
 }
+
+/// A hostile page an allowed user visits opens a socket from that user's own
+/// allowed address. Only `Origin` separates it from the real page.
+#[test]
+fn a_handshake_from_a_foreign_origin_is_refused() {
+    with_server(OriginPolicy::default(), |addr| async move {
+        let Err(refused) =
+            tokio_tungstenite::connect_async(request(addr, Some("https://evil.example"))).await
+        else {
+            panic!("a foreign origin must not reach the document");
+        };
+        // The kind, not the wording: a refused upgrade and a dropped socket
+        // are different outcomes and only one of them is this fix.
+        let tokio_tungstenite::tungstenite::Error::Http(answer) = refused else {
+            panic!("the handshake failed for some other reason: {refused}");
+        };
+        assert_eq!(answer.status(), StatusCode::FORBIDDEN);
+    });
+}
+
+/// The shipped local recipe: the page is served by `python3 -m http.server`
+/// on loopback and opens a socket to the server on another loopback port.
+#[test]
+fn a_handshake_from_a_loopback_page_is_accepted() {
+    with_server(OriginPolicy::default(), |addr| async move {
+        let (mut ws, _) =
+            tokio_tungstenite::connect_async(request(addr, Some("http://localhost:8080")))
+                .await
+                .expect("the local pairing recipe must keep working");
+        send(&mut ws, ClientFrame::Viewport { rows: 10, cols: 10 }).await;
+        assert!(matches!(recv(&mut ws).await, ServerFrame::Snapshot(_)));
+    });
+}
+
+/// Non-browser clients — curl, a native client, this file's other tests —
+/// send no `Origin` at all, and absent is decided rather than defaulted.
+#[test]
+fn a_handshake_with_no_origin_is_accepted() {
+    with_server(OriginPolicy::default(), |addr| async move {
+        let (mut ws, _) = tokio_tungstenite::connect_async(request(addr, None))
+            .await
+            .expect("a client that sends no Origin is not a browser page");
+        send(&mut ws, ClientFrame::Viewport { rows: 10, cols: 10 }).await;
+        assert!(matches!(recv(&mut ws).await, ServerFrame::Snapshot(_)));
+    });
+}
+
+/// The deployed page reaches the server through a TLS proxy, and `Origin` is
+/// the one thing that survives the proxy to say which page it was.
+#[test]
+fn a_named_origin_reaches_the_document() {
+    let named = OriginPolicy::new(["https://fiddlerwoaroof.github.io".to_string()]);
+    with_server(named, |addr| async move {
+        let (mut ws, _) = tokio_tungstenite::connect_async(request(
+            addr,
+            Some("https://fiddlerwoaroof.github.io"),
+        ))
+        .await
+        .expect("--allow-origin must actually widen the policy");
+        send(&mut ws, ClientFrame::Viewport { rows: 10, cols: 10 }).await;
+        assert!(matches!(recv(&mut ws).await, ServerFrame::Snapshot(_)));
+
+        // Widening for one page does not widen for its neighbours.
+        assert!(
+            tokio_tungstenite::connect_async(request(addr, Some("https://evil.example")))
+                .await
+                .is_err()
+        );
+    });
+}
+
+/// A handshake request for `addr`, carrying `origin` when there is one.
+fn request(
+    addr: std::net::SocketAddr,
+    origin: Option<&str>,
+) -> tokio_tungstenite::tungstenite::http::Request<()> {
+    use tokio_tungstenite::tungstenite::client::IntoClientRequest;
+    let mut request = format!("ws://{addr}/")
+        .into_client_request()
+        .expect("a loopback address is a valid websocket url");
+    if let Some(origin) = origin {
+        request
+            .headers_mut()
+            .insert("Origin", origin.parse().expect("a header value"));
+    }
+    request
+}
+
+/// Serve the one-document fixture on an ephemeral port and run `body`.
+fn with_server<F, Fut>(origins: OriginPolicy, body: F)
+where
+    F: FnOnce(std::net::SocketAddr) -> Fut,
+    Fut: std::future::Future<Output = ()>,
+{
+    let runtime = tokio::runtime::Builder::new_current_thread()
+        .enable_all()
+        .build()
+        .unwrap();
+    let local = tokio::task::LocalSet::new();
+    runtime.block_on(local.run_until(async move {
+        let listener = TcpListener::bind(("127.0.0.1", 0)).await.unwrap();
+        let addr = listener.local_addr().unwrap();
+        tokio::task::spawn_local(async move {
+            let _ = improvise_ws_server::transport::serve(registry(), listener, origins).await;
+        });
+        body(addr).await;
+    }));
+}