diff --git a/README.md b/README.md index e644e98..9955d3a 100644 --- a/README.md +++ b/README.md @@ -438,6 +438,14 @@ re-run `:share` on those entries to publish them again. Uploaded files are only ever served as types that cannot execute — anything else is handed back as an opaque download. +A write that a browser says came from another origin — `Sec-Fetch-Site`, or +`Origin` against `Host` where a browser is too old to send that — is refused, +so a proxy must pass the `Host` header through unchanged (traefik and nginx's +`proxy_set_header Host $host` both do). Only the upload routes accept a body as +large as `WORKSPACE_MAX_UPLOAD_MB`; everything else stops at 16 MiB, and the +login at 16 KiB, before anyone has proved who they are. `SIGTERM` gets in-flight +requests eight seconds to finish, inside the ten `docker stop` waits. + ### Docker ```sh diff --git a/compose.yml b/compose.yml index 9ae4a91..b54badf 100644 --- a/compose.yml +++ b/compose.yml @@ -3,6 +3,8 @@ services: build: . image: narl-workspace:latest restart: unless-stopped + # tini as PID 1: forwards SIGTERM and reaps anything the binary leaves behind. + init: true env_file: - .env environment: diff --git a/src/auth.rs b/src/auth.rs index 44bc02e..5dede65 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,6 +1,11 @@ +use std::convert::Infallible; + use axum::{ - extract::FromRequestParts, - http::{header, request::Parts}, + extract::{FromRequestParts, OptionalFromRequestParts, Request}, + http::{header, request::Parts, HeaderMap, StatusCode}, + middleware::Next, + response::{IntoResponse, Response}, + Json, }; use base64::{engine::general_purpose::URL_SAFE_NO_PAD as B64, Engine}; use hmac::{Hmac, Mac}; @@ -97,9 +102,66 @@ impl FromRequestParts for Session { } } +/// For a handler that behaves differently for a caller who is logged in, but +/// must also answer one who is not. +impl OptionalFromRequestParts for Session { + type Rejection = Infallible; + + async fn from_request_parts( + parts: &mut Parts, + state: &AppState, + ) -> Result, Self::Rejection> { + Ok(>::from_request_parts(parts, state) + .await + .ok()) + } +} + +/// Refuse a write that another site's page asked the browser to make. +/// `SameSite=Lax` already keeps the cookie off a cross-site POST, but not off +/// one from a sibling subdomain, which counts as the same site — and a +/// multipart form needs no preflight to get sent. A browser says where a +/// request came from; a request that says nothing is not from a browser page +/// and carries no ambient cookie worth stealing. +pub async fn same_origin(request: Request, next: Next) -> Response { + if !request.method().is_safe() && !is_same_origin(request.headers()) { + tracing::warn!( + "refused a cross-origin {} {}", + request.method(), + request.uri().path() + ); + return ( + StatusCode::FORBIDDEN, + Json(serde_json::json!({ "error": "cross-origin request refused" })), + ) + .into_response(); + } + next.run(request).await +} + +fn is_same_origin(headers: &HeaderMap) -> bool { + // Every current browser sends this, and it cannot be set from a page. + if let Some(site) = headers.get("sec-fetch-site") { + return matches!(site.as_bytes(), b"same-origin" | b"none"); + } + // Older browsers: the Origin they send against the host they asked. + let Some(origin) = headers.get(header::ORIGIN) else { + return true; + }; + let origin_host = origin + .to_str() + .ok() + .and_then(|origin| origin.split_once("://")) + .map(|(_, host)| host); + let host = headers.get(header::HOST).and_then(|h| h.to_str().ok()); + matches!((origin_host, host), (Some(a), Some(b)) if a.eq_ignore_ascii_case(b)) +} + #[cfg(test)] mod tests { - use super::{credentials_match, issue_token, verify_token}; + use axum::http::{HeaderMap, HeaderValue}; + + use super::{credentials_match, is_same_origin, issue_token, verify_token}; use crate::config::{BackupConfig, Config, MailConfig, SftpConfig}; fn config(session_days: i64) -> Config { @@ -188,4 +250,27 @@ mod tests { assert!(!credentials_match(&config, "nar", "hunter2")); assert!(!credentials_match(&config, "", "")); } + + fn headers(pairs: &[(&'static str, &'static str)]) -> HeaderMap { + let mut map = HeaderMap::new(); + for (name, value) in pairs { + map.insert(*name, HeaderValue::from_static(value)); + } + map + } + + #[test] + fn only_same_origin_writes_get_through() { + assert!(is_same_origin(&headers(&[("sec-fetch-site", "same-origin")]))); + assert!(!is_same_origin(&headers(&[("sec-fetch-site", "same-site")]))); + assert!(!is_same_origin(&headers(&[("sec-fetch-site", "cross-site")]))); + + let host = ("host", "diary.example.org"); + assert!(is_same_origin(&headers(&[("origin", "https://diary.example.org"), host]))); + assert!(!is_same_origin(&headers(&[("origin", "https://evil.example.org"), host]))); + assert!(!is_same_origin(&headers(&[("origin", "null"), host]))); + + // curl, the CLI, a script with its own credentials. + assert!(is_same_origin(&headers(&[host]))); + } } diff --git a/src/main.rs b/src/main.rs index 2ad7ca2..f115515 100644 --- a/src/main.rs +++ b/src/main.rs @@ -129,7 +129,12 @@ async fn main() -> Result<()> { }); } - serve(bind, max_upload, state).await + let db = state.db.clone(); + serve(bind, max_upload, state).await?; + // Checkpoints the write-ahead log, so what was written is in diary.db + // itself and not only beside it. + db.close().await; + Ok(()) } /// Open the workspace database and bring it up to date. Both the server and the @@ -162,9 +167,9 @@ async fn open_db(config: &Config) -> Result { async fn serve(bind: std::net::SocketAddr, max_upload: usize, state: AppState) -> Result<()> { let app = Router::new() - .nest("/api", routes::api_router()) + .nest("/api", routes::api_router(max_upload)) .fallback(static_files::handler) - .layer(axum::extract::DefaultBodyLimit::max(max_upload + 1024 * 1024)) + .layer(axum::middleware::from_fn(auth::same_origin)) .layer(CompressionLayer::new()) .layer(security_headers()) .layer(TraceLayer::new_for_http()) @@ -176,12 +181,26 @@ async fn serve(bind: std::net::SocketAddr, max_upload: usize, state: AppState) - tracing::info!("narl-workspace listening on http://{bind}"); - axum::serve(listener, app) - .with_graceful_shutdown(shutdown_signal()) - .await?; + let server = axum::serve(listener, app).with_graceful_shutdown(async { + shutdown_signal().await; + tracing::info!("shutting down"); + }); + // Requests in flight get a few seconds to finish; an upload that would + // take longer is cut off rather than holding the process past the point + // where a supervisor stops asking and kills it. + tokio::select! { + result = server => result?, + () = async { + shutdown_signal().await; + tokio::time::sleep(SHUTDOWN_GRACE).await; + } => tracing::warn!("requests still open after {SHUTDOWN_GRACE:?}, closing anyway"), + } Ok(()) } +/// Below the ten seconds `docker stop` waits before it sends SIGKILL. +const SHUTDOWN_GRACE: std::time::Duration = std::time::Duration::from_secs(8); + /// Content-Security-Policy for the app shell. `unsafe-inline` is granted to /// styles only, because CodeMirror injects its theme as a