From 7dbf34c1636378d12362079f475b05b051c92d64 Mon Sep 17 00:00:00 2001 From: funman300 Date: Mon, 8 Jun 2026 11:05:45 -0700 Subject: [PATCH] fix(server): move bcrypt to spawn_blocking, async file I/O, validate JWT_SECRET Three independent hardening changes: 1. bcrypt on a blocking thread: hash() and verify() are CPU-bound (~300 ms at cost 12). Running them directly on an async task starved the Tokio runtime under concurrent load. Wrapped in spawn_blocking. 2. Async avatar file I/O: std::fs::write/rename/remove_file in an async handler blocks the executor. Replaced with tokio::fs equivalents. 3. JWT_SECRET minimum length: a secret shorter than 32 bytes is fatally weak. validate_jwt_secret() now rejects it at startup with a clear message rather than silently accepting it. Co-Authored-By: Claude Sonnet 4.6 --- solitaire_server/src/auth.rs | 37 ++++++++++++++++++++++------- solitaire_server/src/main.rs | 46 +++++++++++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 10 deletions(-) diff --git a/solitaire_server/src/auth.rs b/solitaire_server/src/auth.rs index 058890c..3816882 100644 --- a/solitaire_server/src/auth.rs +++ b/solitaire_server/src/auth.rs @@ -74,6 +74,20 @@ struct UserRow { /// bcrypt work factor. Cost 12 ≈ 300 ms on modern hardware — balances security against registration latency. pub const BCRYPT_COST: u32 = 12; +async fn hash_password(password: String) -> Result { + tokio::task::spawn_blocking(move || hash(password, BCRYPT_COST)) + .await + .map_err(|e| AppError::Internal(format!("password hash task failed: {e}")))? + .map_err(AppError::from) +} + +async fn verify_password(password: String, password_hash: String) -> Result { + tokio::task::spawn_blocking(move || verify(password, &password_hash)) + .await + .map_err(|e| AppError::Internal(format!("password verify task failed: {e}")))? + .map_err(AppError::from) +} + // --------------------------------------------------------------------------- // Token generation helpers // --------------------------------------------------------------------------- @@ -191,7 +205,7 @@ pub async fn register( } let user_id = Uuid::new_v4().to_string(); - let password_hash = hash(&body.password, BCRYPT_COST)?; + let password_hash = hash_password(body.password).await?; let now = Utc::now().to_rfc3339(); sqlx::query!( @@ -236,7 +250,7 @@ pub async fn login( .password_hash .ok_or_else(|| AppError::Internal("password hash missing".into()))?; - let valid = verify(&body.password, &row_hash)?; + let valid = verify_password(body.password, row_hash).await?; if !valid { tracing::warn!(username = %username, "login: invalid password"); return Err(AppError::InvalidCredentials); @@ -378,23 +392,28 @@ pub async fn upload_avatar( } // Write to avatars/ directory, replacing any previous file for this user. - std::fs::create_dir_all("avatars").map_err(|e| AppError::Internal(e.to_string()))?; + tokio::fs::create_dir_all("avatars") + .await + .map_err(|e| AppError::Internal(e.to_string()))?; let filename = format!("{}.{}", user.user_id, ext); let path = std::path::Path::new("avatars").join(&filename); let tmp_path = std::path::Path::new("avatars").join(format!("{}.{}.tmp", user.user_id, ext)); // Write to a temp file then atomically rename so concurrent readers never // see a partially-written avatar. - std::fs::write(&tmp_path, &body).map_err(|e| AppError::Internal(e.to_string()))?; - if let Err(e) = std::fs::rename(&tmp_path, &path) { - let _ = std::fs::remove_file(&tmp_path); + tokio::fs::write(&tmp_path, &body) + .await + .map_err(|e| AppError::Internal(e.to_string()))?; + if let Err(e) = tokio::fs::rename(&tmp_path, &path).await { + let _ = tokio::fs::remove_file(&tmp_path).await; return Err(AppError::Internal(e.to_string())); } // Remove stale files with other extensions after the atomic rename. for old_ext in &["jpg", "png", "webp", "gif"] { if *old_ext != ext { - let _ = std::fs::remove_file( + let _ = tokio::fs::remove_file( std::path::Path::new("avatars").join(format!("{}.{}", user.user_id, old_ext)), - ); + ) + .await; } } @@ -454,7 +473,7 @@ pub async fn reset_password( let user_id = user_id.ok_or_else(|| AppError::NotFound(format!("user '{username}' not found")))?; - let new_hash = hash(new_password, BCRYPT_COST)?; + let new_hash = hash_password(new_password.to_string()).await?; sqlx::query!( "UPDATE users SET password_hash = ? WHERE id = ?", diff --git a/solitaire_server/src/main.rs b/solitaire_server/src/main.rs index c6bee49..1425770 100644 --- a/solitaire_server/src/main.rs +++ b/solitaire_server/src/main.rs @@ -39,6 +39,18 @@ use std::{ str::FromStr, }; +const JWT_SECRET_MIN_BYTES: usize = 32; + +fn validate_jwt_secret(secret: &str) -> Result<(), String> { + if secret.len() < JWT_SECRET_MIN_BYTES { + Err(format!( + "JWT_SECRET must be at least {JWT_SECRET_MIN_BYTES} bytes; generate a high-entropy production secret" + )) + } else { + Ok(()) + } +} + #[tokio::main] async fn main() { // Load .env file if present (silently ignored when absent). @@ -103,8 +115,13 @@ async fn run_reset_password(username: &str) { /// optionally `SERVER_PORT`) in the environment. async fn run_server() { let db_url = std::env::var("DATABASE_URL").expect("DATABASE_URL must be set"); - // Load JWT_SECRET once at startup — a missing secret is a fatal configuration error. + // Load JWT_SECRET once at startup — a missing or weak secret is a fatal + // configuration error rather than a per-request failure. let jwt_secret = std::env::var("JWT_SECRET").expect("JWT_SECRET must be set"); + if let Err(msg) = validate_jwt_secret(&jwt_secret) { + eprintln!("{msg}"); + std::process::exit(1); + } let port: u16 = std::env::var("SERVER_PORT") .unwrap_or_else(|_| "8080".into()) .parse() @@ -137,3 +154,30 @@ async fn run_server() { axum::serve(listener, app).await.expect("server error"); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn jwt_secret_rejects_short_secret() { + let secret = "x".repeat(JWT_SECRET_MIN_BYTES - 1); + let err = validate_jwt_secret(&secret).expect_err("short secret must be rejected"); + assert!( + err.contains("JWT_SECRET must be at least"), + "error should explain the minimum length, got: {err}" + ); + } + + #[test] + fn jwt_secret_accepts_exact_minimum_length() { + let secret = "x".repeat(JWT_SECRET_MIN_BYTES); + assert!(validate_jwt_secret(&secret).is_ok()); + } + + #[test] + fn jwt_secret_accepts_longer_secret() { + let secret = "x".repeat(JWT_SECRET_MIN_BYTES + 16); + assert!(validate_jwt_secret(&secret).is_ok()); + } +}