diff --git a/CHANGELOG.md b/CHANGELOG.md index dc1b9c8..6218b5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- **Login/session hardening** (#334): client IP for login rate limiting is now the TCP peer address; `X-Forwarded-For`/`X-Real-IP` are honoured only with the new `TRAPFALL_TRUST_PROXY=true` (**breaking for reverse-proxy deployments**: set it, otherwise all clients share the proxy IP). Lockout is keyed per (email, IP) so an attacker cannot lock the admin out from elsewhere. Argon2 runs on the blocking pool. First-admin setup is atomic. Session tokens are stored hashed; **existing sessions are invalidated once on upgrade** (users re-login). Changing the password revokes other sessions. + ## [0.3.0] - 2026-09-03 ### Added diff --git a/crates/trapfall-core/Cargo.toml b/crates/trapfall-core/Cargo.toml index 1c24f60..0487f41 100644 --- a/crates/trapfall-core/Cargo.toml +++ b/crates/trapfall-core/Cargo.toml @@ -18,6 +18,7 @@ chrono = { workspace = true } uuid = { workspace = true } tracing = { workspace = true } rand = "0.8" +tokio = { workspace = true } [dev-dependencies] tokio = { workspace = true } diff --git a/crates/trapfall-core/src/auth.rs b/crates/trapfall-core/src/auth.rs index 235c7b2..fa5ae09 100644 --- a/crates/trapfall-core/src/auth.rs +++ b/crates/trapfall-core/src/auth.rs @@ -26,6 +26,13 @@ fn hash_params() -> &'static Params { PARAMS.get_or_init(|| Params::new(19456, 2, 1, None).unwrap_or_default()) } +/// A valid Argon2 hash of a random value, used to equalise timing for unknown users. +fn dummy_hash() -> &'static str { + use std::sync::OnceLock; + static HASH: OnceLock = OnceLock::new(); + HASH.get_or_init(|| hash_password(&Uuid::new_v4().to_string()).unwrap_or_default()) +} + /// Session duration in days. const SESSION_DURATION_DAYS: i64 = 7; @@ -83,6 +90,45 @@ pub fn verify_password(password: &str, hash: &str) -> bool { argon2.verify_password(password.as_bytes(), &parsed).is_ok() } +/// Hash a password on the blocking pool so Argon2 (tens of ms, 19 MiB) never +/// stalls the single-threaded async runtime. +pub async fn hash_password_async(password: &str) -> Result { + let password = password.to_string(); + tokio::task::spawn_blocking(move || hash_password(&password)) + .await + .map_err(|e| anyhow::anyhow!("hash task failed: {e}"))? +} + +/// Verify a password on the blocking pool (see [`hash_password_async`]). +pub async fn verify_password_async(password: &str, hash: &str) -> bool { + let (password, hash) = (password.to_string(), hash.to_string()); + tokio::task::spawn_blocking(move || verify_password(&password, &hash)).await.unwrap_or(false) +} + +/// Prefix marking a stored (hashed) session token. Rows without it are legacy +/// plaintext tokens and are purged at startup. +pub const SESSION_TOKEN_PREFIX: &str = "h1:"; + +/// Per-(email, IP) failed-login threshold within [`LOCKOUT_WINDOW_MINUTES`]. +const MAX_FAILS_PER_EMAIL_IP: i64 = 5; +/// Per-IP failed-login threshold within [`LOCKOUT_WINDOW_MINUTES`]. +/// +/// Deliberately high: behind a reverse proxy without `TRAPFALL_TRUST_PROXY` +/// every client shares one IP, and a low cap would let one attacker lock out +/// everyone. +const MAX_FAILS_PER_IP: i64 = 100; +/// Global per-email ceiling across all IPs. Bounds IP-rotating guessing +/// (botnets, IPv6 ranges) at ~30 guesses / 15 min. Trade-off: an attacker with +/// many IPs can still lock the account for the rest of the window. +const MAX_FAILS_PER_EMAIL: i64 = 30; +const LOCKOUT_WINDOW_MINUTES: i64 = 15; + +/// Hash a session token for storage (blake3; the token is a high-entropy +/// random value so a fast hash is appropriate). +pub fn hash_session_token(token: &str) -> String { + format!("{SESSION_TOKEN_PREFIX}{}", blake3::hash(token.as_bytes()).to_hex()) +} + /// Validate password strength. pub fn validate_password(password: &str) -> Result<(), String> { if password.len() < MIN_PASSWORD_LEN { @@ -132,7 +178,7 @@ impl Store { pub async fn create_user(&self, email: &str, name: &str, password: &str) -> Result { validate_email(email).map_err(|e| anyhow::anyhow!(e))?; validate_password(password).map_err(|e| anyhow::anyhow!(e))?; - let hash = hash_password(password)?; + let hash = hash_password_async(password).await?; self.backend().create_user(email, name, &hash).await?; let user = self .backend() @@ -142,6 +188,23 @@ impl Store { Ok(user) } + /// Create the first admin atomically (setup wizard). Returns `Ok(None)` + /// when a user already exists, so two concurrent setups cannot both win. + pub async fn create_first_user(&self, email: &str, name: &str, password: &str) -> Result> { + validate_email(email).map_err(|e| anyhow::anyhow!(e))?; + validate_password(password).map_err(|e| anyhow::anyhow!(e))?; + let hash = hash_password_async(password).await?; + if !self.backend().create_first_user(email, name, &hash).await? { + return Ok(None); + } + let user = self + .backend() + .get_user_by_email(email) + .await? + .ok_or_else(|| anyhow::anyhow!("user not found after insert"))?; + Ok(Some(user)) + } + /// Get user by email. pub async fn get_user_by_email(&self, email: &str) -> Result> { self.backend().get_user_by_email(email).await @@ -155,10 +218,23 @@ impl Store { /// Update user password. pub async fn update_password(&self, user_id: &str, new_password: &str) -> Result<()> { validate_password(new_password).map_err(|e| anyhow::anyhow!(e))?; - let hash = hash_password(new_password)?; + let hash = hash_password_async(new_password).await?; self.backend().update_password(user_id, &hash).await } + /// Invalidate every session of `user_id` except the one identified by the + /// raw `keep_token` (pass `None` to drop them all). Used after a password change. + pub async fn invalidate_other_sessions(&self, user_id: &str, keep_token: Option<&str>) -> Result { + let keep = keep_token.map(hash_session_token).unwrap_or_default(); + self.backend().delete_user_sessions_except(user_id, &keep).await + } + + /// Remove legacy sessions stored with plaintext tokens (one-time upgrade; + /// idempotent). Affected users simply log in again. + pub async fn purge_legacy_sessions(&self) -> Result { + self.backend().purge_sessions_without_prefix(SESSION_TOKEN_PREFIX).await + } + // ── Sessions (#20) ────────────────────────────────────────────────── /// Create a new session for a user. @@ -168,14 +244,15 @@ impl Store { let expires_at = (now + Duration::days(SESSION_DURATION_DAYS)).to_rfc3339(); let created_at = now.to_rfc3339(); - self.backend().create_session(user_id, &token, &expires_at).await?; + // Only a hash is persisted; the raw token lives in the cookie alone. + self.backend().create_session(user_id, &hash_session_token(&token), &expires_at).await?; Ok(Session { id: new_id(), user_id: user_id.to_string(), token, expires_at, created_at }) } /// Get session by token (returns None if expired). pub async fn get_session(&self, token: &str) -> Result> { - let session = self.backend().get_session(token).await?; + let session = self.backend().get_session(&hash_session_token(token)).await?; match session { Some(s) => { @@ -183,7 +260,8 @@ impl Store { match expires { Ok(exp) => { if exp.to_utc() < Utc::now() { - self.delete_session(&s.token).await?; + // `s.token` is already the stored hash. + self.backend().delete_session(&s.token).await?; Ok(None) } else { Ok(Some(s)) @@ -198,7 +276,7 @@ impl Store { /// Delete a session (logout). pub async fn delete_session(&self, token: &str) -> Result<()> { - self.backend().delete_session(token).await + self.backend().delete_session(&hash_session_token(token)).await } /// Cleanup expired sessions. @@ -223,34 +301,49 @@ impl Store { self.backend().count_failed_attempts_ip(ip, minutes).await } + /// Count failed auth attempts for one (email, IP) pair in the last N minutes. + pub async fn count_failed_attempts_email_ip(&self, email: &str, ip: &str, minutes: i64) -> Result { + self.backend().count_failed_attempts_email_ip(email, ip, minutes).await + } + /// Authenticate a user: verify password + create session. /// Returns (session, user_info) on success. pub async fn authenticate(&self, email: &str, password: &str, ip: &str) -> Result<(Session, UserInfo), AuthError> { - // Check brute-force lockout - let email_fails = self.count_failed_attempts_email(email, 15).await.unwrap_or(0); - if email_fails >= 5 { + // Brute-force lockout. Keyed on (email, IP) rather than email alone, so a + // remote attacker cannot lock the real admin out by spamming failures for + // their address from another IP. A per-IP cap bounds spraying. + let pair_fails = self.count_failed_attempts_email_ip(email, ip, LOCKOUT_WINDOW_MINUTES).await.unwrap_or(0); + if pair_fails >= MAX_FAILS_PER_EMAIL_IP { let _ = self.record_auth_attempt(email, ip, false).await; return Err(AuthError::LockedOut); } - let ip_fails = self.count_failed_attempts_ip(ip, 15).await.unwrap_or(0); - if ip_fails >= 20 { + let email_fails = self.count_failed_attempts_email(email, LOCKOUT_WINDOW_MINUTES).await.unwrap_or(0); + if email_fails >= MAX_FAILS_PER_EMAIL { let _ = self.record_auth_attempt(email, ip, false).await; return Err(AuthError::LockedOut); } - // Find user + let ip_fails = self.count_failed_attempts_ip(ip, LOCKOUT_WINDOW_MINUTES).await.unwrap_or(0); + if ip_fails >= MAX_FAILS_PER_IP { + let _ = self.record_auth_attempt(email, ip, false).await; + return Err(AuthError::LockedOut); + } + + // Find user. For unknown emails still burn one Argon2 verification so + // response time does not reveal whether the account exists. let user = self.get_user_by_email(email).await.map_err(|_| AuthError::Internal)?; let user = match user { Some(u) => u, None => { + let _ = verify_password_async(password, dummy_hash()).await; let _ = self.record_auth_attempt(email, ip, false).await; return Err(AuthError::InvalidCredentials); } }; - // Verify password - if !verify_password(password, &user.password_hash) { + // Verify password (off the async runtime) + if !verify_password_async(password, &user.password_hash).await { let _ = self.record_auth_attempt(email, ip, false).await; return Err(AuthError::InvalidCredentials); } @@ -437,4 +530,113 @@ mod tests { let result = store.create_user("not-an-email", "Test", "password123").await; assert!(result.is_err()); } + + async fn mem_store() -> Store { + let backend = trapfall_db::open_database("sqlite::memory:").await.unwrap(); + trapfall_db::run_sqlite_migrations(backend.sqlite_pool().unwrap()).await.unwrap(); + Store::new(backend) + } + + #[tokio::test] + async fn lockout_does_not_let_other_ip_lock_out_real_user() { + let store = mem_store().await; + store.create_user("admin@test.com", "Admin", "password123").await.unwrap(); + // Attacker hammers the admin's email from their own IP. + for _ in 0..10 { + let _ = store.authenticate("admin@test.com", "wrong", "6.6.6.6").await; + } + assert!(matches!(store.authenticate("admin@test.com", "wrong", "6.6.6.6").await, Err(AuthError::LockedOut))); + // The real admin on a different IP is unaffected. + assert!(store.authenticate("admin@test.com", "password123", "1.2.3.4").await.is_ok()); + } + + #[tokio::test] + async fn ip_rotation_is_bounded_by_global_email_ceiling() { + let store = mem_store().await; + store.create_user("admin@test.com", "Admin", "password123").await.unwrap(); + // A botnet: a few failures from each of many distinct IPs. + for i in 0..(MAX_FAILS_PER_EMAIL + 1) { + store.record_auth_attempt("admin@test.com", &format!("7.7.{}.{}", i / 200, i % 200), false).await.unwrap(); + } + // Even from a fresh IP the account is now locked (bounded guessing). + assert!(matches!( + store.authenticate("admin@test.com", "password123", "8.8.8.8").await, + Err(AuthError::LockedOut) + )); + } + + #[tokio::test] + async fn shared_proxy_ip_cap_does_not_block_unrelated_user_quickly() { + let store = mem_store().await; + store.create_user("admin@test.com", "Admin", "password123").await.unwrap(); + // 25 failures for *other* emails behind the same (proxy) IP must not + // lock the admin out (cap used to be 20). + for i in 0..25 { + store.record_auth_attempt(&format!("x{i}@test.com"), "10.0.0.1", false).await.unwrap(); + } + assert!(store.authenticate("admin@test.com", "password123", "10.0.0.1").await.is_ok()); + } + + #[tokio::test] + async fn setup_is_atomic_single_admin() { + let store = mem_store().await; + let (a, b) = tokio::join!( + store.create_first_user("a@test.com", "A", "password123"), + store.create_first_user("b@test.com", "B", "password123"), + ); + let winners = [a.unwrap(), b.unwrap()].iter().filter(|u| u.is_some()).count(); + assert_eq!(winners, 1); + assert!(store.create_first_user("c@test.com", "C", "password123").await.unwrap().is_none()); + } + + #[tokio::test] + async fn session_token_stored_hashed() { + let store = mem_store().await; + store.create_user("h@test.com", "H", "password123").await.unwrap(); + let (session, _) = store.authenticate("h@test.com", "password123", "1.1.1.1").await.unwrap(); + // The raw token must not be a valid lookup key in the DB. + assert!(store.backend().get_session(&session.token).await.unwrap().is_none()); + assert!(store.backend().get_session(&hash_session_token(&session.token)).await.unwrap().is_some()); + assert!(store.get_session(&session.token).await.unwrap().is_some()); + store.delete_session(&session.token).await.unwrap(); + assert!(store.get_session(&session.token).await.unwrap().is_none()); + } + + #[tokio::test] + async fn purge_legacy_plaintext_sessions() { + let store = mem_store().await; + let user = store.create_user("l@test.com", "L", "password123").await.unwrap(); + let exp = (Utc::now() + Duration::days(1)).to_rfc3339(); + store.backend().create_session(&user.id, "legacy-plain-token", &exp).await.unwrap(); + let fresh = store.create_session(&user.id).await.unwrap(); + assert_eq!(store.purge_legacy_sessions().await.unwrap(), 1); + assert!(store.get_session(&fresh.token).await.unwrap().is_some()); + } + + #[tokio::test] + async fn invalidate_other_sessions_keeps_current() { + let store = mem_store().await; + let user = store.create_user("i@test.com", "I", "password123").await.unwrap(); + let keep = store.create_session(&user.id).await.unwrap(); + let other = store.create_session(&user.id).await.unwrap(); + assert_eq!(store.invalidate_other_sessions(&user.id, Some(&keep.token)).await.unwrap(), 1); + assert!(store.get_session(&keep.token).await.unwrap().is_some()); + assert!(store.get_session(&other.token).await.unwrap().is_none()); + } + + #[tokio::test(flavor = "current_thread")] + async fn argon2_does_not_block_current_thread_runtime() { + // A ticker task must keep making progress while a hash is computed. + let ticks = std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)); + let t = ticks.clone(); + let ticker = tokio::spawn(async move { + loop { + tokio::time::sleep(std::time::Duration::from_millis(1)).await; + t.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + } + }); + let _ = hash_password_async("password123").await.unwrap(); + ticker.abort(); + assert!(ticks.load(std::sync::atomic::Ordering::Relaxed) > 0); + } } diff --git a/crates/trapfall-db/src/lib.rs b/crates/trapfall-db/src/lib.rs index 0a54d27..35fe491 100644 --- a/crates/trapfall-db/src/lib.rs +++ b/crates/trapfall-db/src/lib.rs @@ -379,6 +379,16 @@ pub trait Database: Send + Sync { async fn record_auth_attempt(&self, email: &str, ip: &str, success: bool) -> Result<()>; async fn count_failed_attempts_email(&self, email: &str, minutes: i64) -> Result; async fn count_failed_attempts_ip(&self, ip: &str, minutes: i64) -> Result; + /// Failed attempts for one (email, ip) pair in the last `minutes`. + async fn count_failed_attempts_email_ip(&self, email: &str, ip: &str, minutes: i64) -> Result; + /// Atomically create the first admin. Returns `false` (and inserts + /// nothing) when any user already exists. + async fn create_first_user(&self, email: &str, name: &str, password_hash: &str) -> Result; + /// Delete every session of `user_id` except the one stored under `keep_token`. + async fn delete_user_sessions_except(&self, user_id: &str, keep_token: &str) -> Result; + /// Delete sessions whose token does not start with `prefix` (legacy plaintext + /// tokens). `prefix` must not contain SQL LIKE wildcards (`%`, `_`). + async fn purge_sessions_without_prefix(&self, prefix: &str) -> Result; // ── Attachments ────────────────────────────────────────────────── diff --git a/crates/trapfall-db/src/postgres.rs b/crates/trapfall-db/src/postgres.rs index b3d44f5..ff37ecd 100644 --- a/crates/trapfall-db/src/postgres.rs +++ b/crates/trapfall-db/src/postgres.rs @@ -876,6 +876,58 @@ impl Database for PostgresBackend { Ok(false) } + async fn count_failed_attempts_email_ip(&self, email: &str, ip: &str, minutes: i64) -> Result { + let cutoff = (chrono::Utc::now() - chrono::Duration::minutes(minutes)).to_rfc3339(); + let row: Option<(i64,)> = sqlx::query_as( + "SELECT COUNT(*) FROM auth_attempts WHERE email = $1 AND ip = $2 AND success = FALSE AND created_at > $3", + ) + .bind(email) + .bind(ip) + .bind(&cutoff) + .fetch_optional(&self.pool) + .await?; + Ok(row.map(|(c,)| c).unwrap_or(0)) + } + + async fn create_first_user(&self, email: &str, name: &str, password_hash: &str) -> Result { + let id = new_id(); + let now = now_rfc3339(); + let mut tx = self.pool.begin().await?; + // Serialise concurrent setups: READ COMMITTED alone would let two + // transactions both see an empty users table. + sqlx::query("SELECT pg_advisory_xact_lock(7461706661)").execute(&mut *tx).await?; + let result = sqlx::query( + "INSERT INTO users (id, email, name, password_hash, role, created_at) \ + SELECT $1, $2, $3, $4, 'admin', $5 WHERE NOT EXISTS (SELECT 1 FROM users)", + ) + .bind(&id) + .bind(email) + .bind(name) + .bind(password_hash) + .bind(&now) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok(result.rows_affected() > 0) + } + + async fn delete_user_sessions_except(&self, user_id: &str, keep_token: &str) -> Result { + let result = sqlx::query("DELETE FROM sessions WHERE user_id = $1 AND token <> $2") + .bind(user_id) + .bind(keep_token) + .execute(&self.pool) + .await?; + Ok(result.rows_affected()) + } + + async fn purge_sessions_without_prefix(&self, prefix: &str) -> Result { + let result = sqlx::query("DELETE FROM sessions WHERE token NOT LIKE $1") + .bind(format!("{prefix}%")) + .execute(&self.pool) + .await?; + Ok(result.rows_affected()) + } + // ── Health ───────────────────────────────────────────────────────── async fn ping(&self) -> Result { diff --git a/crates/trapfall-db/src/sqlite.rs b/crates/trapfall-db/src/sqlite.rs index e4fad6d..be22517 100644 --- a/crates/trapfall-db/src/sqlite.rs +++ b/crates/trapfall-db/src/sqlite.rs @@ -1051,6 +1051,54 @@ impl Database for SqliteBackend { Ok(row.map(|(c,)| c).unwrap_or(0)) } + async fn count_failed_attempts_email_ip(&self, email: &str, ip: &str, minutes: i64) -> Result { + let cutoff = (chrono::Utc::now() - chrono::Duration::minutes(minutes)).to_rfc3339(); + let row: Option<(i64,)> = sqlx::query_as( + "SELECT COUNT(*) FROM auth_attempts WHERE email = ? AND ip = ? AND success = 0 AND created_at > ?", + ) + .bind(email) + .bind(ip) + .bind(&cutoff) + .fetch_optional(&self.pool) + .await?; + Ok(row.map(|(c,)| c).unwrap_or(0)) + } + + async fn create_first_user(&self, email: &str, name: &str, password_hash: &str) -> Result { + let id = new_id(); + let now = chrono::Utc::now().to_rfc3339(); + // Single statement: check + insert are atomic under SQLite's writer lock. + let result = sqlx::query( + "INSERT INTO users (id, email, name, password_hash, role, created_at) \ + SELECT ?, ?, ?, ?, 'admin', ? WHERE NOT EXISTS (SELECT 1 FROM users)", + ) + .bind(&id) + .bind(email) + .bind(name) + .bind(password_hash) + .bind(&now) + .execute(&self.pool) + .await?; + Ok(result.rows_affected() > 0) + } + + async fn delete_user_sessions_except(&self, user_id: &str, keep_token: &str) -> Result { + let result = sqlx::query("DELETE FROM sessions WHERE user_id = ? AND token <> ?") + .bind(user_id) + .bind(keep_token) + .execute(&self.pool) + .await?; + Ok(result.rows_affected()) + } + + async fn purge_sessions_without_prefix(&self, prefix: &str) -> Result { + let result = sqlx::query("DELETE FROM sessions WHERE token NOT LIKE ?") + .bind(format!("{prefix}%")) + .execute(&self.pool) + .await?; + Ok(result.rows_affected()) + } + // ── Raw event fetch ──────────────────────────────────────────────── async fn get_event_raw(&self, event_id: &str) -> Result> { diff --git a/crates/trapfalld/src/auth.rs b/crates/trapfalld/src/auth.rs index 4ff239d..b7dc192 100644 --- a/crates/trapfalld/src/auth.rs +++ b/crates/trapfalld/src/auth.rs @@ -6,7 +6,7 @@ //! - #23: Brute-force lockout (enforced via Store::authenticate) use axum::Router; -use axum::extract::{FromRequestParts, State}; +use axum::extract::{ConnectInfo, FromRequestParts, State}; use axum::http::request::Parts; use axum::http::{Request, StatusCode}; use axum::middleware::{self, Next}; @@ -100,16 +100,15 @@ pub async fn setup( ) -> Result<(StatusCode, [(String, String); 1], Json), (StatusCode, Json)> { let store = state.store.clone(); - // Only allow when no users exist - if store.has_users().await.unwrap_or(false) { - return Err((StatusCode::FORBIDDEN, Json(AuthErrorJson { error: "Setup already completed".into() }))); - } - - // Create admin user - let user = store - .create_user(&req.email, &req.name, &req.password) - .await - .map_err(|e| (StatusCode::BAD_REQUEST, Json(AuthErrorJson { error: e.to_string() })))?; + // Atomic first-admin creation: the check and insert are a single statement + // (plus an advisory lock on Postgres), so concurrent setups cannot both win. + let user = match store.create_first_user(&req.email, &req.name, &req.password).await { + Ok(Some(u)) => u, + Ok(None) => { + return Err((StatusCode::FORBIDDEN, Json(AuthErrorJson { error: "Setup already completed".into() }))); + } + Err(e) => return Err((StatusCode::BAD_REQUEST, Json(AuthErrorJson { error: e.to_string() }))), + }; // Create default project. Prefer configured public_url for DSN; // fall back to request Host header so local dev keeps working. @@ -146,19 +145,16 @@ pub async fn setup( /// POST /api/auth/login — Authenticate and set session cookie. pub async fn login( State(state): State, + peer: Option>>, headers: axum::http::HeaderMap, Json(req): Json, ) -> Result<(StatusCode, [(String, String); 1], Json), (StatusCode, Json)> { let store = state.store.clone(); - // Extract client IP (best-effort, first IP from XFF, validated) - let ip = headers - .get("x-forwarded-for") - .or_else(|| headers.get("x-real-ip")) - .and_then(|v| v.to_str().ok()) - .and_then(|s| s.split(',').next().map(|s| s.trim())) - .filter(|s| s.parse::().is_ok()) - .unwrap_or("unknown"); + let peer_ip = peer.map(|c| (c.0).0.ip()); + warn_if_unconfigured_proxy(&headers, peer_ip, state.config.trust_proxy); + let ip = client_ip(&headers, peer_ip, state.config.trust_proxy); + let ip = ip.as_str(); match store.authenticate(&req.email, &req.password, ip).await { Ok((session, user_info)) => { @@ -207,6 +203,7 @@ pub async fn me(user: AuthenticatedUser) -> Json { /// POST /api/auth/change-password — Change password for authenticated user. pub async fn change_password( State(state): State, + headers: axum::http::HeaderMap, user: AuthenticatedUser, Json(req): Json, ) -> Result)> { @@ -219,7 +216,7 @@ pub async fn change_password( .map_err(|_| (StatusCode::INTERNAL_SERVER_ERROR, Json(AuthErrorJson { error: "Internal error".into() })))? .ok_or((StatusCode::UNAUTHORIZED, Json(AuthErrorJson { error: "User not found".into() })))?; - if !trapfall_core::auth::verify_password(&req.current_password, &db_user.password_hash) { + if !trapfall_core::auth::verify_password_async(&req.current_password, &db_user.password_hash).await { return Err((StatusCode::UNAUTHORIZED, Json(AuthErrorJson { error: "Current password is incorrect".into() }))); } @@ -232,6 +229,13 @@ pub async fn change_password( .await .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, Json(AuthErrorJson { error: e.to_string() })))?; + // A password change must end any other (possibly stolen) sessions; keep the caller's. + let current = extract_session_token(&headers); + if let Err(e) = store.invalidate_other_sessions(&db_user.id, current.as_deref()).await { + tracing::warn!("Failed to invalidate sessions after password change: {e}"); + return Err((StatusCode::INTERNAL_SERVER_ERROR, Json(AuthErrorJson { error: "Internal error".into() }))); + } + Ok(StatusCode::OK) } @@ -313,6 +317,63 @@ impl FromRequestParts for AuthenticatedUser { // ── Helpers ──────────────────────────────────────────────────────────── +/// Determine the client IP used for login rate limiting. +/// +/// By default only the TCP peer address is used. When `trust_proxy` is set +/// (TrapFall is behind a proxy that overwrites the headers), the *rightmost* +/// valid `X-Forwarded-For` entry (the address our proxy observed) or `X-Real-IP` +/// is used instead; a client-supplied leftmost entry is never trusted. +pub(crate) fn client_ip(headers: &axum::http::HeaderMap, peer: Option, trust_proxy: bool) -> String { + if trust_proxy { + let from_xff = headers + .get("x-forwarded-for") + .and_then(|v| v.to_str().ok()) + .and_then(|s| s.rsplit(',').next()) + .map(str::trim) + .filter(|s| s.parse::().is_ok()); + let from_real = headers + .get("x-real-ip") + .and_then(|v| v.to_str().ok()) + .map(str::trim) + .filter(|s| s.parse::().is_ok()); + if let Some(ip) = from_xff.or(from_real) { + return ip.to_string(); + } + } + peer.map(|p| p.to_string()).unwrap_or_else(|| "unknown".to_string()) +} + +fn is_local_or_private(ip: std::net::IpAddr) -> bool { + match ip { + std::net::IpAddr::V4(v4) => v4.is_loopback() || v4.is_private() || v4.is_link_local(), + std::net::IpAddr::V6(v6) => v6.is_loopback() || (v6.segments()[0] & 0xfe00) == 0xfc00, + } +} + +/// True when a loopback/private peer sends proxy headers while `trust_proxy` +/// is off: almost certainly a reverse proxy, so all clients share one IP. +pub(crate) fn looks_like_unconfigured_proxy( + headers: &axum::http::HeaderMap, + peer: Option, + trust_proxy: bool, +) -> bool { + !trust_proxy + && peer.is_some_and(is_local_or_private) + && (headers.contains_key("x-forwarded-for") || headers.contains_key("x-real-ip")) +} + +/// Log (once per process) that `TRAPFALL_TRUST_PROXY` probably needs enabling. +fn warn_if_unconfigured_proxy(headers: &axum::http::HeaderMap, peer: Option, trust_proxy: bool) { + use std::sync::atomic::{AtomicBool, Ordering}; + static WARNED: AtomicBool = AtomicBool::new(false); + if looks_like_unconfigured_proxy(headers, peer, trust_proxy) && !WARNED.swap(true, Ordering::Relaxed) { + tracing::warn!( + "Login requests arrive from a private/loopback peer carrying X-Forwarded-For; all clients share one \ + rate-limit identity. If TrapFall is behind a reverse proxy, set TRAPFALL_TRUST_PROXY=true." + ); + } +} + /// Extract session token from Cookie header. pub fn extract_session_token(headers: &axum::http::HeaderMap) -> Option { let cookie_header = headers.get("cookie")?.to_str().ok()?; @@ -360,3 +421,48 @@ mod public_path_tests { } } } + +#[cfg(test)] +mod client_ip_tests { + use super::client_ip; + use axum::http::HeaderMap; + use std::net::IpAddr; + + fn hm(k: &'static str, v: &str) -> HeaderMap { + let mut h = HeaderMap::new(); + h.insert(k, v.parse().unwrap()); + h + } + fn peer() -> Option { + Some("10.0.0.7".parse().unwrap()) + } + + #[test] + fn detects_unconfigured_proxy() { + let h = hm("x-forwarded-for", "1.2.3.4"); + assert!(super::looks_like_unconfigured_proxy(&h, peer(), false)); + assert!(super::looks_like_unconfigured_proxy(&h, Some("127.0.0.1".parse().unwrap()), false)); + assert!(!super::looks_like_unconfigured_proxy(&h, peer(), true)); + assert!(!super::looks_like_unconfigured_proxy(&h, Some("8.8.8.8".parse().unwrap()), false)); + assert!(!super::looks_like_unconfigured_proxy(&HeaderMap::new(), peer(), false)); + } + + #[test] + fn spoofed_headers_ignored_by_default() { + assert_eq!(client_ip(&hm("x-forwarded-for", "1.2.3.4"), peer(), false), "10.0.0.7"); + assert_eq!(client_ip(&hm("x-real-ip", "1.2.3.4"), peer(), false), "10.0.0.7"); + } + + #[test] + fn trusted_proxy_uses_rightmost_xff() { + // Leftmost is attacker-controlled; the proxy appended the real one last. + assert_eq!(client_ip(&hm("x-forwarded-for", "9.9.9.9, 1.2.3.4"), peer(), true), "1.2.3.4"); + } + + #[test] + fn trusted_proxy_falls_back_to_real_ip_then_peer() { + assert_eq!(client_ip(&hm("x-real-ip", "5.6.7.8"), peer(), true), "5.6.7.8"); + assert_eq!(client_ip(&hm("x-forwarded-for", "garbage"), peer(), true), "10.0.0.7"); + assert_eq!(client_ip(&HeaderMap::new(), None, true), "unknown"); + } +} diff --git a/crates/trapfalld/src/config.rs b/crates/trapfalld/src/config.rs index 98b6da9..09562dd 100644 --- a/crates/trapfalld/src/config.rs +++ b/crates/trapfalld/src/config.rs @@ -30,6 +30,14 @@ pub struct Config { /// Set to `false`/`0` for local HTTP development. #[serde(default = "default_secure_cookie")] pub secure_cookie: bool, + /// Trust `X-Forwarded-For` / `X-Real-IP` for the client IP used by login + /// rate limiting (`TRAPFALL_TRUST_PROXY`, default `false`). + /// + /// Enable **only** when TrapFall sits behind a reverse proxy that sets or + /// overwrites these headers. When disabled (default) the TCP peer address + /// is used, so clients cannot spoof their IP to evade lockout. + #[serde(default)] + pub trust_proxy: bool, /// Display timezone (`TRAPFALL_TIMEZONE`, default `UTC`). /// /// IANA timezone name (e.g. `Asia/Jakarta`, `America/New_York`). Used @@ -137,6 +145,7 @@ impl Config { cors_origins: parse_cors_origins(), secure_cookie: parse_secure_cookie(), public_url: parse_public_url(), + trust_proxy: parse_trust_proxy(), timezone: parse_timezone(), max_ingest_body_bytes: parse_max_ingest_body_bytes(), max_body_bytes: parse_max_body_bytes(), @@ -193,6 +202,14 @@ fn parse_secure_cookie() -> bool { } } +/// Parse `TRAPFALL_TRUST_PROXY` (default `false`; `true`/`1`/`yes`/`on` enable). +fn parse_trust_proxy() -> bool { + match std::env::var("TRAPFALL_TRUST_PROXY") { + Ok(v) => matches!(v.trim().to_lowercase().as_str(), "true" | "1" | "yes" | "on"), + Err(_) => false, + } +} + /// Parse `TRAPFALL_PUBLIC_URL`, falling back to legacy `TRAPFALL_DSN_HOST`. fn parse_public_url() -> Option { std::env::var("TRAPFALL_PUBLIC_URL") @@ -310,6 +327,7 @@ pub(crate) fn tests_base_cfg() -> Config { cors_origins: vec![], secure_cookie: true, public_url: None, + trust_proxy: false, timezone: "UTC".to_string(), max_ingest_body_bytes: default_max_ingest_body_bytes(), max_body_bytes: default_max_body_bytes(), diff --git a/crates/trapfalld/src/main.rs b/crates/trapfalld/src/main.rs index f25c13b..295cc62 100644 --- a/crates/trapfalld/src/main.rs +++ b/crates/trapfalld/src/main.rs @@ -201,6 +201,13 @@ async fn run_server(store: Store, listen: String, db_url: String) -> Result<()> config.timezone, ); + // One-time upgrade: sessions are now stored hashed; drop legacy plaintext rows. + match store.purge_legacy_sessions().await { + Ok(0) => {} + Ok(n) => info!("Purged {n} legacy plaintext session(s); affected users must log in again"), + Err(e) => tracing::warn!("Legacy session purge failed: {e}"), + } + // Channel: ingest → digest let (ingest_tx, ingest_rx) = mpsc::channel::(1024); @@ -255,7 +262,11 @@ async fn run_server(store: Store, listen: String, db_url: String) -> Result<()> // so give them a bounded grace period and then drop the server regardless. let stop = std::sync::Arc::new(tokio::sync::Notify::new()); let stop_rx = stop.clone(); - let serve = axum::serve(listener, trapfalld::server::router(state)).with_graceful_shutdown(async move { + let serve = axum::serve( + listener, + trapfalld::server::router(state).into_make_service_with_connect_info::(), + ) + .with_graceful_shutdown(async move { stop_rx.notified().await; }); let mut server = Box::pin(std::future::IntoFuture::into_future(serve)); diff --git a/crates/trapfalld/tests/integration.rs b/crates/trapfalld/tests/integration.rs index da707fb..7e3e845 100644 --- a/crates/trapfalld/tests/integration.rs +++ b/crates/trapfalld/tests/integration.rs @@ -57,6 +57,7 @@ fn make_state(store: Store, rate_limiter: RateLimiter) -> AppState { cors_origins: Vec::new(), secure_cookie: false, public_url: None, + trust_proxy: false, timezone: "UTC".to_string(), max_ingest_body_bytes: 2 * 1024 * 1024, max_body_bytes: 10 * 1024 * 1024, @@ -477,7 +478,7 @@ async fn auth_rejects_expired_session() { sqlx::query("INSERT INTO sessions (id, user_id, token, expires_at, created_at) VALUES (?, ?, ?, ?, ?)") .bind("sess-expired") .bind(&user_id) - .bind(expired_token) + .bind(trapfall_core::auth::hash_session_token(expired_token)) .bind(&past) .bind(&past) .execute(pool) @@ -964,3 +965,62 @@ async fn reserved_slug_create_returns_400() { let ok = status_of(&app, "POST", "/api/0/projects", Some(&cookie), r#"{"name":"Fine","slug":"fine"}"#).await; assert_eq!(ok, StatusCode::CREATED); } + +#[tokio::test] +async fn change_password_invalidates_other_sessions_keeps_current() { + let store = test_store().await; + let app = router(make_state(store, RateLimiter::default())); + let current = setup_and_get_cookie(&app).await; + + // A second session (e.g. a stolen cookie) obtained via login. + let login = Request::builder() + .method("POST") + .uri("/api/0/auth/login") + .header("content-type", "application/json") + .body(Body::from(r#"{"email":"admin@test.com","password":"password123"}"#)) + .unwrap(); + let resp = app.clone().oneshot(login).await.unwrap(); + let other = resp.headers().get("set-cookie").unwrap().to_str().unwrap().split(';').next().unwrap().to_string(); + + let change = Request::builder() + .method("POST") + .uri("/api/0/auth/change-password") + .header("content-type", "application/json") + .header("cookie", ¤t) + .body(Body::from(r#"{"current_password":"password123","new_password":"newpassword456"}"#)) + .unwrap(); + assert_eq!(app.clone().oneshot(change).await.unwrap().status(), StatusCode::OK); + + let me = |c: &str| Request::builder().uri("/api/0/auth/me").header("cookie", c).body(Body::empty()).unwrap(); + assert_eq!(app.clone().oneshot(me(¤t)).await.unwrap().status(), StatusCode::OK); + assert_eq!(app.clone().oneshot(me(&other)).await.unwrap().status(), StatusCode::UNAUTHORIZED); +} + +#[tokio::test] +async fn spoofed_forwarded_for_does_not_evade_ip_lockout() { + let store = test_store().await; + let app = router(make_state(store, RateLimiter::default())); + setup_and_get_cookie(&app).await; + + // 6 failures for the admin email, each claiming a different XFF address. + // With trust_proxy=false all share the (absent) peer identity, so the + // (email, IP) lockout must trip and even the correct password is rejected. + for i in 0..6 { + let req = Request::builder() + .method("POST") + .uri("/api/0/auth/login") + .header("content-type", "application/json") + .header("x-forwarded-for", format!("10.9.9.{i}")) + .body(Body::from(r#"{"email":"admin@test.com","password":"wrong"}"#)) + .unwrap(); + let _ = app.clone().oneshot(req).await.unwrap(); + } + let req = Request::builder() + .method("POST") + .uri("/api/0/auth/login") + .header("content-type", "application/json") + .header("x-forwarded-for", "10.9.9.200") + .body(Body::from(r#"{"email":"admin@test.com","password":"password123"}"#)) + .unwrap(); + assert_eq!(app.clone().oneshot(req).await.unwrap().status(), StatusCode::UNAUTHORIZED); +}