diff --git a/.gitattributes b/.gitattributes index f1e96bbc..9446e049 100644 --- a/.gitattributes +++ b/.gitattributes @@ -21,3 +21,6 @@ # doesn't see noisy diffs against Windows contributors. Cargo.lock text eol=lf bun.lock text eol=lf + +# Vendored, pre-compressed assets (assets/scalar/): bytes, never text. +*.gz binary diff --git a/Cargo.lock b/Cargo.lock index 2e784871..5091a985 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3578,18 +3578,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "utoipa-scalar" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8eb5bc76936ceb2bb790a62bec7fb99e22b8ecffca739e3d5a5afec0081ea7f3" -dependencies = [ - "axum", - "serde", - "serde_json", - "utoipa", -] - [[package]] name = "uuid" version = "1.28.0" @@ -3763,6 +3751,7 @@ dependencies = [ "dashmap", "dotenvy", "encoding_rs", + "flate2", "futures-util", "hex", "http-body-util", @@ -3796,7 +3785,6 @@ dependencies = [ "tracing-subscriber", "url", "utoipa", - "utoipa-scalar", "uuid", "walkdir", "waveflow-core", diff --git a/Cargo.toml b/Cargo.toml index 37ba73df..997e86a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,10 @@ clap = { version = "4", features = ["derive"] } dashmap = "6" dotenvy = "0.15" encoding_rs = "0.8.42" +# The API reference bundle is stored gzip-compressed (`src/reference.rs`) and +# decompressed only for a client that takes no gzip. Already in the tree +# through the response compression layer. +flate2 = "1" futures-util = "0.3" hex = "0.4" ipnet = "2" @@ -92,7 +96,6 @@ tower-http = { version = "0.7", features = [ tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter", "json"] } utoipa = { version = "6", features = ["axum_extras", "uuid"] } -utoipa-scalar = { version = "0.4", features = ["axum"] } url = "2" uuid = { version = "1", features = ["v4", "v8", "serde"] } walkdir = "2" diff --git a/assets/scalar/LICENSE b/assets/scalar/LICENSE new file mode 100644 index 00000000..05ee3dd2 --- /dev/null +++ b/assets/scalar/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2023-present Scalar + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/assets/scalar/api-reference-1.73.1.js.gz b/assets/scalar/api-reference-1.73.1.js.gz new file mode 100644 index 00000000..0f858b1f Binary files /dev/null and b/assets/scalar/api-reference-1.73.1.js.gz differ diff --git a/docs/privacy.md b/docs/privacy.md index 139a4fb6..cbdd2d5c 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -27,13 +27,20 @@ All outbound clients are built by one function, `src/outbound.rs`: - TLS is rustls; a connection attempt may take a third of the request's timeout. - No URL and no response body is written to the log. +- **Every answer is read under a ceiling** (RFC-023 decision 1): 2 MiB for an + API answer, 10 MiB for an image, the podcast ceilings for a feed or an + episode. An answer that announces more is refused + unread; one that keeps coming is cut off at the line. Either is logged as a + verdict, never with its URL or its body. The server never turns a request it receives into a request of its own, with one exception an operator switches on: an administrator subscribing to a podcast, or asking for an episode, below. Radio station URLs are stored and handed to clients, never fetched; remote entries in an imported playlist are dropped; FFmpeg and ffprobe are only ever given local -files inside a library or the data directory. +files inside a library or the data directory, and are told so: every input is +passed with `-protocol_whitelist file`, so neither can open a network or +`concat:` resource whatever a file names inside it (RFC-023 decision 2). ## Metadata enrichment: Last.fm, Deezer, TheAudioDB @@ -130,7 +137,8 @@ artist is not sent. application, with a callback to `WAVEFLOW_PUBLIC_URL`; the server then calls `auth.getSession` once to obtain the session key. The command-line journey (`scrobble authorize`, `scrobble exchange`) calls `auth.getToken` and then -`auth.getSession`. +`auth.getSession`. Each of these calls is bounded by +`WAVEFLOW_SCROBBLE_REQUEST_TIMEOUT_SECS` like a listen. ## Podcasts: the feeds an administrator subscribes to @@ -181,12 +189,15 @@ the log. The embedded web client loads nothing from a third party: its fonts are bundled, every API call is relative to the server, and artwork is served by the server rather than linked from a provider. Its Content-Security-Policy holds it -to that (RFC-019 decision 2). Three exceptions, each deliberate: +to that (RFC-019 decision 2). Two exceptions, each deliberate: - **Internet radio.** The browser plays a station straight from the station's own server, at the URL a user entered. That host sees the listener's address. Subsonic clients receive the same URLs and connect themselves. - **Linking Last.fm**, above: a navigation the user starts. -- **`/reference`**, the API reference page. It loads the Scalar viewer from - `cdn.jsdelivr.net` into the visitor's browser. It is outside the web - client's policy, and nobody visits it unless they open it. + +**`/reference`**, the API reference page, was a third until RFC-023: it loaded +the Scalar viewer, unversioned, from `cdn.jsdelivr.net`. The viewer is now part +of the binary (`@scalar/api-reference` 1.73.1) and served by the server itself, +under a policy that admits scripts and connections from this origin only; its +fonts, telemetry, AI agent and publishing toolbar are switched off. diff --git a/docs/rfcs/RFC-023-hardening.md b/docs/rfcs/RFC-023-hardening.md new file mode 100644 index 00000000..cb77cea3 --- /dev/null +++ b/docs/rfcs/RFC-023-hardening.md @@ -0,0 +1,226 @@ +# RFC-023 — Hardening: bounded reads, FFmpeg's protocols, the API reference, and disabled accounts + +- **Status**: Proposed +- **Implemented by**: [#332](https://github.com/InstaZDLL/waveflow-server/pull/332) + (this RFC and its implementation, in one pull request). The *Status* field + above never flips in this project; this line is the one that says what is + running. +- **Date**: 2026-10-25 +- **Authors**: the WaveFlow project +- **Depends on**: [RFC-002](RFC-002-waveflow-server-v2.md) (the credential + model, the frozen Subsonic contract), + [RFC-019](RFC-019-exposure-hardening.md) (whose *What is still open* lists + the first three items below) + +## Problem + +RFC-019 closed what a default installation exposed and left three findings +open. A pass over the server's other external inputs, made for this RFC, found +more of the same kind. Each was confirmed in the code: + +1. **Answers from other hosts were read whole.** The Last.fm and Maloja + scrobbling adapters (`src/scrobblers/`) called `.json()` on whatever came + back. The metadata clients of `waveflow-core` already capped theirs at + 2 MiB; these did not, and the two calls of the Last.fm authorisation + journey were not bounded in time either. +2. **FFmpeg was given no `-protocol_whitelist`.** Every input the server hands + FFmpeg or ffprobe is a local path, but what a file may make either tool open + from inside it rested on the installed build's defaults, which nothing here + pinned or tested. +3. **`/reference` loaded Scalar from `cdn.jsdelivr.net`, unversioned.** Every + visitor's browser ran whatever that CDN called the latest release, on this + server's origin. +4. **A disabled account's shares still opened**, by their page, their JSON and + their stream. So did its stream and canvas tickets, for up to an hour: a + share is opened by nobody and a ticket is sealed rather than looked up, so + the account was never asked. +5. **Four client inputs were bounded by the request body alone**, which is not + a bound on what they cost: + - an **LRC sheet** may put any number of timestamps before a line, and each + becomes a copy of the line once read: a 1 MiB upload of 100,000 `[0:0]` + before half a mebibyte of text expanded to some fifty gigabytes, on upload + and on every read of the stored sheet; + - a **playlist import** built every entry of the file before refusing more + than 10,000 of them: a 5 MiB body of one-letter lines is millions; + - the **sync socket** accepted tungstenite's default 64 MiB message, which it + buffers whole before dropping it unread; + - a **search** became one FTS5 phrase per word, three times over, for as many + words as a 700 KB Subsonic form carries. + +## Decision 1 — every answer from another host is read under a ceiling + +- **One reader**, `outbound::read_bounded_body(response, limit)`, beside the + client every outbound request is already built with. It refuses an answer + whose `Content-Length` is over the limit before reading a byte, and cuts off + one that announces nothing, or lies, as soon as it crosses the line. It knows + nothing about who asked, so any later fetcher adopts it as it is. The + podcast feed fetcher of RFC-025 (`podcast::fetch_bounded`), merged before + this, had written the same two checks itself and now calls it, under its own + feed ceiling. Episode downloads stream to disk rather than into memory and + keep their own bounded writer. +- **2 MiB for an API answer** (`outbound::MAX_ANSWER_BYTES`), the figure + `waveflow-core` holds the metadata providers to. The largest answer this + server really reads is a few hundred bytes. The portrait fetcher keeps its own + 10 MiB, now through the same reader. +- **What an oversized answer means depends on what the body was for.** + - Where the body only *refines* a verdict the status line already gave — a + Last.fm `track.scrobble`, a Maloja submission — it refines nothing, and the + status line decides, exactly as for a body that will not parse. + `Ambiguous` was not chosen: RFC-010 turns it into a decision a person has + to make, one per listen, over an answer no real destination sends. + - Where the body *is* the answer — the Last.fm session key, the request + token — the call fails as `Unavailable`. +- **Logged by verdict, never by URL or body.** `BodyError` carries one of two + words and nothing else: a `reqwest::Error` prints its URL, and a Last.fm URL + carries the operator's key. +- **The Last.fm authorisation calls are bounded in time** by + `WAVEFLOW_SCROBBLE_REQUEST_TIMEOUT_SECS`, the deadline every listen already + has. Nothing above them had one. +- The metadata clients are unchanged: `waveflow-core` already bounds them, and + its reader is private to that crate. + +## Decision 2 — FFmpeg and ffprobe open local files and nothing else + +- **Every input is named through `media::file_input`**, which passes + `-protocol_whitelist file -i `: the transcode, the quality probe, the + measurement and the spectrogram, the canvas probe and the canvas remux. No + call site builds `-i` itself, so none can forget the list. +- **`file` is all any of them needs.** Every input is a path the server chose. + Outputs (`pipe:1`, a staging file) are not inputs and are not affected. +- **What it changes, measured on FFmpeg 8.0.** A top-level `http://` or + `concat:` input was opened as asked, and a file opened through `file` could + still name `crypto` and `data` resources from inside it. With the list, + FFmpeg refuses `http` and `concat` at the door. An HLS playlist naming an + `http` segment was already refused by that build's own default for nested + opens (`file,crypto,data`); the list makes that hold on a build whose default + is wider. +- **What it does not change.** A playlist-shaped file may still name another + *local* file. FFmpeg 8 only detects HLS under an `.m3u8` or `.m3u` name, which + no library file the scanner indexes carries, and the concat demuxer's safe + mode refuses absolute paths; a demuxer whitelist would close it fully and is + listed below as open. + +## Decision 3 — `/reference` is served from the binary + +- **The bundle is embedded.** `dist/browser/standalone.js` of + `@scalar/api-reference` 1.73.1 — the file jsDelivr resolved the package to — + stored as `gzip -9n` in `assets/scalar/` (1.3 MB, 4.4 MB decompressed) with + Scalar's MIT licence beside it. A test holds the embedded bytes to the + SHA-256 of the published file, and `src/reference.rs` records the npm + integrity of the tarball and how to move to another release. +- **Served as stored**, at a versioned path (`/reference/scalar-1.73.1.js`) + with `Cache-Control: immutable`, outside the compression layer: gzip to every + client that takes it, decompressed for one that does not. +- **No request costs more than a reference count.** The route is open to + whoever `/reference` is open to, and a first draft copied the 1.3 MB on + every request and decompressed 4.4 MB on every request that took no gzip — + CPU and memory for the asking, found by an automated review before merge. The + gzip body is now the embedded bytes themselves (`Bytes::from_static`); the + decompressed one is made once per process, on the first request that needs + it, kept in a `OnceLock` and shared from then on. A test serves it five + times without `Accept-Encoding` and counts one decompression. + - *Kept, not dropped.* Always sending gzip whatever the request said would + have removed the path entirely, and every browser that can run the page + accepts gzip. It was not chosen because a body coded in a way the request + did not admit is a wrong answer under HTTP, and the client most likely to + ask without gzip — a script or a mirror fetching the file — would store + compressed bytes under a `.js` name. The cost of answering it right is + 4.4 MB resident once such a client has asked, and nothing per request. + - The page itself (some 200 KB with the document) is built once and shared + the same way. +- **The page is built once** from the OpenAPI document. The document travels + in a `\n\ + \n\ + \n\ + \n" + ) +} + +fn serve_page(page: Bytes) -> Response { + let mut response = ([(header::CONTENT_TYPE, "text/html; charset=utf-8")], page).into_response(); + let headers = response.headers_mut(); + headers.insert(header::CACHE_CONTROL, HeaderValue::from_static("no-store")); + crate::security_headers::apply_reference(headers, &crate::security_headers::reference_policy()); + response +} + +/// The bundle decompressed, made at most once per process. +/// +/// **Neither variant costs a request more than a reference count.** The gzip +/// one is the embedded bytes themselves (`Bytes::from_static`), and this one is +/// made the first time a client that takes no gzip asks, then kept: the route +/// is open to anyone `/reference` is open to, and decompressing four megabytes +/// on every such request was a way to make the server spend CPU and memory for +/// the asking. Kept rather than dropped: the 4.4 MB stay resident once a single +/// client without gzip has asked, which is the price of answering it at all. +/// +/// A bundle that will not decompress is remembered as such too, so a defect in +/// the embedded file costs one attempt, not one per request. A test holds the +/// file to its published digest, so that branch is a build gone wrong. +static DECOMPRESSED: OnceLock> = OnceLock::new(); + +/// How many times the bundle was decompressed for serving. A test reads it. +#[cfg(test)] +static DECOMPRESSIONS: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +fn decompress_for_serving() -> Option { + #[cfg(test)] + DECOMPRESSIONS.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + match decompressed() { + Ok(bytes) => Some(Bytes::from(bytes)), + Err(error) => { + tracing::error!(%error, "the embedded API reference bundle does not decompress"); + None + } + } +} + +async fn serve_script(request_headers: HeaderMap) -> Response { + let gzip = accepts_gzip(&request_headers); + let body = if gzip { + Bytes::from_static(SCALAR_GZIP) + } else { + match DECOMPRESSED.get_or_init(decompress_for_serving) { + Some(bytes) => bytes.clone(), + None => return StatusCode::INTERNAL_SERVER_ERROR.into_response(), + } + }; + let mut response = body.into_response(); + let headers = response.headers_mut(); + headers.insert( + header::CONTENT_TYPE, + HeaderValue::from_static("text/javascript; charset=utf-8"), + ); + headers.insert( + header::CACHE_CONTROL, + HeaderValue::from_static("public, max-age=31536000, immutable"), + ); + headers.insert(header::VARY, HeaderValue::from_static("accept-encoding")); + if gzip { + headers.insert(header::CONTENT_ENCODING, HeaderValue::from_static("gzip")); + } + crate::security_headers::apply_reference( + headers, + &HeaderValue::from_static("default-src 'none'; frame-ancestors 'none'"), + ); + response +} + +/// Whether the request accepts gzip, read as RFC 9110 §12.5.3 and §12.4.2 +/// write `Accept-Encoding`. +/// +/// - **An entry naming gzip decides**, whatever its place in the list; `*` +/// decides only when none does, because it stands for the codings the field +/// does not name. So `gzip;q=0, *` refuses gzip, and `*` alone accepts it. +/// `x-gzip` is gzip. +/// - **Names are case-insensitive**, the coding's and the weight's alike: +/// `GZIP;Q=0` refuses. Whitespace around `;` and `=` is tolerated. +/// - **A weight of zero refuses**; one above zero, or none, accepts. A weight +/// outside the grammar — above 1, negative, more than three decimals, not a +/// number — makes the entry say nothing, so it neither accepts nor refuses. +/// The first entry of a kind that does say something is the one kept. +/// - **No header, or an empty one, is identity only**: no gzip. +fn accepts_gzip(headers: &HeaderMap) -> bool { + let mut named = None; + let mut wildcard = None; + for coding in headers + .get_all(header::ACCEPT_ENCODING) + .iter() + .filter_map(|value| value.to_str().ok()) + .flat_map(|value| value.split(',')) + { + let mut parts = coding.split(';').map(str::trim); + let name = parts.next().unwrap_or_default(); + let mut weight = Some(1000); + for parameter in parts { + let Some((key, value)) = parameter.split_once('=') else { + continue; + }; + if key.trim().eq_ignore_ascii_case("q") { + weight = qvalue(value.trim()); + } + } + let Some(weight) = weight else { + continue; + }; + if name.eq_ignore_ascii_case("gzip") || name.eq_ignore_ascii_case("x-gzip") { + named.get_or_insert(weight > 0); + } else if name == "*" { + wildcard.get_or_insert(weight > 0); + } + } + named.or(wildcard).unwrap_or(false) +} + +/// A `qvalue` in thousandths, or `None` outside RFC 9110's grammar: +/// `( "0" [ "." 0*3DIGIT ] ) / ( "1" [ "." 0*3("0") ] )`. +fn qvalue(text: &str) -> Option { + let (whole, fraction) = text.split_once('.').unwrap_or((text, "")); + if fraction.len() > 3 || !fraction.bytes().all(|byte| byte.is_ascii_digit()) { + return None; + } + let thousandths: u16 = format!("{fraction:0<3}").parse().ok()?; + match whole { + "0" => Some(thousandths), + "1" if thousandths == 0 => Some(1000), + _ => None, + } +} + +/// The bundle as published. +pub fn decompressed() -> std::io::Result> { + let mut bytes = Vec::new(); + flate2::read::GzDecoder::new(SCALAR_GZIP).read_to_end(&mut bytes)?; + Ok(bytes) +} + +#[cfg(test)] +mod tests { + use sha2::{Digest, Sha256}; + + use super::*; + + /// The embedded bytes are the release this module names, and nothing + /// else: a file replaced without its digest, or a digest changed without + /// its file, fails here. + #[test] + fn the_embedded_bundle_is_the_pinned_release() { + let bundle = decompressed().expect("the bundle decompresses"); + assert_eq!(hex::encode(Sha256::digest(&bundle)), SCALAR_SHA256); + assert!(SCALAR_SCRIPT_PATH.contains(SCALAR_VERSION)); + } + + /// A client that takes no gzip costs one decompression per process, not + /// one per request: the route is open to whoever `/reference` is open to. + /// Served twice, and three times more, the count stays at one, and both + /// answers are the published bundle. + #[tokio::test] + async fn the_bundle_is_decompressed_once_whatever_the_requests() { + use http_body_util::BodyExt as _; + let mut bodies = Vec::new(); + for _ in 0..5 { + let response = serve_script(HeaderMap::new()).await; + assert_eq!(response.status(), StatusCode::OK); + assert!(response.headers().get(header::CONTENT_ENCODING).is_none()); + bodies.push(response.into_body().collect().await.unwrap().to_bytes()); + } + assert_eq!( + DECOMPRESSIONS.load(std::sync::atomic::Ordering::SeqCst), + 1, + "the bundle must be decompressed once and kept" + ); + assert_eq!(hex::encode(Sha256::digest(&bodies[0])), SCALAR_SHA256); + assert!(bodies.iter().all(|body| body == &bodies[0])); + } + + #[test] + fn gzip_is_sent_only_to_whoever_takes_it() { + let with = |value: &str| { + let mut headers = HeaderMap::new(); + headers.insert( + header::ACCEPT_ENCODING, + HeaderValue::from_str(value).unwrap(), + ); + accepts_gzip(&headers) + }; + assert!(with("gzip, deflate, br, zstd")); + assert!(with("br;q=1.0, GZIP;q=0.5")); + assert!(with("*")); + assert!(!with("br")); + assert!(!with("gzip;q=0")); + assert!(!accepts_gzip(&HeaderMap::new())); + // A coding the field names is decided by its own entry, and `*` only + // stands for the ones it does not name (RFC 9110 §12.5.3). + assert!(!with("gzip;q=0, *;q=1")); + assert!(!with("*, gzip;q=0")); + assert!(with("*;q=0, gzip")); + assert!(!with("br, *;q=0")); + assert!(with("x-gzip")); + // Names are case-insensitive, the weight's as much as the coding's, + // and whitespace around `;` and `=` changes nothing. + assert!(!with("gzip;Q=0")); + assert!(!with("GZIP ; q = 0")); + assert!(!with("gzip;q=0.000, *")); + assert!(with("Gzip;Q=0.001")); + assert!(with("gzip;q=1.000")); + // A weight outside the grammar makes its entry say nothing: the next + // entry, or the wildcard, decides, and nothing panics. + for malformed in ["2", "1.5", "-1", "0.0001", "abc", "", "NaN", "inf", "1e0"] { + assert!( + !with(&format!("gzip;q={malformed}")), + "q={malformed} is no acceptance" + ); + assert!( + with(&format!("gzip;q={malformed}, *")), + "q={malformed} leaves the wildcard deciding" + ); + assert!( + !with(&format!("gzip;q={malformed}, gzip;q=0")), + "q={malformed} leaves the next gzip entry deciding" + ); + } + // An empty field is identity only. + assert!(!with("")); + assert!(!with(" , ;q=1")); + } + + /// No string in the document can end the block it is carried in. + #[test] + fn the_document_cannot_close_its_own_block() { + let mut openapi = utoipa::openapi::OpenApi::default(); + openapi.info.description = Some("".to_owned()); + let page = page_template(&openapi); + assert_eq!(page.matches("").count(), 2, "{page}"); + assert!(page.contains("\\u003c/script>")); + } +} diff --git a/src/scrobblers/lastfm.rs b/src/scrobblers/lastfm.rs index b2a65fe1..fd89b865 100644 --- a/src/scrobblers/lastfm.rs +++ b/src/scrobblers/lastfm.rs @@ -29,6 +29,14 @@ pub struct LastFm { client: reqwest::Client, endpoint: url::Url, application: LastFmApplication, + /// The whole of one authorisation call, its answer included. + /// + /// A listen is bounded by the drain's own deadline. The two calls of the + /// authorisation journey are made for a person waiting on a page or a + /// terminal, and nothing above them bounded them at all (RFC-023 + /// decision 1). It is the `WAVEFLOW_SCROBBLE_REQUEST_TIMEOUT_SECS` the + /// drain uses. + deadline: Duration, } impl LastFm { @@ -36,6 +44,7 @@ impl LastFm { client: reqwest::Client, base: &url::Url, application: LastFmApplication, + deadline: Duration, ) -> Result { // **The trailing slash is kept, and it is not decoration.** Last.fm // documents its endpoint as `/2.0/`, answers `/2.0` with a redirect, @@ -53,8 +62,63 @@ impl LastFm { client, endpoint: submit, application, + deadline, }) } + + /// One signed authorisation call, and the narrow answer it is read into. + /// + /// Bounded twice, which is the whole of RFC-023 decision 1 for this path: + /// in time by [`Self::deadline`], and in size by + /// [`crate::outbound::read_bounded_body`]. Either refusal is + /// `Unavailable`, a failure of the way to Last.fm rather than of what the + /// person brought. + /// + /// **The status, and never the body, on a refusal.** This answer is the + /// one exception decision 10's "log what came back" rule must carve out: + /// it carries the session key itself on success, and on failure it quotes + /// the request token or the application key back in its error text. + async fn call( + &self, + params: &[(&str, String)], + what: &'static str, + ) -> Result { + use crate::services::ServiceError; + let answer = async { + let response = self + .client + .post(self.endpoint.clone()) + .form(params) + .send() + .await + .map_err(|_| { + tracing::warn!(call = what, "last.fm was not reached"); + ServiceError::Unavailable + })?; + let status = response.status(); + if !status.is_success() { + tracing::warn!(call = what, %status, "last.fm refused the call"); + return Err(ServiceError::Invalid); + } + let body = + crate::outbound::read_bounded_body(response, crate::outbound::MAX_ANSWER_BYTES) + .await + .map_err(|error| { + tracing::warn!(call = what, %error, "last.fm's answer was not read"); + ServiceError::Unavailable + })?; + serde_json::from_slice::(&body).map_err(|_| { + tracing::warn!(call = what, "last.fm's answer is not the expected JSON"); + ServiceError::Unavailable + }) + }; + tokio::time::timeout(self.deadline, answer) + .await + .unwrap_or_else(|_| { + tracing::warn!(call = what, "last.fm did not answer in time"); + Err(ServiceError::Unavailable) + }) + } } /// The signature Last.fm requires on every authenticated call. @@ -181,10 +245,25 @@ impl ScrobbleTarget for LastFm { // of the `Retry-After` the header gave — defeating the whole point // of reading that header — and a `5xx` whose body happened to // parse as accepted would be read as a listen that arrived. + // + // **Read under a ceiling** (RFC-023 decision 1): an answer over it, + // or one that breaks off, refines nothing, and the status line + // decides as it does for a body that will not parse. if !status.is_success() { return status_verdict(status, retry_after); } - let answer = response.json::().await.ok(); + let answer = match crate::outbound::read_bounded_body( + response, + crate::outbound::MAX_ANSWER_BYTES, + ) + .await + { + Ok(body) => serde_json::from_slice::(&body).ok(), + Err(error) => { + tracing::warn!(%error, "last.fm's answer was not read; its status line decides"); + None + } + }; match answer.as_ref().and_then(ScrobbleAnswer::verdict) { Some(verdict) => verdict, None => status_verdict(status, retry_after), @@ -316,28 +395,11 @@ impl crate::services::LastFmSessionExchange for LastFm { ]; params.push(("api_sig", signature(¶ms, &self.application.secret))); params.push(("format", "json".to_owned())); - let response = self - .client - .post(self.endpoint.clone()) - .form(¶ms) - .send() - .await - .map_err(|_| ServiceError::Unavailable)?; - if !response.status().is_success() { - // The status, and never the body. **This response is the one - // exception decision 10's "log what came back" rule must carve - // out**: it carries the session key itself on success, and on - // failure it carries the request token back in its error text. - tracing::warn!(status = %response.status(), "last.fm refused the session exchange"); - return Err(ServiceError::Invalid); - } // Read here, and only here. Every other answer from a destination is // judged by its status line — but a session key is the point of this - // call, and there is nowhere else to get it. - let body: SessionResponse = response - .json() - .await - .map_err(|_| ServiceError::Unavailable)?; + // call, and there is nowhere else to get it. A refusing status is + // `Invalid`, as it always was: the token was spent or never approved. + let body: SessionResponse = self.call(¶ms, "auth.getSession").await?; let key = body.session.map(|session| session.key).unwrap_or_default(); if key.is_empty() { // Last.fm answers `200` with an error object for a spent or @@ -375,22 +437,11 @@ impl crate::services::LastFmSessionExchange for LastFm { ]; params.push(("api_sig", signature(¶ms, &self.application.secret))); params.push(("format", "json".to_owned())); - let response = self - .client - .post(self.endpoint.clone()) - .form(¶ms) - .send() - .await - .map_err(|_| ServiceError::Unavailable)?; - if !response.status().is_success() { - // The status and nothing else, like the exchange beside it: a - // failure here quotes the application key back in its error - // text. - tracing::warn!(status = %response.status(), "last.fm refused to mint a request token"); - return Err(ServiceError::Unavailable); - } - let body: TokenResponse = response - .json() + // Every failure is `Unavailable` here, a refusing status included: + // nothing the person brought has been asked yet, so whatever went + // wrong is on the way to Last.fm or on this server's side of it. + let body: TokenResponse = self + .call(¶ms, "auth.getToken") .await .map_err(|_| ServiceError::Unavailable)?; let token = body.token.unwrap_or_default(); @@ -624,7 +675,8 @@ mod tests { api_key: "a-key".to_owned(), secret: "a-secret".to_owned(), }; - let target = LastFm::new(client, &base, application).expect("an adapter"); + let target = + LastFm::new(client, &base, application, Duration::from_secs(5)).expect("an adapter"); assert_eq!( target.endpoint.as_str(), "https://ws.audioscrobbler.com/2.0/" @@ -653,4 +705,86 @@ mod tests { } )); } + + /// An adapter aimed at a listener of the test's own. + async fn reaching(body: axum::body::Body) -> (LastFm, tokio::task::JoinHandle<()>) { + let (url, server) = crate::outbound::tests::answering(body).await; + let base = super::super::validate_destination(&url, true).expect("a destination"); + let client = super::super::outbound_client(Duration::from_secs(5)).expect("a client"); + let application = crate::config::LastFmApplication { + api_key: uuid::Uuid::new_v4().to_string(), + secret: uuid::Uuid::new_v4().to_string(), + }; + let target = + LastFm::new(client, &base, application, Duration::from_secs(10)).expect("an adapter"); + (target, server) + } + + /// A session answer that is well formed and says what it should, but is + /// padded past the ceiling, is not read: the exchange fails as an outage + /// rather than holding the whole body (RFC-023 decision 1). + /// + /// Finite on purpose, and parseable: without the ceiling this same answer + /// is read and yields its key, so the test tells the ceiling from its + /// absence by the outcome and not only by the clock. + #[tokio::test] + async fn a_session_answer_over_the_ceiling_is_not_read() { + use crate::services::{LastFmSessionExchange, ServiceError}; + let key = uuid::Uuid::new_v4().simple().to_string(); + let padding = " ".repeat(crate::outbound::MAX_ANSWER_BYTES); + let (target, server) = reaching(axum::body::Body::from(format!( + "{{\"session\":{{\"key\":\"{key}\"}}{padding}}}" + ))) + .await; + let token = uuid::Uuid::new_v4().to_string(); + let exchanged = target.exchange(&token).await; + server.abort(); + assert!( + matches!(exchanged, Err(ServiceError::Unavailable)), + "an oversized answer must not be read" + ); + + // And the same answer within the ceiling is read, so the refusal above + // is the ceiling's and not the listener's. + let (target, server) = reaching(axum::body::Body::from(format!( + "{{\"session\":{{\"key\":\"{key}\"}}}}" + ))) + .await; + let exchanged = target.exchange(&token).await; + server.abort(); + assert_eq!(exchanged.ok().as_deref(), Some(key.as_str())); + } + + /// The same for the token the CLI journey asks for first. + #[tokio::test] + async fn a_token_answer_over_the_ceiling_is_not_read() { + use crate::services::{LastFmSessionExchange, ServiceError}; + let token = uuid::Uuid::new_v4().simple().to_string(); + let padding = " ".repeat(crate::outbound::MAX_ANSWER_BYTES); + let (target, server) = reaching(axum::body::Body::from(format!( + "{{\"token\":\"{token}\"{padding}}}" + ))) + .await; + let minted = target.request_token().await; + server.abort(); + assert!(matches!(minted, Err(ServiceError::Unavailable))); + } + + /// A listen whose acknowledgement is over the ceiling is judged by its + /// status line, as one that will not parse is. + /// + /// The body says `"error": 9`, a dead session key, so a reader without the + /// ceiling breaks the link over it; that difference is what this tells + /// apart. + #[tokio::test] + async fn a_scrobble_answer_over_the_ceiling_leaves_the_status_line_deciding() { + let padding = " ".repeat(crate::outbound::MAX_ANSWER_BYTES); + let (target, server) = + reaching(axum::body::Body::from(format!("{{\"error\":9{padding}}}"))).await; + let envelope = an_envelope(); + let session_key = uuid::Uuid::new_v4().to_string(); + let verdict = target.submit(&envelope, &session_key).await; + server.abort(); + assert!(matches!(verdict, ScrobbleVerdict::Accepted)); + } } diff --git a/src/scrobblers/maloja.rs b/src/scrobblers/maloja.rs index 214c3b02..de28f7f7 100644 --- a/src/scrobblers/maloja.rs +++ b/src/scrobblers/maloja.rs @@ -149,10 +149,24 @@ impl ScrobbleTarget for Maloja { // The narrowness is the safeguard: one enumerated word, nothing // textual, and a body that will not parse leaves the status line // deciding. - let refused = response - .json::() - .await - .is_ok_and(|answer| answer.status.as_deref() == Some("failure")); + // + // **And the body is read under a ceiling** (RFC-023 decision 1). An + // answer over it, or one that breaks off, is not read at all, and + // the status line decides — exactly as for a body that will not + // parse. The verdict is logged; the body never is. + let refused = match crate::outbound::read_bounded_body( + response, + crate::outbound::MAX_ANSWER_BYTES, + ) + .await + { + Ok(body) => serde_json::from_slice::(&body) + .is_ok_and(|answer| answer.status.as_deref() == Some("failure")), + Err(error) => { + tracing::warn!(%error, "maloja's answer was not read; its status line decides"); + false + } + }; if status.is_success() && refused { tracing::warn!("maloja read the submission and refused it"); return ScrobbleVerdict::PermanentReject; @@ -375,4 +389,27 @@ mod tests { ScrobbleVerdict::Retryable { after: None } )); } + + /// A `200` whose body is over the ceiling is not read, and the status line + /// decides (RFC-023 decision 1). + /// + /// The body is the one refusal this adapter reads, padded: a reader + /// without the ceiling reads all of it and rejects the listen for good, + /// which is the difference this tells apart. + #[tokio::test] + async fn an_answer_over_the_ceiling_leaves_the_status_line_deciding() { + let padding = " ".repeat(crate::outbound::MAX_ANSWER_BYTES); + let (url, server) = crate::outbound::tests::answering(axum::body::Body::from(format!( + "{{\"status\":\"failure\"{padding}}}" + ))) + .await; + let base = super::super::validate_destination(&url, true).expect("a destination"); + let client = super::super::outbound_client(Duration::from_secs(5)).expect("a client"); + let target = Maloja::new(client, &base).expect("an adapter"); + let secret = uuid::Uuid::new_v4().to_string(); + let envelope = an_envelope(); + let verdict = target.submit(&envelope, &secret).await; + server.abort(); + assert!(matches!(verdict, ScrobbleVerdict::Accepted)); + } } diff --git a/src/security_headers.rs b/src/security_headers.rs index 357e3af2..cfc2d2f4 100644 --- a/src/security_headers.rs +++ b/src/security_headers.rs @@ -1,12 +1,15 @@ //! The response headers that tell a browser how far to trust what this server //! sends (RFC-019 decision 2). //! -//! Two surfaces carry them, and only two: +//! Three surfaces carry them: //! //! - **the embedded web client** ([`crate::webui`]), every response of the //! router fallback: the shell, its assets, and its refusals; //! - **the public shares** under `/share/`, whose token *is* the path, so a -//! referrer would hand it to whatever host a page linked to. +//! referrer would hand it to whatever host a page linked to; +//! - **the API reference** at `/reference` ([`crate::reference`]), since +//! RFC-023 serves its script from the binary and can therefore hold it to a +//! policy that names no other host. //! //! The Subsonic façade is deliberately left alone: its observable wire //! behaviour is frozen (RFC-002), and a header a validated client has never @@ -124,6 +127,48 @@ pub async fn share(mut response: Response) -> Response { response } +/// The policy of `/reference` (RFC-023 decision 3), whose bundle is served by +/// this server. +/// +/// - **Scripts from this origin only**: the bundle, at its versioned address. +/// No inline script and no `eval`. The specification block is +/// `type="application/json"`, which no browser executes and no `script-src` +/// governs. +/// - **Styles from this origin and inline.** The one concession, and measured: +/// Scalar injects several `