From 8f0aafa9efa6fbc3b76d180ef1f7e87172735a7a Mon Sep 17 00:00:00 2001 From: Tran Duc Anh Dang Date: Thu, 12 Nov 2026 18:37:10 +1100 Subject: [PATCH] test(device-client): prove the real transport never follows a redirect A loopback server answers 301, 302, 303, 307 and 308 with a Location to a second listener. Every method with a bearer token comes back as the redirect answer and the second listener never gets a connection. Plain http is also refused for any host that is not a loopback IP literal. --- crates/orbyn-device-client/tests/redirects.rs | 121 ++++++++++++++++++ docs/device-client.md | 1 + 2 files changed, 122 insertions(+) create mode 100644 crates/orbyn-device-client/tests/redirects.rs diff --git a/crates/orbyn-device-client/tests/redirects.rs b/crates/orbyn-device-client/tests/redirects.rs new file mode 100644 index 0000000..0d99209 --- /dev/null +++ b/crates/orbyn-device-client/tests/redirects.rs @@ -0,0 +1,121 @@ +//! The real transport never follows a redirect: a redirect could carry the bearer token to +//! another host. A loopback server answers every kind of redirect with a `Location` that points +//! at a second loopback listener, and that listener must never see a connection. + +use orbyn_device_client::{DeviceApi, DeviceError, HttpApi, Method, Request, Zeroizing}; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::Duration; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpListener; + +/// A listener that only counts the connections it accepts. +async fn counting_listener() -> (String, Arc) { + let listener = TcpListener::bind("127.0.0.1:0").await.expect("bind"); + let address = format!("http://{}", listener.local_addr().expect("addr")); + let count = Arc::new(AtomicUsize::new(0)); + let seen = count.clone(); + tokio::spawn(async move { + loop { + let Ok((mut socket, _)) = listener.accept().await else { + return; + }; + seen.fetch_add(1, Ordering::SeqCst); + // Answer, so a client that did follow would not hang. + let _ = socket + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n") + .await; + } + }); + (address, count) +} + +/// A server that answers every request with `status` and a `Location` to `target`. +async fn redirecting_listener(status: &'static str, target: String) -> String { + let listener = TcpListener::bind("127.0.0.1:0").await.expect("bind"); + let address = format!("http://{}", listener.local_addr().expect("addr")); + tokio::spawn(async move { + loop { + let Ok((mut socket, _)) = listener.accept().await else { + return; + }; + let target = target.clone(); + tokio::spawn(async move { + let mut buffer = vec![0u8; 8192]; + let _ = socket.read(&mut buffer).await; + let reply = format!( + "HTTP/1.1 {status}\r\nLocation: {target}/stolen\r\nContent-Length: 0\r\nConnection: close\r\n\r\n" + ); + let _ = socket.write_all(reply.as_bytes()).await; + let _ = socket.shutdown().await; + }); + } + }); + address +} + +fn request(method: Method) -> Request { + Request { + method, + path: "/v1/devices".to_owned(), + bearer: Some(Zeroizing::new("secret-token".to_owned())), + body: match method { + Method::Get | Method::Delete => None, + Method::Post | Method::Put => Some(Zeroizing::new(b"{}".to_vec())), + }, + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_redirect_is_returned_as_the_answer_and_the_target_is_never_contacted() { + let (target, contacted) = counting_listener().await; + for (status, code) in [ + ("301 Moved Permanently", 301), + ("302 Found", 302), + ("303 See Other", 303), + ("307 Temporary Redirect", 307), + ("308 Permanent Redirect", 308), + ] { + let first = redirecting_listener(status, target.clone()).await; + let api = HttpApi::new(&first).expect("loopback http is allowed"); + // Every method, with the bearer token: 307 and 308 would repeat the body and the header. + for method in [Method::Get, Method::Post, Method::Put, Method::Delete] { + let response = api + .send(request(method)) + .await + .expect("the redirect is an answer, not followed"); + assert_eq!(response.status, code, "{status} {method:?}"); + } + } + // Give a follower time to connect, then check that nothing did. + tokio::time::sleep(Duration::from_millis(300)).await; + assert_eq!( + contacted.load(Ordering::SeqCst), + 0, + "the redirect target was contacted" + ); +} + +#[test] +fn plain_http_is_refused_for_any_host_that_is_not_a_loopback_ip_literal() { + for refused in [ + "http://example.com", + "http://localhost", + "http://10.0.0.1", + "http://192.168.1.5:8080", + "http://[2001:db8::1]", + "http://127.0.0.1.example.com", + ] { + assert!( + matches!(HttpApi::new(refused), Err(DeviceError::InvalidInput(_))), + "{refused}" + ); + } + for allowed in [ + "http://127.0.0.1:8080", + "http://[::1]:8080", + "https://api.orbyn.dev", + ] { + assert!(HttpApi::new(allowed).is_ok(), "{allowed}"); + } +} diff --git a/docs/device-client.md b/docs/device-client.md index e18b2da..799c72c 100644 --- a/docs/device-client.md +++ b/docs/device-client.md @@ -86,6 +86,7 @@ An error never carries a password, a token, a code, a key or a URL. `Request` an ## Tests +- `tests/redirects.rs` serves 301, 302, 303, 307 and 308 from a loopback server with a `Location` to a second listener, sends every method with a bearer token through `HttpApi`, and checks that each redirect comes back as the answer and the second listener never gets a connection; it also checks that plain `http` is refused for any host that is not a loopback IP literal. - `tests/client.rs` runs the client against an in-memory engine (`tests/support`) that speaks the **real OPAQUE server side** and checks the real signatures: proof of work, invite, code, trusted start, second factor, re-login after eleven minutes, account gone, rate limit, and that no request body holds the password. The same file serves a raw HTTP reply to check that redirects are not followed and `Retry-After` is read. - `tests/local_stack.rs` (ignored by default, and skipped with a printed note unless `ORBYN_LOCAL_STACK=1`, so `cargo test -- --include-ignored` in CI passes without a stack) runs sign-up, a second pending device, a wrong password, the re-login and sign-out against the real engine on `scripts/local.sh up`, using `scripts/local.sh invite-code` and `scripts/local.sh latest-code `.