diff --git a/apps/api/src/auth/services/challenge.service.test.ts b/apps/api/src/auth/services/challenge.service.test.ts index 003c10dd5..197c1fa75 100644 --- a/apps/api/src/auth/services/challenge.service.test.ts +++ b/apps/api/src/auth/services/challenge.service.test.ts @@ -21,6 +21,14 @@ describe('ChallengeService', () => { expect(expiresAt.getTime()).toBe(clock.now().getTime() + 300_000); }); + // The engine pins this exact shape before it will sign a challenge, and + // refuses anything else. Narrowing the tail here fails every login, so the + // contract is pinned on both sides rather than only in Rust. + it('issues a tail the engine will sign: 32 bytes of lowercase hex', () => { + const { challenge } = service.issueIdentityChallenge(PUBLIC_KEY); + expect(challenge).toMatch(/^cipherbox-login:v2:[0-9a-f]{64}$/); + }); + it('consumes a live challenge exactly once', () => { const { challenge } = service.issueIdentityChallenge(PUBLIC_KEY); service.consume(challenge, 'identity', PUBLIC_KEY); diff --git a/crates/desktop-seams/src/http.rs b/crates/desktop-seams/src/http.rs index 28343bd60..837f7da57 100644 --- a/crates/desktop-seams/src/http.rs +++ b/crates/desktop-seams/src/http.rs @@ -34,25 +34,8 @@ impl ReqwestHttp { pub fn new() -> SeamResult { let client = reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) - .redirect(reqwest::redirect::Policy::custom(|attempt| { - // The engine decides over which transport a request carrying an - // `Authorization` header may go (blueprint/engine.md "Content - // plane"). A redirect that downgrades to plaintext would carry - // the credential onto the clear network past that decision, so - // it is surfaced as the 3xx it is rather than followed. - let downgrade = attempt.url().scheme() == "http" - && attempt - .previous() - .last() - .is_some_and(|previous| previous.scheme() == "https"); - if downgrade { - attempt.stop() - } else if attempt.previous().len() > 10 { - attempt.error("too many redirects") - } else { - attempt.follow() - } - })) + // No redirects; see the `Http` seam contract. + .redirect(reqwest::redirect::Policy::none()) .build() .map_err(|err| SeamError::new(format!("http client build: {err}")))?; Ok(Self { client }) diff --git a/crates/desktop-seams/tests/conformance.rs b/crates/desktop-seams/tests/conformance.rs index a87339482..2e50cfaf0 100644 --- a/crates/desktop-seams/tests/conformance.rs +++ b/crates/desktop-seams/tests/conformance.rs @@ -337,6 +337,34 @@ async fn reqwest_http_returns_non_2xx_as_a_response_not_an_error() { assert_eq!(response.status, 418); } +/// The seam follows no redirect: the 3xx comes back as the response it is, and +/// the `Authorization` header never reaches the hop's target. A doc comment +/// cannot fail CI, so the policy is asserted against a live server. +#[tokio::test] +async fn reqwest_http_follows_no_redirect_and_does_not_replay_the_bearer() { + let server = MockServer::start(); + let http = ReqwestHttp::new().expect("client builds"); + + let response = http + .send(HttpRequest { + method: HttpMethod::Get, + url: format!("{}/redirect", server.base_url()), + headers: vec![("Authorization".into(), "Bearer member-token".into())], + body: None, + credentials: HttpCredentials::Omit, + timeout_ms: None, + }) + .await + .expect("a 3xx is a response, never a seam Err"); + + assert_eq!(response.status, 302, "the hop is surfaced, not taken"); + + // `/redirect` served no body and `/echo` would have echoed one, so the + // recorded request proves the target was never reached. + let recorded = server.last_request().expect("a request was recorded"); + assert_eq!(recorded.path, "/redirect"); +} + #[tokio::test] async fn reqwest_http_capped_fetch_rejects_a_chunk_larger_than_the_cap() { let server = MockServer::start(); diff --git a/crates/desktop-seams/tests/mock_http/mod.rs b/crates/desktop-seams/tests/mock_http/mod.rs index 33c2e17a3..cc729295a 100644 --- a/crates/desktop-seams/tests/mock_http/mod.rs +++ b/crates/desktop-seams/tests/mock_http/mod.rs @@ -10,6 +10,7 @@ //! header; records the request for assertions. //! - `GET /teapot` — returns 418, to prove a non-2xx status is a response, //! not a seam error. +//! - `GET /redirect` — 302 to `/echo`, to prove the seam follows no hop. //! - `GET /stream/` — `n` bytes with `Transfer-Encoding: chunked` and no //! `Content-Length`, so a capped read has only the streaming drain to gate //! on. @@ -28,6 +29,8 @@ use std::time::Duration; /// One request the server received, captured for test assertions. #[derive(Clone)] pub struct RecordedRequest { + /// Request target, so a test can prove which route was reached. + pub path: String, pub headers: Vec<(String, String)>, pub body: Vec, } @@ -157,6 +160,7 @@ fn handle_conn( body.truncate(content_length); *last.lock().expect("lock") = Some(RecordedRequest { + path: path.clone(), headers, body: body.clone(), }); @@ -186,6 +190,8 @@ fn handle_conn( (200, "OK", vec![("x-echo", "yes")], body) } else if path == "/teapot" { (418, "I'm a teapot", Vec::new(), b"teapot".to_vec()) + } else if path == "/redirect" { + (302, "Found", vec![("location", "/echo")], Vec::new()) } else { (404, "Not Found", Vec::new(), Vec::new()) }; diff --git a/crates/engine/src/api/client.rs b/crates/engine/src/api/client.rs index 585c2052c..c6dd8a5e9 100644 --- a/crates/engine/src/api/client.rs +++ b/crates/engine/src/api/client.rs @@ -25,7 +25,10 @@ use super::types::{ TestLoginResponse, TokenResponse, UploadResult, }; use crate::content::DAG_ROOT_CODEC; -use crate::seams::{CredentialStore, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse}; +use crate::seams::{ + CredentialStore, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, SeamError, + bearer_header, +}; /// Control-plane deadline: small JSON round trips must not park a UI flow. const CONTROL_TIMEOUT_MS: u64 = 10_000; @@ -34,7 +37,6 @@ const CONTROL_TIMEOUT_MS: u64 = 10_000; const TRANSFER_TIMEOUT_MS: u64 = 120_000; const CONTENT_TYPE: &str = "Content-Type"; -const AUTHORIZATION: &str = "Authorization"; const APPLICATION_JSON: &str = "application/json"; const APPLICATION_OCTET_STREAM: &str = "application/octet-stream"; /// Carries an upload's declared content address. A header, not a query @@ -56,6 +58,30 @@ struct State { refresh_waiters: Option>>>, } +/// Holds single-flight leadership for one rotation and releases it on `Drop`, +/// so a leader cancelled while parked on the network cannot leave the slot +/// occupied by senders nothing will ever fire. +struct RefreshLead<'a> { + state: &'a RefCell, +} + +impl RefreshLead<'_> { + /// Takes the waiters to notify, leaving the slot for `Drop` to release. + fn waiters(&self) -> Vec>> { + self.state + .borrow_mut() + .refresh_waiters + .take() + .unwrap_or_default() + } +} + +impl Drop for RefreshLead<'_> { + fn drop(&mut self) { + self.state.borrow_mut().refresh_waiters = None; + } +} + /// The engine's single API client. Generic over the two seams it drives so the /// contract suite can point it at a real HTTP stack while unit tests drive it /// with the scripted fake. @@ -116,6 +142,9 @@ impl ApiClient { .await?; let response = ok_or_err(response)?; let body: ChallengeResponse = decode(&response)?; + if !is_identity_challenge(&body.challenge) { + return Err(ApiError::Decode("unusable login challenge".into())); + } body.challenge }; let signature = signer.sign_challenge(&challenge); @@ -236,18 +265,17 @@ impl ApiClient { if let Some(rx) = receiver { return match rx.await { Ok(result) => result, - Err(oneshot::Canceled) => Err(ApiError::Unauthorized), + // A cancelled leader is availability, not a dead session — the + // caller must not be told to re-login. + Err(oneshot::Canceled) => Err(ApiError::Transport(SeamError::new( + "refresh was cancelled before it completed", + ))), }; } + let lead = RefreshLead { state: &self.state }; let result = self.do_refresh().await; - let waiters = self - .state - .borrow_mut() - .refresh_waiters - .take() - .unwrap_or_default(); - for tx in waiters { + for tx in lead.waiters() { let _ = tx.send(result.clone()); } result @@ -516,11 +544,28 @@ impl ApiClient { headers.push(((*name).to_owned(), (*value).to_owned())); } // Scope the borrow so it never crosses the await below. - if let Some(token) = self.state.borrow().access_token.as_ref() { - headers.push(( - AUTHORIZATION.to_owned(), - format!("Bearer {}", token.as_str()), - )); + let bearer = { + let mut state = self.state.borrow_mut(); + let built = state + .access_token + .as_ref() + .map(|token| bearer_header(token.as_str())); + match built { + Some(Ok(header)) => Some(header), + // Drop a token that can never be a header value instead of + // refusing every later call while still holding it: the next + // call then goes out unauthenticated and its 401 drives one + // refresh. The refresh credential is a separate secret and + // stays, so a malformed response cannot end the session. + Some(Err(_)) => { + state.access_token = None; + return Err(ApiError::Decode("unusable access token".into())); + } + None => None, + } + }; + if let Some(header) = bearer { + headers.push(header); } let request = HttpRequest { method, @@ -605,6 +650,31 @@ fn is_success(status: u16) -> bool { (200..300).contains(&status) } +/// The domain tag the API stamps on an identity login challenge +/// (`apps/api/src/auth/services/challenge.service.ts`). Versioned, so a format +/// change bumps it rather than silently widening what this key will sign. +const IDENTITY_CHALLENGE_PREFIX: &str = "cipherbox-login:v2:"; +/// The challenge's random tail: 32 bytes rendered lowercase hex. +const IDENTITY_CHALLENGE_NONCE_LEN: usize = 64; + +/// Whether the server's answer is a challenge this key may sign: the login +/// domain tag followed by exactly the API's random tail. +/// +/// The signer hands `sha256(utf8(challenge))` to the secp256k1 identity key, +/// so an unchecked challenge makes that key a signing oracle for any UTF-8 +/// preimage. Pinning the whole shape — not just the tag — leaves a hostile +/// responder no steerable byte outside the hex alphabet the API renders. +fn is_identity_challenge(challenge: &str) -> bool { + challenge + .strip_prefix(IDENTITY_CHALLENGE_PREFIX) + .is_some_and(|nonce| { + nonce.len() == IDENTITY_CHALLENGE_NONCE_LEN + && nonce + .bytes() + .all(|byte| matches!(byte, b'0'..=b'9' | b'a'..=b'f')) + }) +} + /// EIP-4361 fixes the nonce at 8+ alphanumerics. The check is fail-closed /// rather than cosmetic: the nonce is interpolated verbatim into the text a /// wallet signs, so anything outside that class lets a hostile challenge @@ -651,6 +721,7 @@ mod tests { use super::*; use cipherbox_core::content::{CONTENT_CID_CODEC, compute_cid, encode_content_cid_str}; + use crate::seams::AUTHORIZATION; use crate::testkit::block_on; use crate::testkit::fakes::{InMemoryCredentialStore, ScriptedHttp}; use serde_json::{Value, json}; @@ -702,11 +773,17 @@ mod tests { (http, creds, client) } + /// A challenge shaped exactly as the API issues one: the login domain tag + /// plus 32 random bytes in lowercase hex. + fn challenge() -> String { + IDENTITY_CHALLENGE_PREFIX.to_owned() + &"0123456789abcdef".repeat(4) + } + /// Log in so the client holds an access token and a stored refresh token. fn login(http: &ScriptedHttp, client: &ApiClient) { http.enqueue_response(json_response( 200, - json!({ "challenge": "cipherbox-login:v2:abc", "expiresAt": "2026-01-01T00:00:00Z" }), + json!({ "challenge": challenge(), "expiresAt": "2026-01-01T00:00:00Z" }), )); http.enqueue_response(json_response( 200, @@ -737,19 +814,99 @@ mod tests { ); assert_eq!(requests[1].url, "http://api.test/auth/login"); let login_body = body_json(&requests[1]); - assert_eq!(login_body["challenge"], "cipherbox-login:v2:abc"); - assert_eq!(login_body["signature"], "sig-for-cipherbox-login:v2:abc"); + assert_eq!(login_body["challenge"], challenge()); + assert_eq!(login_body["signature"], format!("sig-for-{}", challenge())); let stored = block_on(creds.load_refresh_token()).unwrap().unwrap(); assert_eq!(stored, "a".repeat(64).as_bytes()); } + /// Every shape the API could not have issued. Each breaks a different part + /// of the pin, so none is subsumed by another. + fn hostile_challenges() -> Vec { + let hex64 = "0123456789abcdef".repeat(4); + vec![ + String::new(), + // The tag with no tail at all. + IDENTITY_CHALLENGE_PREFIX.to_owned(), + // No domain tag: an arbitrary preimage of the responder's choosing. + hex64.clone(), + // Another protocol's tag, and an older version of this one. + format!("cipherbox-grant:v2:{hex64}"), + format!("cipherbox-login:v1:{hex64}"), + // The tag as a suffix, not a prefix — guards a `contains` regression. + format!("{hex64}{IDENTITY_CHALLENGE_PREFIX}"), + // Right tag and alphabet, wrong width — short, then long. + format!("{IDENTITY_CHALLENGE_PREFIX}{}", &hex64[..63]), + format!("{IDENTITY_CHALLENGE_PREFIX}{hex64}0"), + // Right width, outside the hex alphabet: all-caps, one uppercase + // digit among 64, then a wholly attacker-chosen tail. + format!( + "{IDENTITY_CHALLENGE_PREFIX}{}", + "0123456789ABCDEF".repeat(4) + ), + format!("{IDENTITY_CHALLENGE_PREFIX}{}A", &hex64[..63]), + format!("{IDENTITY_CHALLENGE_PREFIX}{:_<64}", "sign anything"), + // 64 chars but 65 bytes, then 64 bytes with a multi-byte tail: the + // width check counts bytes, and the alphabet catches what it misses. + format!("{IDENTITY_CHALLENGE_PREFIX}{}\u{e9}", &hex64[..63]), + format!("{IDENTITY_CHALLENGE_PREFIX}{}\u{e9}", &hex64[..62]), + // An interior control character; the tail is echoed to /auth/login. + format!("{IDENTITY_CHALLENGE_PREFIX}{}\0", &hex64[..63]), + ] + } + + /// The accept side of the pin: every tail the API's hex renderer can emit + /// is admitted, so a tightening that would break a real login fails here + /// rather than in staging. + #[test] + fn the_shape_the_api_issues_is_accepted_at_the_class_boundaries() { + for tail in ["0".repeat(64), "f".repeat(64), "0123456789abcdef".repeat(4)] { + assert!(is_identity_challenge(&format!( + "{IDENTITY_CHALLENGE_PREFIX}{tail}" + ))); + } + } + + /// The guard is "never signs", not merely "never sends": a refused + /// challenge must not reach the identity key at all. + #[test] + fn a_challenge_the_api_could_not_have_issued_is_never_signed() { + struct PanickingSigner; + + impl ChallengeSigner for PanickingSigner { + fn public_key_hex(&self) -> String { + "02".to_owned() + &"ab".repeat(32) + } + fn sign_challenge(&self, challenge: &str) -> String { + panic!("the identity key signed a refused challenge: {challenge:?}"); + } + } + + for challenge in hostile_challenges() { + let (http, _creds, client) = fakes(); + http.enqueue_response(json_response( + 200, + json!({ "challenge": challenge, "expiresAt": "2026-01-01T00:00:00Z" }), + )); + assert_eq!( + block_on(client.login_identity(&PanickingSigner)).unwrap_err(), + ApiError::Decode("unusable login challenge".into()), + "challenge {challenge:?} must be refused" + ); + let requests = http.requests(); + assert_eq!(requests.len(), 1, "only /auth/challenge for {challenge:?}"); + assert_eq!(requests[0].url, "http://api.test/auth/challenge"); + assert!(!client.is_authenticated()); + } + } + #[test] fn identity_login_bad_signature_is_unauthorized() { let (http, _creds, client) = fakes(); http.enqueue_response(json_response( 200, - json!({ "challenge": "c", "expiresAt": "2026-01-01T00:00:00Z" }), + json!({ "challenge": challenge(), "expiresAt": "2026-01-01T00:00:00Z" }), )); http.enqueue_response(json_response( 401, @@ -1137,6 +1294,104 @@ mod tests { assert!(client.is_authenticated()); } + /// A leader dropped while parked on the network must hand leadership back, + /// or every later caller enqueues behind a rotation that will never finish. + #[test] + fn a_cancelled_refresh_leader_lets_the_next_caller_lead() { + let http = GatedHttp::default(); + let creds = InMemoryCredentialStore::default(); + block_on(creds.store_refresh_token(b"seed-refresh-token")).unwrap(); + let client = ApiClient::new(http.clone(), creds, "http://api.test"); + let mut cx = Context::from_waker(Waker::noop()); + + { + let mut leader = pin!(client.refresh()); + assert!(leader.as_mut().poll(&mut cx).is_pending()); + assert_eq!(http.request_count(), 1); + } // The leader's future is dropped mid-flight. + + // The next caller leads a fresh rotation rather than parking. + let mut next = pin!(client.refresh()); + assert!(next.as_mut().poll(&mut cx).is_pending()); + assert_eq!(http.request_count(), 2, "the slot was handed back"); + + http.release(json_response( + 200, + json!({ "accessToken": "jwt", "refreshToken": "f".repeat(64) }), + )); + assert!(matches!(next.as_mut().poll(&mut cx), Poll::Ready(Ok(())))); + assert!(client.is_authenticated()); + } + + /// A waiter behind a cancelled leader is woken, and told it was an + /// availability failure — not that its session is dead. + #[test] + fn a_waiter_behind_a_cancelled_leader_is_woken_not_parked() { + let http = GatedHttp::default(); + let creds = InMemoryCredentialStore::default(); + block_on(creds.store_refresh_token(b"seed-refresh-token")).unwrap(); + let client = ApiClient::new(http.clone(), creds, "http://api.test"); + let mut cx = Context::from_waker(Waker::noop()); + + let mut waiter = pin!(client.refresh()); + { + let mut leader = pin!(client.refresh()); + assert!(leader.as_mut().poll(&mut cx).is_pending()); + assert!(waiter.as_mut().poll(&mut cx).is_pending()); + assert_eq!(http.request_count(), 1, "the waiter coalesced"); + } + + match waiter.as_mut().poll(&mut cx) { + Poll::Ready(Err(ApiError::Transport(_))) => {} + other => panic!("a cancelled leader is availability, got {other:?}"), + } + } + + /// The access token is the API's bytes, so it meets the seam's header-value + /// rule like any other bearer: an unusable one fails closed rather than + /// reaching the transport. + #[test] + fn an_access_token_that_cannot_be_a_header_value_is_refused() { + let (http, _creds, client) = fakes(); + http.enqueue_response(json_response( + 200, + json!({ "challenge": challenge(), "expiresAt": "2026-01-01T00:00:00Z" }), + )); + http.enqueue_response(json_response( + 200, + json!({ + "accessToken": "jwt-1\r\nX-Injected: yes", + "refreshToken": "a".repeat(64), + }), + )); + block_on(client.login_identity(&StubSigner)).expect("login"); + + assert_eq!( + block_on(client.quota()).unwrap_err(), + ApiError::Decode("unusable access token".into()) + ); + assert_eq!(http.requests().len(), 2, "no request carried the token"); + assert!(!client.is_authenticated(), "the unusable token was dropped"); + + // Self-heal: the next call goes out unauthenticated, and its 401 buys + // one rotation off the still-held refresh credential. + http.enqueue_response(json_response(401, json!({ "message": "no bearer" }))); + http.enqueue_response(json_response( + 200, + json!({ "accessToken": "jwt-2", "refreshToken": "b".repeat(64) }), + )); + http.enqueue_response(json_response( + 200, + json!({ "usedBytes": 1, "limitBytes": 2, "advisory": false }), + )); + + block_on(client.quota()).expect("the session recovered"); + let requests = http.requests(); + assert!(!has_bearer(&requests[2]), "the dropped token was not sent"); + assert_eq!(requests[3].url, "http://api.test/auth/refresh"); + assert!(has_bearer(&requests[4]), "the retry carried the new token"); + } + #[test] fn upload_declares_the_block_address_on_the_wire() { let (http, _creds, client) = fakes(); diff --git a/crates/engine/src/content/provider.rs b/crates/engine/src/content/provider.rs index df9c98027..5466b793d 100644 --- a/crates/engine/src/content/provider.rs +++ b/crates/engine/src/content/provider.rs @@ -17,7 +17,8 @@ use zeroize::Zeroizing; use crate::content::DAG_ROOT_CODEC; use crate::seams::{ - CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, + AUTHORIZATION, CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, + check_bearer, }; /// Deadline for a BYO-provider reachability probe: an unresponsive endpoint @@ -35,7 +36,6 @@ const PLACEMENT_TIMEOUT_MS: u64 = 60_000; /// stream is the largest, one short object per block put. const MAX_PROVIDER_RESPONSE_BYTES: usize = 64 * 1024; -const AUTHORIZATION: &str = "Authorization"; const CONTENT_TYPE: &str = "Content-Type"; const APPLICATION_JSON: &str = "application/json"; @@ -343,14 +343,10 @@ pub async fn test_connection( pub fn validate_byo_config(config: &ByoIpfsConfig) -> Result<(), ProviderError> { validate_endpoint(&config.endpoint)?; match &config.access_token { - // The token is spliced into a header value verbatim, so a control - // character in it would inject a header. A present-but-empty one is an - // `Authorization: Bearer ` no provider accepts; `None` is how a - // credential-less provider is spelled. - Some(token) if token.is_empty() || !token.bytes().all(is_bearer_byte) => { - Err(ProviderError::InvalidCredential) - } - _ => Ok(()), + // `None` is how a credential-less provider is spelled, so it is not a + // verdict; a token that is present must be sendable as a header value. + Some(token) => check_bearer(token.as_str()).map_err(|_| ProviderError::InvalidCredential), + None => Ok(()), } } @@ -464,12 +460,6 @@ fn is_path_byte(b: u8) -> bool { is_authority_byte(b) || matches!(b, b'/' | b'_' | b'~' | b'%' | b'+' | b'=' | b'&' | b',') } -/// The bytes a bearer credential admits: visible ASCII, the header-value set -/// minus the whitespace no token carries. -fn is_bearer_byte(b: u8) -> bool { - matches!(b, 0x21..=0x7e) -} - /// The per-kind reachability probe. The endpoints are each provider's standard /// identity/auth check: Kubo `POST /api/v0/id`, PSA `GET /pins?limit=1`, Pinata /// `GET /data/testAuthentication`. @@ -681,6 +671,23 @@ mod tests { ); } + /// The config gate and the header the splice builds are the same rule, so + /// no token can pass one and be refused by the other. + #[test] + fn the_config_gate_and_the_header_splice_agree_on_every_token() { + for token in ["", "tok\r\n", "tok tok", "tok\u{80}", "ok-token", "!", "~"] { + assert_eq!( + validate_byo_config(&config(ByoKind::Psa, Some(token))).is_ok(), + crate::seams::bearer_header(token).is_ok(), + "{token:?}" + ); + } + assert!( + validate_byo_config(&config(ByoKind::Psa, None)).is_ok(), + "a credential-less provider is not a verdict" + ); + } + fn leaf(bytes: &[u8]) -> Vec { compute_cid(CONTENT_CID_CODEC, bytes) } diff --git a/crates/engine/src/content/read.rs b/crates/engine/src/content/read.rs index 1c72eb4f0..65dd3a8ae 100644 --- a/crates/engine/src/content/read.rs +++ b/crates/engine/src/content/read.rs @@ -20,9 +20,10 @@ use zeroize::Zeroizing; use super::dag::DAG_ROOT_CODEC; use super::limits::MAX_RESOLVED_RECORD_BYTES; -use crate::seams::{CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest}; +use crate::seams::{ + CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, SeamError, bearer_header, +}; -const AUTHORIZATION: &str = "Authorization"; const ACCEPT: &str = "Accept"; /// Deadline for one leaf-block GET: a seek issues one per leaf against sources /// of unknown quality, so a stalled gateway must fail over. @@ -270,10 +271,11 @@ async fn fetch( let base = source.base_url.trim_end_matches('/'); let mut headers = vec![(ACCEPT.to_owned(), RAW_BLOCK.to_owned())]; if let Some(bearer) = &source.bearer { - headers.push(( - AUTHORIZATION.to_owned(), - format!("Bearer {}", bearer.as_str()), - )); + // A source whose token cannot be a header value is skipped, never + // contacted unauthenticated: rotation drops to the next source. + headers.push(bearer_header(bearer.as_str()).map_err(|_| { + CappedFetchError::Transport(SeamError::new("gateway source bearer is unusable")) + })?); } let request = HttpRequest { method: HttpMethod::Get, @@ -322,7 +324,7 @@ mod tests { use super::*; use crate::content::chunk::{ContentKey, frame_and_seal}; use crate::content::profile::ContentProfile; - use crate::seams::HttpResponse; + use crate::seams::{AUTHORIZATION, HttpResponse}; use crate::testkit::SeededEntropy; use crate::testkit::block_on; use crate::testkit::fakes::ScriptedHttp; @@ -550,6 +552,40 @@ mod tests { ); } + /// A source whose bearer cannot be a header value is skipped, never + /// contacted without it — and rotation still reaches a healthy source. + #[test] + fn a_source_with_an_unusable_bearer_is_skipped_not_contacted_bare() { + let leaf = one_leaf(); + let http = ScriptedHttp::default(); + http.enqueue_response(raw_response(leaf.sealed.clone())); + + let gateway = Gateway { + accelerator: Some(GatewaySource { + base_url: "https://gw.cipherbox.test".into(), + bearer: Some(Zeroizing::new("member\r\nX-Injected: 1".to_owned())), + }), + public_fallbacks: vec![GatewaySource { + base_url: "https://public.gw.test".into(), + bearer: None, + }], + }; + + let out = block_on(read_block( + &gateway, + &http, + &cid_str(), + &leaf.cid, + ContentPlane::Leaf, + )) + .unwrap(); + assert_eq!(out, leaf.sealed); + + let requests = http.requests(); + assert_eq!(requests.len(), 1, "the accelerator was never contacted"); + assert!(requests[0].url.starts_with("https://public.gw.test/ipfs/")); + } + #[test] fn all_sources_unavailable_is_unavailable_not_a_violation() { let leaf = one_leaf(); diff --git a/crates/engine/src/facade.rs b/crates/engine/src/facade.rs index c68e1f95c..9b69fe085 100644 --- a/crates/engine/src/facade.rs +++ b/crates/engine/src/facade.rs @@ -3367,11 +3367,15 @@ mod tests { #[test] fn start_performs_identity_login_and_persists_a_refresh_token() { + /// Shaped as the API issues one; the engine signs nothing else. + const LOGIN_CHALLENGE: &str = + "cipherbox-login:v2:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + let (mut engine, _events, device) = engine_over(ApiBaseUrl::parse("http://api.test").expect("a configured base")); device.http.enqueue_response(json_response( 200, - json!({ "challenge": "cipherbox-login:v2:abc", "expiresAt": "2099-01-01T00:00:00Z" }), + json!({ "challenge": LOGIN_CHALLENGE, "expiresAt": "2099-01-01T00:00:00Z" }), )); device.http.enqueue_response(json_response( 200, @@ -3394,7 +3398,7 @@ mod tests { let identity = engine.session().unwrap().identity(); let expected = hex_lower( &identity - .sign_detcbor(b"cipherbox-login:v2:abc") + .sign_detcbor(LOGIN_CHALLENGE.as_bytes()) .to_compact(), ); assert_eq!(login_body["signature"], expected); diff --git a/crates/engine/src/seams/http.rs b/crates/engine/src/seams/http.rs index 582aa0ecc..34c7c30a1 100644 --- a/crates/engine/src/seams/http.rs +++ b/crates/engine/src/seams/http.rs @@ -24,6 +24,35 @@ pub enum CappedFetchError { }, } +/// The `Authorization` header name — one spelling for every splice site. +pub const AUTHORIZATION: &str = "Authorization"; + +/// A bearer credential refused before it became a header value. Carries +/// nothing: the token itself must never reach an error string or a log. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct InvalidBearer; + +/// The rule a bearer must meet to be sent: non-empty, and visible ASCII +/// throughout (`0x21..=0x7e`). A control character or space in a header value +/// splits or injects a header at the host transport. +/// +/// Separate from [`bearer_header`] so a caller that only asks the question — +/// a config gate — never materializes a second, non-zeroized copy of the +/// credential just to throw it away. +pub fn check_bearer(token: &str) -> Result<(), InvalidBearer> { + if token.is_empty() || !token.bytes().all(|byte| matches!(byte, 0x21..=0x7e)) { + return Err(InvalidBearer); + } + Ok(()) +} + +/// Builds the `Authorization: Bearer …` pair for a token meeting +/// [`check_bearer`]. +pub fn bearer_header(token: &str) -> Result<(String, String), InvalidBearer> { + check_bearer(token)?; + Ok((AUTHORIZATION.to_owned(), format!("Bearer {token}"))) +} + /// Formats headers as their names only. Header values ride this seam /// carrying live credentials (`Authorization` bearer JWTs, refresh /// cookies) and must never reach logs. @@ -154,10 +183,15 @@ impl fmt::Debug for HttpResponse { /// are responses, not errors — a seam `Err` is reserved for transport-level /// failure (unreachable, aborted, deadline elapsed). /// -/// One obligation the transport owns: a request the engine sent over `https` -/// must not be replayed over `http` by following a redirect, or an -/// `Authorization` header would reach the clear network past the engine's -/// transport decision (blueprint/engine.md "Content plane"). +/// One obligation the transport owns: it must not follow a redirect. Every +/// target on this seam — the API, a gateway, a BYO provider — is directly +/// addressed and gated on the URL the engine supplied, so a hop the engine did +/// not choose can only escape that gate: it replays the request past +/// [`crate::content::validate_byo_config`]'s endpoint rules, and a downgrade to +/// `http` would carry an `Authorization` header onto the clear network +/// (blueprint/engine.md "Content plane"). How a refusal surfaces is the +/// transport's own: desktop returns the 3xx as a response, web rejects it as a +/// transport failure — both fail closed, so no caller may branch on which. /// /// No conformance kit ships for this seam: it has no seam-local durable /// semantics; its behavior is exercised end-to-end by the live contract @@ -220,6 +254,41 @@ mod tests { assert!(debug.contains("<19 bytes>"), "body renders as a length"); } + #[test] + fn a_usable_bearer_becomes_the_authorization_pair() { + assert_eq!( + bearer_header("eyJhbGciOi.J9-_~+/=").unwrap(), + ( + AUTHORIZATION.to_owned(), + "Bearer eyJhbGciOi.J9-_~+/=".to_owned() + ) + ); + assert!(bearer_header("!").is_ok(), "0x21, the low edge"); + assert!(bearer_header("~").is_ok(), "0x7e, the high edge"); + } + + #[test] + fn a_bearer_that_could_reshape_the_request_is_refused() { + for token in [ + "", // an `Authorization: Bearer ` no server accepts + "jwt\r\nX-Injected: 1", // header injection + "jwt token", // a space splits the credential + "jwt\u{7f}", // DEL, just above the class + "jwt\u{80}", // non-ASCII, and every multi-byte tail with it + ] { + assert_eq!(bearer_header(token), Err(InvalidBearer), "token {token:?}"); + assert_eq!(check_bearer(token), Err(InvalidBearer), "token {token:?}"); + } + } + + /// A tripwire on the refusal type: it must stay field-less, so no future + /// diagnostic can carry the credential into an error string or a log. + #[test] + fn the_bearer_refusal_carries_no_credential() { + let refusal = bearer_header("super-secret-jwt token").unwrap_err(); + assert!(!format!("{refusal:?}").contains("super-secret-jwt")); + } + #[test] fn response_debug_redacts_header_values_and_body() { let response = HttpResponse { diff --git a/crates/engine/src/seams/mod.rs b/crates/engine/src/seams/mod.rs index a77712959..a47503d0b 100644 --- a/crates/engine/src/seams/mod.rs +++ b/crates/engine/src/seams/mod.rs @@ -22,7 +22,13 @@ mod staging_store; pub use credential_store::CredentialStore; pub use floor_store::{FloorNamespace, FloorRaise, FloorStore}; -pub use http::{CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse}; +pub use http::{ + CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, InvalidBearer, + bearer_header, check_bearer, +}; +// The header name is an engine-internal spelling: a host implements the +// transport, it never builds a bearer. +pub(crate) use http::AUTHORIZATION; pub use mailbox::{Mailbox, MailboxItem}; pub use record_transport::{EndpointId, RecordTransport}; pub use refresh_hint::{RefreshHint, RefreshHintSource}; diff --git a/crates/load/src/runner.rs b/crates/load/src/runner.rs index d2aea3cfa..b913c4037 100644 --- a/crates/load/src/runner.rs +++ b/crates/load/src/runner.rs @@ -10,7 +10,9 @@ use std::time::{Duration, Instant}; use cipherbox_core::content::{CONTENT_CID_CODEC, compute_cid, encode_content_cid_str}; use cipherbox_desktop_seams::ReqwestHttp; use cipherbox_engine::api::{ApiClient, ApiError}; -use cipherbox_engine::seams::{Http, HttpCredentials, HttpMethod, HttpRequest}; +use cipherbox_engine::seams::{ + Http, HttpCredentials, HttpMethod, HttpRequest, SeamError, bearer_header, +}; use crate::metrics::{Collector, Outcome, Sample}; use crate::plan::{MAX_BLOCK_BYTES, RunPlan}; @@ -87,9 +89,13 @@ pub(crate) async fn gateway_get( url: &str, token: Option<&str>, ) -> Result { - let headers = token - .map(|token| vec![("Authorization".to_owned(), format!("Bearer {token}"))]) - .unwrap_or_default(); + let headers = match token { + Some(token) => vec![ + bearer_header(token) + .map_err(|_| ApiError::Transport(SeamError::new("gateway bearer is unusable")))?, + ], + None => Vec::new(), + }; let request = HttpRequest { method: HttpMethod::Get, url: url.to_owned(), diff --git a/crates/load/src/seams.rs b/crates/load/src/seams.rs index ebc3e81c4..b91ffd7fe 100644 --- a/crates/load/src/seams.rs +++ b/crates/load/src/seams.rs @@ -18,6 +18,10 @@ pub(crate) fn build_http() -> Result { let client = reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) .timeout(Duration::from_secs(60)) + // The seam forbids following a redirect, and a `with_client` caller + // owns that policy — a measured run must move bytes the way the + // shipping transport does. + .redirect(reqwest::redirect::Policy::none()) .build() .map_err(|error| format!("build reqwest client: {error}"))?; Ok(ReqwestHttp::with_client(client)) diff --git a/packages/client/src/seams/http.test.ts b/packages/client/src/seams/http.test.ts index 158401cdc..3a58ee710 100644 --- a/packages/client/src/seams/http.test.ts +++ b/packages/client/src/seams/http.test.ts @@ -95,6 +95,42 @@ describe('FetchHttp credential scoping', () => { }); }); +/** + * A server answering 3xx, modelled as the Fetch spec resolves one: under + * `redirect: 'error'` the redirect is a network error, otherwise it is handed + * back as a response. + */ +function redirectingFetch(): void { + vi.stubGlobal( + 'fetch', + vi.fn((_url: string, init: RequestInit) => + init.redirect === 'error' + ? Promise.reject(new TypeError('Failed to fetch')) + : Promise.resolve(new Response(null, { status: 302, headers: { location: '/elsewhere' } })) + ) + ); +} + +describe('FetchHttp redirects', () => { + it('refuses them on both paths, as the record transport does', async () => { + const fetches = recordingFetch(); + const http = new FetchHttp(); + + await http.send(GET); + await http.sendCapped(GET, 1000); + + expect(fetches.inits.map((init) => init.redirect)).toEqual(['error', 'error']); + }); + + it('rejects a redirecting server rather than following or returning it', async () => { + redirectingFetch(); + const http = new FetchHttp(); + + await expect(http.send(GET)).rejects.toThrow(TypeError); + await expect(http.sendCapped(GET, 1000)).rejects.toThrow(TypeError); + }); +}); + describe('FetchHttp deadlines', () => { it('carries no abort signal when the request sets no deadline', async () => { const fetches = recordingFetch(); diff --git a/packages/client/src/seams/http.ts b/packages/client/src/seams/http.ts index 7564a2d42..64545af5e 100644 --- a/packages/client/src/seams/http.ts +++ b/packages/client/src/seams/http.ts @@ -25,6 +25,8 @@ function requestInit(request: HttpRequestData): RequestInit { headers, // Fail-safe default: authority is opt-in per request, never inferred. credentials: request.credentials ?? 'omit', + // No redirects; see the `Http` seam contract. Rejects rather than resolving. + redirect: 'error', }; const timeoutMs = request.timeoutMs; if (timeoutMs !== undefined && timeoutMs !== null) { diff --git a/packages/client/test/browser/mockAuth.ts b/packages/client/test/browser/mockAuth.ts index 33ccea991..56610bbdf 100644 --- a/packages/client/test/browser/mockAuth.ts +++ b/packages/client/test/browser/mockAuth.ts @@ -42,7 +42,9 @@ function challenge(res: ServerResponse, dto: Fields): void { send(res, 400, { error: 'publicKey must be a string' }); return; } - const value = `cipherbox-login:v2:${publicKey.slice(0, 16)}`; + // Shaped as the API issues one — the domain tag plus 32 bytes of lowercase + // hex — because the engine refuses to sign anything else. + const value = `cipherbox-login:v2:${publicKey.slice(0, 64).padEnd(64, '0')}`; issued.set(publicKey, value); completed.challenges += 1; send(res, 200, { challenge: value, expiresAt: '2099-01-01T00:00:00Z' });