derive TLS from --public-url, catch a wrong url, sweep expired shares
--https and FILEBROWSER_HTTPS are gone. The scheme of --public-url says the same thing, and two flags for one fact could contradict each other. A TLS proxy now needs the URL, not the flag. relying_party refuses a ceremony whose Origin header does not match the configured origin, naming both in the log. Before this the browser threw a bare SecurityError and nothing said why. Expired shares were never deleted, only refused on read. Db::sweep drops them once a minute, and takes over the unlock prune that create_share_unlock used to do inline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MREADME.md
@@ -74,9 +74,8 @@ filebrowser-ng --root /srv/files --db /var/lib/filebrowser/db.sqlite --bind 0.0.
| `--db` | _(required)_ | SQLite file. Created when missing. |
| `--port` | `8080` | Listen port |
| `--bind` | `127.0.0.1` | Listen address. `0.0.0.0` exposes the server beyond localhost. |
| `--https` | off | Set when behind a TLS-terminating proxy. Marks the cookie `Secure`. Also `FILEBROWSER_HTTPS=true`. |
| `--root-name` | folder name | Display name of the root folder. Also `FILEBROWSER_ROOT_NAME`. |
| `--public-url` | off | Public base URL, e.g. `https://files.example.com`. Share and WebDAV links are built from it instead of the browser's address. Also `FILEBROWSER_PUBLIC_URL`. |
| `--public-url` | off | Public base URL, e.g. `https://files.example.com`. Share and WebDAV links are built from it instead of the browser's address, and an `https` scheme marks cookies `Secure` and binds passkeys to that origin. Also `FILEBROWSER_PUBLIC_URL`. |
| `--cache` | off | Folder for the thumbnail cache. Turns thumbnails on. Also `FILEBROWSER_CACHE`. |
Log level comes from `RUST_LOG` (`error`, `warn`, `info`, `debug`, `trace`).
@@ -84,9 +83,13 @@ Log level comes from `RUST_LOG` (`error`, `warn`, `info`, `debug`, `trace`).
### Reverse proxy
The server speaks plain HTTP. Put a TLS-terminating proxy in front of it and
pass `--https`, or set `FILEBROWSER_HTTPS=true` in `compose.yml`. Without the flag the session cookie is sent over plain HTTP
as well. Search uses server-sent events, so the proxy must not buffer
responses on `/api/search`.
set `--public-url` to the address the browser uses, e.g.
`FILEBROWSER_PUBLIC_URL=https://files.example.com` in `compose.yml`. The
server cannot see the proxy's TLS by itself, so without that URL it assumes
plain HTTP: cookies lose `Secure` and passkeys are bound to the wrong origin.
Search uses server-sent events, so the proxy must not buffer responses on
`/api/search`.
## Thumbnails
@@ -155,6 +158,10 @@ Passkeys need a domain name. Set `--public-url` behind a proxy that rewrites
`Host`, and note that a bare IP address will not work at all. Password sign-in
still works on such a host, but an account requiring both factors does not.
If `--public-url` names an address other than the one in the browser's URL
bar, every passkey operation fails with "Passkeys are set up for a different
address" and the server log names both addresses side by side.
> [!NOTE]
> The account password is not what a mount should carry. HTTP Basic sends a
> password and nothing else, so an account that requires both factors cannot
Mcompose.yml
@@ -6,8 +6,7 @@ services:
- "8080:8080"
environment:
RUST_LOG: info # server log level: trace/debug/info/warn/error
# FILEBROWSER_HTTPS: "true" # behind a TLS-terminating proxy: marks the session cookie Secure
# FILEBROWSER_PUBLIC_URL: https://files.example.com # base of share links (default: the address the browser uses)
# FILEBROWSER_PUBLIC_URL: https://files.example.com # public address: base of share links; https here marks cookies Secure and binds passkeys to it
# FILEBROWSER_ROOT_NAME: Media # UI name of the root folder (default: its folder name, here "data")
# FILEBROWSER_CACHE: /var/cache/filebrowser # turns grid thumbnails on; ffmpeg is in the image
volumes:
Mserver/src/cli.rs
@@ -36,15 +36,11 @@ pub struct Cli {
#[arg(long, env = "FILEBROWSER_CACHE")]
pub cache: Option<PathBuf>,
/// Assume the server runs behind a TLS-terminating reverse proxy
/// (sets the `Secure` attribute on the session cookie).
/// The env form takes `true` or `false`.
#[arg(long, env = "FILEBROWSER_HTTPS")]
pub https: bool,
/// Public base URL of this server, e.g. `https://files.example.com`.
/// Share and WebDAV links in the UI are built from it. Without it they
/// use whatever address the browser is connected to.
/// Share and WebDAV links in the UI are built from it, and an `https`
/// scheme is what tells the server it sits behind a TLS-terminating
/// proxy. Without it links use whatever address the browser is connected
/// to, and the server assumes plain HTTP.
#[arg(long, env = "FILEBROWSER_PUBLIC_URL")]
pub public_url: Option<String>,
}
@@ -55,19 +51,14 @@ mod tests {
use clap::Parser;
use std::path::PathBuf;
/// `defaults_applied` asserts `!https`, which the env var would flip.
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
#[test]
fn defaults_applied() {
let _guard = ENV_LOCK.lock().unwrap();
let c = Cli::try_parse_from(["filebrowser-ng", "--root", "/r", "--db", "/d"]).unwrap();
assert_eq!(c.root, PathBuf::from("/r"));
assert_eq!(c.db, PathBuf::from("/d"));
assert_eq!(c.port, 8080);
assert_eq!(c.bind, "127.0.0.1");
assert_eq!(c.cache, None);
assert!(!c.https);
}
#[test]
@@ -82,28 +73,10 @@ mod tests {
"9000",
"--bind",
"0.0.0.0",
"--https",
])
.unwrap();
assert_eq!(c.port, 9000);
assert_eq!(c.bind, "0.0.0.0");
assert!(c.https);
}
#[test]
fn https_from_env() {
// Env-driven flags carry the daemon's whole config: the compose file
// has no `command:` and must still be able to turn this on.
let _guard = ENV_LOCK.lock().unwrap();
// clap accepts only these two literals for a flag read from the env.
for (val, want) in [("true", true), ("false", false)] {
// SAFETY: the process env is shared; ENV_LOCK serialises every test
// that reads or writes it.
unsafe { std::env::set_var("FILEBROWSER_HTTPS", val) };
let c = Cli::try_parse_from(["filebrowser-ng", "--root", "/r", "--db", "/d"]).unwrap();
assert_eq!(c.https, want, "FILEBROWSER_HTTPS={val}");
}
unsafe { std::env::remove_var("FILEBROWSER_HTTPS") };
}
#[test]
Mserver/src/db.rs
@@ -1028,13 +1028,6 @@ impl Db {
pub async fn create_share_unlock(&self, share_id: i64) -> DbResult<String> {
let token = crate::auth::random_token();
let c = self.0.lock().await;
// Old unlocks go first. The cookie carrying them is a session
// cookie, so it is already gone from every browser; without this the
// rows would accumulate forever, one per unlock.
c.execute(
"DELETE FROM share_unlocks WHERE created_at < ?1",
[expiry_cutoff()],
)?;
c.execute(
"INSERT INTO share_unlocks (token, share_id, created_at) VALUES (?1, ?2, ?3)",
params![token, share_id, now()],
@@ -1042,6 +1035,31 @@ impl Db {
Ok(token)
}
/// Delete what has outlived its use. Returns how many shares and how
/// many unlock rows went.
///
/// Housekeeping only: every read already refuses an expired share, so
/// nothing here is load-bearing and the interval does not matter.
pub async fn sweep(&self) -> DbResult<(usize, usize)> {
let c = self.0.lock().await;
// SQLite parses the timestamp instead of comparing it as text:
// `expires_at` is stored exactly as the client sent it and may carry
// an offset or fractional seconds. An unparseable one yields NULL and
// so survives, which is what `ShareRow::is_expired` decided too.
let shares = c.execute(
"DELETE FROM shares
WHERE expires_at IS NOT NULL AND julianday(expires_at) <= julianday('now')",
[],
)?;
// The cookie carrying an unlock is a session cookie, so it is already
// gone from every browser. Deleting a share takes its own with it.
let unlocks = c.execute(
"DELETE FROM share_unlocks WHERE created_at < ?1",
[expiry_cutoff()],
)?;
Ok((shares, unlocks))
}
/// Whether `token` is a live unlock for `share_id`.
///
/// The share id is part of the lookup, so an unlock for one share cannot
@@ -1305,6 +1323,23 @@ static DUMMY_HASH: std::sync::LazyLock<String> = std::sync::LazyLock::new(|| {
/// browser, so this only bounds the rows left behind by closed sessions.
const UNLOCK_MAX_AGE_DAYS: i64 = 7;
/// How often [`Db::sweep`] runs. A share the picker can only set to the
/// minute is gone from the owner's list about when it says it is.
const SWEEP_EVERY: std::time::Duration = std::time::Duration::from_secs(60);
/// Run [`Db::sweep`] until the process ends. A failed pass is logged and
/// retried on the next one: nothing downstream depends on it having run.
pub async fn sweep_forever(db: Db) {
loop {
tokio::time::sleep(SWEEP_EVERY).await;
match db.sweep().await {
Ok((0, 0)) => {}
Ok((shares, unlocks)) => tracing::debug!(shares, unlocks, "swept expired shares"),
Err(e) => tracing::warn!(error = %e, "the share sweep failed"),
}
}
}
/// The timestamp an unlock row must be newer than to survive a cleanup.
fn expiry_cutoff() -> String {
(chrono::Utc::now() - chrono::Duration::days(UNLOCK_MAX_AGE_DAYS))
@@ -1822,6 +1857,64 @@ mod tests {
assert!(!db.delete_share(s1.id, admin.id).await.unwrap());
}
/// The sweep decides which timestamps are past, and `expires_at` is
/// stored in whatever RFC 3339 shape the client sent.
#[tokio::test]
async fn the_sweep_drops_expired_shares_and_stale_unlocks() {
let (db, admin) = db_with_admin().await;
let make = async |token: &str, expires: Option<&str>| {
db.create_share(admin.id, token, "docs", false, Mode::Ro, expires, None)
.await
.unwrap()
};
let past_offset = make("gone-offset", Some("2000-01-01T00:00:00+02:00")).await;
make("gone-utc", Some("2000-01-01T00:00:00Z")).await;
let future = make("stays-future", Some("2999-01-01T00:00:00Z")).await;
make("stays-forever", None).await;
// `is_expired` keeps an unreadable timestamp; the sweep must agree,
// or the two disagree about the same row.
make("stays-garbage", Some("not-a-date")).await;
let fresh = db.create_share_unlock(future.id).await.unwrap();
let stale = db.create_share_unlock(future.id).await.unwrap();
let doomed = db.create_share_unlock(past_offset.id).await.unwrap();
{
let c = db.0.lock().await;
c.execute(
"UPDATE share_unlocks SET created_at = '2000-01-01T00:00:00Z' WHERE token = ?1",
[&stale],
)
.unwrap();
}
let (shares, unlocks) = db.sweep().await.unwrap();
assert_eq!(shares, 2, "only the two past timestamps");
// The stale one, plus the cascade from the expired share it opened.
assert_eq!(unlocks, 1, "the cascade runs with the share, not here");
for token in ["gone-offset", "gone-utc"] {
assert!(db.share_by_token(token).await.unwrap().is_none(), "{token}");
}
for token in ["stays-future", "stays-forever", "stays-garbage"] {
assert!(db.share_by_token(token).await.unwrap().is_some(), "{token}");
}
assert!(db.share_unlock_valid(&fresh, future.id).await.unwrap());
assert!(!db.share_unlock_valid(&stale, future.id).await.unwrap());
assert!(
!db.share_unlock_valid(&doomed, past_offset.id)
.await
.unwrap(),
"an unlock must not outlive the share it opened"
);
assert_eq!(
db.sweep().await.unwrap(),
(0, 0),
"a second pass is a no-op"
);
}
/// The unlock token is what a visitor's cookie carries, so an unlock
/// that opened the wrong share would be a full bypass of the password.
#[tokio::test]
Mserver/src/error.rs
@@ -14,6 +14,8 @@ pub struct AppState {
/// What the UI calls the root folder (`--root-name`, else its file name).
pub root_name: String,
/// Whether we sit behind a TLS-terminating reverse proxy.
/// Whether the browser reaches this server over TLS. Derived from
/// `--public-url`; see [`crate::secure`].
pub https: bool,
/// `--public-url` with any trailing slash removed; `None` when unset.
pub public_url: Option<String>,
Mserver/src/lib.rs
@@ -26,6 +26,19 @@ use crate::cli::Cli;
use crate::db::Db;
use crate::error::AppState;
/// Whether the browser reaches this server over TLS.
///
/// The process itself only ever speaks plain HTTP, so it cannot observe this;
/// `--public-url` is the operator telling it. The answer decides the `Secure`
/// attribute on cookies and the origin passkeys are bound to, and both must
/// agree with the address in the URL bar.
fn secure(public_url: Option<&str>) -> bool {
public_url.is_some_and(|u| {
u.split_once("://")
.is_some_and(|(scheme, _)| scheme.eq_ignore_ascii_case("https"))
})
}
/// Validate the CLI config and build the running state + router without
/// binding the port (so tests can exercise everything up to `serve`).
pub async fn build_app(cli: &Cli) -> anyhow::Result<(axum::Router, SocketAddr)> {
@@ -55,12 +68,13 @@ pub async fn build_app(cli: &Cli) -> anyhow::Result<(axum::Router, SocketAddr)>
if let Some(dir) = cli.cache.clone() {
tokio::spawn(crate::thumb::sweep_forever(dir));
}
tokio::spawn(crate::db::sweep_forever(db.clone()));
let state = Arc::new(AppState {
db,
root: root.clone(),
root_name,
https: cli.https,
https: secure(cli.public_url.as_deref()),
public_url: cli
.public_url
.as_deref()
@@ -146,11 +160,19 @@ mod tests {
port: 8080,
bind: "127.0.0.1".into(),
cache: None,
https: false,
public_url: None,
}
}
#[test]
fn tls_comes_from_the_public_url_scheme() {
assert!(secure(Some("https://files.example.com")));
assert!(secure(Some("HTTPS://files.example.com")));
assert!(!secure(Some("http://files.example.com")));
assert!(!secure(Some("files.example.com")));
assert!(!secure(None));
}
#[tokio::test]
async fn build_app_ok() {
let tmp = tempfile::tempdir().unwrap();
Mserver/src/webauthn.rs
@@ -43,6 +43,7 @@ pub fn relying_party(
headers: &HeaderMap,
) -> Result<Webauthn, ApiError> {
let origin = origin(state, uri, headers).ok_or_else(misconfigured)?;
check_origin(&origin, headers)?;
// `domain()` is None for a bare IP address, and WebAuthn does not work on
// one at all — the RP ID has to be a registrable domain.
let rp_id = origin.domain().ok_or_else(misconfigured)?;
@@ -56,6 +57,10 @@ pub fn relying_party(
/// The origin the browser will report, as far as the server can tell.
///
/// Falls back to the request's own host, which is plain HTTP because that is
/// all this process ever speaks. Behind a TLS proxy that guess is wrong and
/// `--public-url` is the only way to correct it.
///
/// The host arrives in one of two places depending on the protocol version.
/// HTTP/1.1 sends a `Host` header; HTTP/2 sends `:authority`, which hyper
/// puts in the URI and does *not* mirror into a header. Reading only one of
@@ -68,8 +73,36 @@ fn origin(state: &AppState, uri: &Uri, headers: &HeaderMap) -> Option<Url> {
Some(a) => a.as_str().to_string(),
None => headers.get(header::HOST)?.to_str().ok()?.to_string(),
};
let scheme = if state.https { "https" } else { "http" };
Url::parse(&format!("{scheme}://{host}")).ok()
Url::parse(&format!("http://{host}")).ok()
}
/// Refuse a ceremony the browser could not complete anyway.
///
/// A challenge built for the wrong origin fails in the browser with a bare
/// `SecurityError`, or at the last step with a mismatch nobody can see. The
/// `Origin` header is what the browser will sign, so comparing it here turns
/// both into one message that names the two addresses.
///
/// Absent on requests that are not a browser fetch, and then unenforced.
fn check_origin(configured: &Url, headers: &HeaderMap) -> Result<(), ApiError> {
let Some(browser) = headers.get(header::ORIGIN).and_then(|v| v.to_str().ok()) else {
return Ok(());
};
let expected = configured.origin().ascii_serialization();
if browser == expected {
return Ok(());
}
tracing::error!(
%browser,
%expected,
"passkeys are configured for a different address than the browser is on; \
set --public-url to the address the browser uses"
);
Err(ApiError::localized(
StatusCode::INTERNAL_SERVER_ERROR,
"passkeys are set up for a different address",
"err_passkey_origin",
))
}
/// The client is told nothing but "not available here".
@@ -202,6 +235,23 @@ pub fn take(id: &str) -> Option<Pending> {
mod tests {
use super::*;
#[test]
fn a_browser_on_another_origin_is_refused() {
let configured = Url::parse("https://files.example.com/").unwrap();
let header = |v: &str| {
let mut h = HeaderMap::new();
h.insert(header::ORIGIN, v.parse().unwrap());
h
};
assert!(check_origin(&configured, &HeaderMap::new()).is_ok());
assert!(check_origin(&configured, &header("https://files.example.com")).is_ok());
// The three ways --public-url goes wrong.
assert!(check_origin(&configured, &header("http://files.example.com")).is_err());
assert!(check_origin(&configured, &header("https://other.example.com")).is_err());
assert!(check_origin(&configured, &header("https://files.example.com:8443")).is_err());
}
#[test]
fn a_handle_answers_once() {
let id = put(Pending::NeedsPassword { user_id: 7 });
Mweb/src/i18n.rs
@@ -296,6 +296,7 @@ i18n_keys! {
ERR_PASSKEY_LIMIT = "err_passkey_limit" => "This account already has as many passkeys as it may.",
ERR_PASSKEY_MALFORMED = "err_passkey_malformed" => "The browser sent unreadable data.",
ERR_PASSKEY_NOT_FOUND = "err_passkey_not_found" => "No such passkey.",
ERR_PASSKEY_ORIGIN = "err_passkey_origin" => "Passkeys are set up for a different address.",
ERR_PASSKEY_UNAVAILABLE = "err_passkey_unavailable" => "Passkeys are not available here.",
ERR_PASSWORD_LAST_CREDENTIAL = "err_password_last_credential" => "Add a passkey before removing your password.",
ERR_PASSWORD_SHORT = "err_password_short" => "password must be at least 8 characters",
@@ -794,6 +795,10 @@ const DE: &[(&str, &str)] = &[
"Der Browser hat unlesbare Daten gesendet.",
),
(k::ERR_PASSKEY_NOT_FOUND, "Passkey nicht gefunden."),
(
k::ERR_PASSKEY_ORIGIN,
"Passkeys sind für eine andere Adresse eingerichtet.",
),
(
k::ERR_PASSKEY_UNAVAILABLE,
"Passkeys sind hier nicht verfügbar.",
@@ -1478,6 +1483,10 @@ const FR: &[(&str, &str)] = &[
"Le navigateur a envoyé des données illisibles.",
),
(k::ERR_PASSKEY_NOT_FOUND, "Clé d'accès introuvable."),
(
k::ERR_PASSKEY_ORIGIN,
"Les clés d'accès sont configurées pour une autre adresse.",
),
(
k::ERR_PASSKEY_UNAVAILABLE,
"Les clés d'accès ne sont pas disponibles ici.",
Mweb/src/passkey.rs
@@ -36,12 +36,17 @@ export function passkeyCancel() {
}
export async function passkeyCreate(optionsJson) {
const opts = PublicKeyCredential.parseCreationOptionsFromJSON(
JSON.parse(optionsJson).publicKey
);
const cred = await navigator.credentials.create({ publicKey: opts });
if (!cred) throw new Error("no credential was created");
return JSON.stringify(cred.toJSON());
try {
const opts = PublicKeyCredential.parseCreationOptionsFromJSON(
JSON.parse(optionsJson).publicKey
);
const cred = await navigator.credentials.create({ publicKey: opts });
if (!cred) throw new Error("no credential was created");
return JSON.stringify(cred.toJSON());
} catch (e) {
console.error("passkey registration failed", e);
throw e;
}
}
// Resolves to the credential JSON, or to null when the request was cancelled
@@ -68,6 +73,7 @@ export async function passkeyGet(optionsJson, conditional) {
return JSON.stringify(json);
} catch (e) {
if (e && e.name === "AbortError") return null;
console.error("passkey assertion failed", e);
throw e;
} finally {
if (pending === ctl) pending = null;
@@ -124,7 +130,16 @@ pub async fn get(options: &str, conditional: bool) -> Result<Option<String>, Str
///
/// The API deliberately returns the same `NotAllowedError` whether the user
/// cancelled or nothing matched, so there is nothing more specific to say.
/// Claiming "you have no passkey here" would often be wrong.
fn error_text(_e: JsValue) -> String {
crate::i18n::t(crate::i18n::k::PASSKEY_NOT_USED).to_string()
/// Claiming "you have no passkey here" would often be wrong. Any other name
/// is a deployment fault, not a user choice — `SecurityError` means the RP ID
/// does not match the browser's origin — so it is worth showing.
fn error_text(e: JsValue) -> String {
let text = crate::i18n::t(crate::i18n::k::PASSKEY_NOT_USED).to_string();
match js_sys::Reflect::get(&e, &JsValue::from_str("name"))
.ok()
.and_then(|v| v.as_string())
{
Some(name) if name != "NotAllowedError" => format!("{text} ({name})"),
_ => text,
}
}