From 0b6cb74c0d6dddeac5ca1bae1642a7d160976373 Mon Sep 17 00:00:00 2001 From: Ruben Hensen Date: Fri, 19 Jun 2026 14:49:37 +0200 Subject: [PATCH] feat(metrics): require a bearer token on /metrics when configured Add an optional `metrics_token` config (also settable via ROCKET_METRICS_TOKEN). When set, /metrics requires `Authorization: Bearer ` (constant-time compared via subtle); when unset it stays open and logs a startup warning, so rolling this out doesn't break existing scrapers. The auth lives in the app, not the ingress, so it travels with cryptify to external hosts that can't be firewalled. /health stays public. --- conf/config.dev.toml | 3 + conf/config.toml | 6 +- src/config.rs | 11 ++++ src/main.rs | 136 ++++++++++++++++++++++++++++++++++++++----- 4 files changed, 142 insertions(+), 14 deletions(-) diff --git a/conf/config.dev.toml b/conf/config.dev.toml index 0e51aaa..4484c50 100644 --- a/conf/config.dev.toml +++ b/conf/config.dev.toml @@ -15,5 +15,8 @@ usage_db = "/app/data/usage.db" # pkg_url = "https://pkg.staging.yivi.app" pkg_url = "http://postguard-pkg:8087" chunk_size = 5000000 +# Leave unset in dev so /metrics is freely scrapable. In prod set this (or the +# ROCKET_METRICS_TOKEN env var) so /metrics requires `Authorization: Bearer `. +# metrics_token = "dev-token" # When true, finalize logs the email it WOULD have sent and skips SMTP. # staging_mode = true diff --git a/conf/config.toml b/conf/config.toml index b5aaff7..d5d22fa 100644 --- a/conf/config.toml +++ b/conf/config.toml @@ -8,4 +8,8 @@ smtp_port = 1025 # smtp_username = "user" # smtp_password = "pw" allowed_origins = "^https://postguard.(eu|nl)$" -pkg_url = "https://postguard-main.cs.ru.nl/pkg" +pkg_url = "https://postguard-main.cs.ru.nl/pkg" +# Bearer token required to scrape /metrics. Prefer injecting it as a secret +# via the ROCKET_METRICS_TOKEN env var rather than committing it here. When +# unset, /metrics is publicly accessible (a startup warning is logged). +# metrics_token = "change-me" diff --git a/src/config.rs b/src/config.rs index 7773c41..2352d96 100644 --- a/src/config.rs +++ b/src/config.rs @@ -16,6 +16,7 @@ pub struct RawCryptifyConfig { chunk_size: Option, session_ttl_secs: Option, staging_mode: Option, + metrics_token: Option, } #[derive(Debug, Deserialize)] @@ -35,6 +36,7 @@ pub struct CryptifyConfig { chunk_size: u64, session_ttl_secs: u64, staging_mode: bool, + metrics_token: Option, } impl From for CryptifyConfig { @@ -57,6 +59,7 @@ impl From for CryptifyConfig { chunk_size: config.chunk_size.unwrap_or(5_000_000), session_ttl_secs: config.session_ttl_secs.unwrap_or(3600), staging_mode: config.staging_mode.unwrap_or(false), + metrics_token: config.metrics_token, } } } @@ -118,6 +121,13 @@ impl CryptifyConfig { self.staging_mode } + /// Bearer token required to scrape `/metrics`. `None` leaves the endpoint + /// open (with a startup warning); when set, requests must present + /// `Authorization: Bearer `. + pub fn metrics_token(&self) -> Option<&str> { + self.metrics_token.as_deref() + } + #[cfg(test)] pub(crate) fn for_test(server_url: &str, staging_mode: bool) -> Self { CryptifyConfig { @@ -135,6 +145,7 @@ impl CryptifyConfig { chunk_size: 5_000_000, session_ttl_secs: 3600, staging_mode, + metrics_token: None, } } } diff --git a/src/main.rs b/src/main.rs index a6c0b9e..4355fc0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -137,8 +137,51 @@ fn health() -> &'static str { "OK" } +/// Request guard protecting `/metrics`. When `metrics_token` is configured, +/// the endpoint requires `Authorization: Bearer ` (constant-time +/// compared); otherwise it stays open (a startup warning is logged). This +/// auth lives in the app rather than the ingress so the protection travels +/// with cryptify to every deployment, including external hosts we can't +/// firewall. +struct MetricsAuth; + +#[rocket::async_trait] +impl<'r> FromRequest<'r> for MetricsAuth { + type Error = (); + async fn from_request(request: &'r rocket::Request<'_>) -> rocket::request::Outcome { + let expected = request + .rocket() + .state::() + .and_then(CryptifyConfig::metrics_token); + + // No token configured → metrics is open (warned about at startup). + let Some(expected) = expected else { + return rocket::request::Outcome::Success(MetricsAuth); + }; + + let presented = request + .headers() + .get_one("Authorization") + .and_then(|h| { + h.strip_prefix("Bearer ") + .or_else(|| h.strip_prefix("bearer ")) + }) + .map(str::trim); + + match presented { + Some(token) if constant_time_eq(token, expected) => { + rocket::request::Outcome::Success(MetricsAuth) + } + _ => rocket::request::Outcome::Error((rocket::http::Status::Unauthorized, ())), + } + } +} + #[get("/metrics")] -fn metrics_endpoint(metrics: &State>) -> rocket::response::content::RawText { +fn metrics_endpoint( + _auth: MetricsAuth, + metrics: &State>, +) -> rocket::response::content::RawText { rocket::response::content::RawText(metrics.render()) } @@ -888,11 +931,11 @@ struct UploadStatusResponse { prev_offset: Option, } -/// Constant-time compare of the recovery token. Hex-encoded equal-length -/// strings, but `subtle::ConstantTimeEq` makes the timing independent of -/// where the bytes start to differ — defeats timing oracles even though -/// 32 bytes of high-entropy random aren't realistically guessable. -fn recovery_tokens_match(presented: &str, expected: &str) -> bool { +/// Constant-time string equality. `subtle::ConstantTimeEq` makes the timing +/// independent of where the bytes start to differ — defeats timing oracles +/// on secret comparisons. Used for the recovery token and the `/metrics` +/// bearer token. A length difference returns early (lengths aren't secret). +fn constant_time_eq(presented: &str, expected: &str) -> bool { use subtle::ConstantTimeEq; if presented.len() != expected.len() { return false; @@ -918,7 +961,7 @@ async fn upload_status( .ok_or_else(|| Error::upload_session_not_found(uuid, "expired_or_unknown"))?; let state = state.lock().await; - if !recovery_tokens_match(&recovery_token.0, &state.recovery_token) { + if !constant_time_eq(&recovery_token.0, &state.recovery_token) { // Same body shape as evicted/unknown so the response doesn't leak // session existence to a token-guessing attacker. return Err(Error::upload_session_not_found(uuid, "expired_or_unknown")); @@ -1318,6 +1361,13 @@ async fn rocket() -> _ { .extract::() .expect("Missing configuration"); + if config.metrics_token().is_none() { + log::warn!( + "metrics_token is not set — /metrics is publicly accessible without authentication. \ + Set `metrics_token` in config (or ROCKET_METRICS_TOKEN) to require a Bearer token." + ); + } + let pkg_params_url = format!("{}/v2/sign/parameters", config.pkg_url()); let response = minreq::get(&pkg_params_url) .with_timeout(10) @@ -1780,16 +1830,16 @@ mod tests { } #[rocket::async_test] - async fn recovery_tokens_match_constant_time_helper() { + async fn constant_time_eq_helper() { // The function under test is the constant-time wrapper itself — // we can't observe timing in a unit test, but we can pin the // value-equality semantics so a future refactor doesn't silently // turn it into `presented == expected`. - assert!(recovery_tokens_match("abc123", "abc123")); - assert!(!recovery_tokens_match("abc123", "abc124")); - assert!(!recovery_tokens_match("abc123", "abc12")); // length mismatch - assert!(!recovery_tokens_match("", "abc")); - assert!(recovery_tokens_match("", "")); + assert!(constant_time_eq("abc123", "abc123")); + assert!(!constant_time_eq("abc123", "abc124")); + assert!(!constant_time_eq("abc123", "abc12")); // length mismatch + assert!(!constant_time_eq("", "abc")); + assert!(constant_time_eq("", "")); } // Browser preflight regression: design AC for #146 explicitly required @@ -2726,6 +2776,66 @@ mod integration { let _ = std::fs::remove_dir_all(dir); } + // Minimal Rocket exposing only /metrics, with the given config managed so + // the MetricsAuth guard can read `metrics_token`. Avoids needing a real + // VerifyingKey / TestSetup. + fn metrics_only_config(with_token: bool) -> CryptifyConfig { + let (figment, _dir) = test_figment(); + let figment = if with_token { + figment.merge(("metrics_token", "s3cret")) + } else { + figment + }; + figment.extract::().expect("extract config") + } + + async fn metrics_only_client(config: CryptifyConfig) -> Client { + let rocket = rocket::build() + .mount("/", routes![metrics_endpoint]) + .manage(config) + .manage(std::sync::Arc::new(Metrics::new())); + Client::tracked(rocket).await.expect("valid rocket") + } + + #[rocket::async_test] + async fn metrics_requires_bearer_when_token_configured() { + let client = metrics_only_client(metrics_only_config(true)).await; + + // No Authorization header → 401. + assert_eq!( + client.get("/metrics").dispatch().await.status(), + Status::Unauthorized + ); + // Wrong token → 401. + assert_eq!( + client + .get("/metrics") + .header(Header::new("Authorization", "Bearer wrong")) + .dispatch() + .await + .status(), + Status::Unauthorized + ); + // Correct token → 200 with the metrics body. + let ok = client + .get("/metrics") + .header(Header::new("Authorization", "Bearer s3cret")) + .dispatch() + .await; + assert_eq!(ok.status(), Status::Ok); + assert!(ok + .into_string() + .await + .unwrap_or_default() + .contains("cryptify_uploads_total")); + } + + #[rocket::async_test] + async fn metrics_open_when_token_unset() { + let client = metrics_only_client(metrics_only_config(false)).await; + assert_eq!(client.get("/metrics").dispatch().await.status(), Status::Ok); + } + #[rocket::async_test] async fn upload_happy_path_multi_chunk() { // Two chunks >1 MiB to exercise the rolling token chain across