fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: check Origin at the handshake, so a page cannot drive someone's session

Browse Source 
improvise-kc7n. A websocket handshake is exempt from the same-origin policy —
there is no CORS preflight — so any page a user visited could open a socket to
improvise-ws-server from that user's own allowed address and read and edit every
document served. An address check cannot see it: the request genuinely comes
from an allowed client, and only Origin says what is driving it. Loopback is the
least protected case, having no proxy in front of it at all.

`src/origin.rs` is the decision and `transport::admitted` its one caller, which
refuses the upgrade with a 403 rather than accepting and ignoring. The default
allows pages served from loopback — which is the shipped local recipe, page on
`http://localhost:8080` and socket on another loopback port — and refuses
everything else; `--allow-origin` names the deployed page, which reaches the
server through a TLS proxy and is not loopback. An absent Origin is allowed and
decided rather than defaulted: no browser omits it, so absent is a non-browser
client, and refusing those would break this crate's own tests while buying
nothing that --bind does not already buy.

Five edits, five guards, each reverted alone against the named test:

- origin.rs permits() present arm  -> a_foreign_origin_is_refused
- origin.rs permits() None arm     -> a_handshake_with_no_origin_is_accepted
- origin.rs is_loopback()          -> a_handshake_from_a_loopback_page_is_accepted
- transport.rs handshake callback  -> a_handshake_from_a_foreign_origin_is_refused
- main.rs --allow-origin wiring    -> allow_origin_widens_the_policy_and_nothing_else_does

`transport::serve` takes the policy rather than defaulting one, so a host cannot
forget to pass it. The argument is context/code/clients.md §Origin and the
evidence context/test-evidence/clients.md; README no longer says the server
never looks, and the proxy's Origin allow-list is now defence in depth.

This is not authentication — it answers what page is driving a client, never who
is connecting. improvise-kpnr is untouched and still open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-20 21:20:18 -07:00
parent 2afbf37b67
commit d100639348
8 changed files with 517 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+32 -31
View File
@@ -236,26 +236,19 @@ optimising for: `http://localhost` is not a secure-context problem, so nothing
below is needed to develop, to demo locally, or to share a TUI-hosted document
with another tab on the same machine.
One caveat even there, for the reason spelled out below: a WebSocket is exempt
from the same-origin policy, and **nothing in the server checks `Origin`** — so
a page you did not open can connect to `ws://127.0.0.1:8787` and read or edit
the document while the server runs.
The server checks `Origin` on the handshake, which is what makes that safe. A
WebSocket is exempt from the same-origin policy — there is no CORS preflight —
so without the check any page you visited could connect to `ws://127.0.0.1:8787`
and read or edit the document while the server runs. **Pages served from
loopback are allowed by default**, which is exactly the recipe above; every
other page is refused unless you name it with `--allow-origin`. A client that
sends no `Origin` at all — curl, a script, a native client — is allowed, because
no browser omits it and refusing them would answer a question `--bind` already
answers.
How much the *browser* mitigates that is in flux and should not be leaned on.
Chromium has been rolling out **Local Network Access**, which puts requests
from a public-network origin to a loopback or private-network address behind a
permission prompt; per Chrome's own documentation WebSockets were **not yet**
covered when the prompt began shipping, with coverage planned, so whether a
given browser and version gates *this* socket is not something this project can
state — and nothing here can test it: the Playwright harness serves a loopback
origin, so Local Network Access never engages in it. Two gaps are structural
rather than version-dependent: it is a Chromium feature, so other browsers need
not behave the same; and it only engages for a **public** origin, so the
locally-served page this section is about is outside it entirely.
Treat it, then, as tolerable for a local session you start and stop, and as a
reason not to leave one running unattended. The durable fix is the server
checking `Origin` itself, which it does not yet do.
This is not authentication (see the next section) and it does not depend on the
browser: whatever Chromium's **Local Network Access** work ends up covering, the
handshake is refused here rather than at the browser's discretion.
### Sharing a TUI-hosted document with a remote browser
@@ -309,9 +302,13 @@ any hostile page an allowed user visits can open a socket to your endpoint
*from that user's own allowed address*, and read and write the document. An
address check cannot see that, because by then the request genuinely does come
from an allowed client. Only the **`Origin` header** distinguishes them, and
`improvise-ws-server` **never looks at it** — it accepts every handshake. So a
proxy needs an Origin allow-list **in addition to**, never instead of,
whichever control below you pick. The worked example carries both.
`improvise-ws-server` checks it: pages served from loopback are allowed, and
anything else must be named with `--allow-origin`. **Deploying behind a proxy
therefore needs the flag** — the deployed page's origin is
`https://fiddlerwoaroof.github.io`, which is not loopback. Keep the Origin
allow-list in the proxy as well: it is defence in depth now rather than the only
defence, and it is what covers a proxy configuration that strips or rewrites the
header before the server can see it.
So the honest controls are the ones that need **no credential from the page**:
@@ -350,11 +347,11 @@ improvise.example.com {
@denied not remote_ip 203.0.113.4 198.51.100.0/24
abort @denied
# ALSO not optional, and it is NOT redundant with the address check.
# A WebSocket is exempt from the same-origin policy, so any hostile page
# an allowed user visits can open this socket FROM THAT ALLOWED ADDRESS
# and read and edit the document. The address check cannot see that; only
# the Origin can. Name the exact origins the page is served from.
# Defence in depth, and NOT redundant with the address check. A WebSocket
# is exempt from the same-origin policy, so any hostile page an allowed
# user visits can open this socket FROM THAT ALLOWED ADDRESS. The address
# check cannot see that; only the Origin can. The server checks it too —
# this is what covers a proxy that strips or rewrites the header.
@badorigin not header Origin https://fiddlerwoaroof.github.io
abort @badorigin
@@ -363,8 +360,11 @@ improvise.example.com {
```
```sh
improvise-ws-server bank-info.improv --port 8787 # still plain ws, still loopback
caddy run # terminates TLS, proxies to it
# --allow-origin is required here: the deployed page is not a loopback origin,
# so without it the server refuses the handshake the proxy forwards.
improvise-ws-server bank-info.improv --port 8787 \
--allow-origin https://fiddlerwoaroof.github.io # still plain ws, still loopback
caddy run # terminates TLS, proxies to it
```
On the same terms as the browser caveat below: **this configuration has not
@@ -388,8 +388,9 @@ answered as an ordinary request and the socket never opens.
For a one-off share with no DNS name and no certificate of your own,
`cloudflared tunnel --url http://127.0.0.1:8787` or `ngrok http 8787` each
print an https hostname that works the same way — pass the hostname to `?ws=`
without a scheme. Both print a **public** URL and neither authenticates
anything by default, and an unguessable URL is not a control.
without a scheme, and pass the page's own origin to `--allow-origin`, since it
is not loopback. Both print a **public** URL and neither authenticates anything
by default, and an unguessable URL is not a control.
Two things carry over from above. `ngrok http --basic-auth` does **not** help
here — the page cannot answer a basic-auth challenge on a socket handshake —
context/code/clients.md
+47
View File
@@ -255,6 +255,53 @@ document it serves, so it binds a loopback address by default and `serve` says
so. `--bind` exists so the limit is a choice you can see rather than one buried
in the code. ADR 001 owns what multi-writer safety becomes.
### Origin
`src/origin.rs` is the handshake's one access control, and the reason it cannot
be delegated: **a websocket handshake is exempt from the same-origin policy**.
There is no CORS preflight, so any page a user visits can open a socket to the
endpoint *from that user's own address*, and read and edit every document the
server holds — cross-site websocket hijacking (improvise-kc7n). An address
check cannot see it, because the request genuinely comes from an allowed
address; an IP allow-list, a private network and loopback binding all answer
"may this **client** reach the endpoint", and the attacker's answer is honestly
yes. Only `Origin` says what is **driving** the client, which is
`doctrine(who_may_connect_is_two_questions)`. **Loopback is the least protected
case, not the safest**: there is no proxy in front of it to check anything, and
loopback is not a boundary a browser enforces, so while the server runs any page
open in that browser can reach `ws://127.0.0.1:8787`.
**The default allows loopback pages and nothing else**, because a default that
fails open is worth nothing and a default that breaks the shipped recipe is a
different bug. The two shipped paths were the constraint. The local pairing
recipe serves the page with `python3 -m http.server 8080 --directory web`, so
its origin is `http://localhost:8080` while the socket is on another loopback
port — loopback is therefore the *default*, not a flag, or the one case worth
optimising for would need configuring. The deployed Pages page sends
`https://fiddlerwoaroof.github.io` and can only reach the server through a TLS
proxy (`improvise-ws-server` speaks plain `ws://`), which is already a
multi-step setup with an Origin allow-list in it; one `--allow-origin` is
therefore an honest cost there and the project's own deployment origin is not
hard-coded as a default. `OriginPolicy::permits` is the whole decision, and the
transport's `admitted` is the only caller.
**An absent `Origin` is allowed, decided rather than defaulted.** Every browser
sends one on a handshake, unconditionally — unlike `fetch`, there is no
no-`Origin` mode to fall into — so an absent one is a non-browser client: curl,
a native client, this crate's own `tests/socket.rs`. Refusing those would break
the repo's own tests and every scripted client while buying nothing, since an
attacker who can run a non-browser client already has direct network access,
which is the question `--bind` and a proxy answer. The cost is stated rather
than hidden: **a proxy that strips `Origin` turns this check off**, so the
proxy's own Origin allow-list stays in the recipe as defence in depth. An
`Origin` header that is not valid text is refused rather than read as absent —
it is not a value any policy could name.
**What this does not buy is authentication.** It answers "what page is driving
this client", never "who is connecting"; anything that is not a browser page
sends no `Origin` and is unaffected by any of it. improvise-kpnr is still open
and is a different question.
---
## `improvise-protocol/src/resolve.rs` — one keypress → `Command`s
context/test-evidence/clients.md
+68
View File
@@ -2062,3 +2062,71 @@ done
The other two losses are deliberate: `pointer.spec.js`'s sidebar test was
superseded by the open-at-launch ruling, and the widgets file was merged into
this one.
---
## `improvise-ws-server` — the `Origin` check (improvise-kc7n)
The bug: `grep -rni origin crates/improvise-ws-server/src/` returned nothing.
The server inspected no `Origin` header and accepted every handshake, and a
websocket handshake is exempt from the same-origin policy — no CORS preflight —
so any page a user visited could open a socket from that user's own allowed
address and read and edit every document served. An address check cannot see
it; the argument is `context/code/clients.md` §Origin.
Five edits, five guards. Each was reverted alone and the named test failed;
nothing here is covered only by a comment.
### a_handshake_from_a_foreign_origin_is_refused
The reproduction, written first and watched fail on `2afbf37`: a handshake
carrying `Origin: https://evil.example` connected, and the test's
`connect_async(...).err()` was `None`. After the fix the upgrade is refused with
a 403 and the assertion is on the status, not merely on "an error happened" —
a refused upgrade and a dropped connection are different outcomes and only one
of them is the fix.
**Injection:** replacing the handshake callback's `admitted(&origins, req)` with
`Ok(())` — the transport collecting a policy it never asks — fails this and
`a_named_origin_reaches_the_document`, and nothing else in the workspace.
### a_handshake_with_no_origin_is_accepted
The case most likely to be wrong later, so it is named rather than left to a
match arm. Non-browser clients send no `Origin`: curl, a native client, this
file's own other tests. **Injection:** flipping `OriginPolicy::permits`'s `None`
arm to `false` fails this, `an_absent_origin_is_allowed`,
`the_handshake_refuses_what_the_policy_refuses`, and *both pre-existing socket
tests* — which is the point: the repo's own instrument is the non-browser client
this arm is about.
### a_handshake_from_a_loopback_page_is_accepted
The shipped local recipe — the page served by `python3 -m http.server 8080`,
the socket on another loopback port — must keep working with no flag, or the
default is a different bug from the one being fixed. **Injection:** making
`is_loopback` return `false` fails this and
`a_loopback_page_is_allowed_by_default` and leaves the refusal tests green,
which separates "allows loopback" from "allows everything".
### a_named_origin_reaches_the_document
`--allow-origin` end to end over a real socket, plus the negative arm in the
same test: widening for one page does not widen for its neighbours. Without
that second assertion the test passes against a policy that permits everything,
which is the fault E1a injects.
### allow_origin_widens_the_policy_and_nothing_else_does
The wiring, which no socket test can reach: the other tests build an
`OriginPolicy` directly, so a flag collected by clap and dropped on the floor is
invisible to every one of them. This drives `Args::parse_from` and asserts both
directions — bare refuses the deployed https origin and still allows loopback;
widened allows the named origin and not its neighbours. **Injection:** making
`Args::origin_policy` return `OriginPolicy::default()` fails this test alone.
What the signature covers instead of a test: `transport::serve` **takes** the
policy, so a host cannot forget to pass one — `doctrine(an_invariant_is_made_
unrepresentable_not_validated)`. What remains uncovered and is stated rather
than guarded: a proxy that strips `Origin` turns the check off, which is why the
proxy's own Origin allow-list stays in `README.md` as defence in depth.
crates/improvise-ws-server/src/lib.rs
+2
View File
@@ -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
+45 -3
View File
@@ -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,23 @@ 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, such as
/// `https://example.github.io`. Repeatable. Loopback pages are always
/// allowed; every other page is refused unless it is named here.
#[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 +61,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 +164,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
+132
View File
@@ -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
+75 -6
View File
@@ -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,27 @@ async fn connection(stream: TcpStream, tx: mpsc::UnboundedSender<Inbound>) -> Re
Ok(())
}
/// Whether `origins` permits this handshake. An `Origin` that is not text is not
/// one a policy can name, so it is refused rather than read as absent.
fn admitted(origins: &OriginPolicy, req: &Request) -> bool {
match req.headers().get(header::ORIGIN) {
None => origins.permits(None),
Some(value) => value
.to_str()
.is_ok_and(|origin| origins.permits(Some(origin))),
}
}
/// 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 +264,39 @@ 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
}
/// 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
+116 -2
View File
@@ -13,6 +13,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 +51,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 +204,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 +270,109 @@ 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");
};
assert!(
format!("{refused}").contains("403"),
"the upgrade must be refused, not accepted and ignored: {refused}"
);
});
}
/// 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;
}));
}