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 <noreply@anthropic.com>
This commit is contained in:
@@ -74,6 +74,20 @@ struct UserRow {
|
|||||||
/// bcrypt work factor. Cost 12 ≈ 300 ms on modern hardware — balances security against registration latency.
|
/// bcrypt work factor. Cost 12 ≈ 300 ms on modern hardware — balances security against registration latency.
|
||||||
pub const BCRYPT_COST: u32 = 12;
|
pub const BCRYPT_COST: u32 = 12;
|
||||||
|
|
||||||
|
async fn hash_password(password: String) -> Result<String, AppError> {
|
||||||
|
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<bool, AppError> {
|
||||||
|
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
|
// Token generation helpers
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -191,7 +205,7 @@ pub async fn register(
|
|||||||
}
|
}
|
||||||
|
|
||||||
let user_id = Uuid::new_v4().to_string();
|
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();
|
let now = Utc::now().to_rfc3339();
|
||||||
|
|
||||||
sqlx::query!(
|
sqlx::query!(
|
||||||
@@ -236,7 +250,7 @@ pub async fn login(
|
|||||||
.password_hash
|
.password_hash
|
||||||
.ok_or_else(|| AppError::Internal("password hash missing".into()))?;
|
.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 {
|
if !valid {
|
||||||
tracing::warn!(username = %username, "login: invalid password");
|
tracing::warn!(username = %username, "login: invalid password");
|
||||||
return Err(AppError::InvalidCredentials);
|
return Err(AppError::InvalidCredentials);
|
||||||
@@ -378,23 +392,28 @@ pub async fn upload_avatar(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Write to avatars/ directory, replacing any previous file for this user.
|
// 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 filename = format!("{}.{}", user.user_id, ext);
|
||||||
let path = std::path::Path::new("avatars").join(&filename);
|
let path = std::path::Path::new("avatars").join(&filename);
|
||||||
let tmp_path = std::path::Path::new("avatars").join(format!("{}.{}.tmp", user.user_id, ext));
|
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
|
// Write to a temp file then atomically rename so concurrent readers never
|
||||||
// see a partially-written avatar.
|
// see a partially-written avatar.
|
||||||
std::fs::write(&tmp_path, &body).map_err(|e| AppError::Internal(e.to_string()))?;
|
tokio::fs::write(&tmp_path, &body)
|
||||||
if let Err(e) = std::fs::rename(&tmp_path, &path) {
|
.await
|
||||||
let _ = std::fs::remove_file(&tmp_path);
|
.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()));
|
return Err(AppError::Internal(e.to_string()));
|
||||||
}
|
}
|
||||||
// Remove stale files with other extensions after the atomic rename.
|
// Remove stale files with other extensions after the atomic rename.
|
||||||
for old_ext in &["jpg", "png", "webp", "gif"] {
|
for old_ext in &["jpg", "png", "webp", "gif"] {
|
||||||
if *old_ext != ext {
|
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)),
|
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 =
|
let user_id =
|
||||||
user_id.ok_or_else(|| AppError::NotFound(format!("user '{username}' not found")))?;
|
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!(
|
sqlx::query!(
|
||||||
"UPDATE users SET password_hash = ? WHERE id = ?",
|
"UPDATE users SET password_hash = ? WHERE id = ?",
|
||||||
|
|||||||
@@ -39,6 +39,18 @@ use std::{
|
|||||||
str::FromStr,
|
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]
|
#[tokio::main]
|
||||||
async fn main() {
|
async fn main() {
|
||||||
// Load .env file if present (silently ignored when absent).
|
// 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.
|
/// optionally `SERVER_PORT`) in the environment.
|
||||||
async fn run_server() {
|
async fn run_server() {
|
||||||
let db_url = std::env::var("DATABASE_URL").expect("DATABASE_URL must be set");
|
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");
|
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")
|
let port: u16 = std::env::var("SERVER_PORT")
|
||||||
.unwrap_or_else(|_| "8080".into())
|
.unwrap_or_else(|_| "8080".into())
|
||||||
.parse()
|
.parse()
|
||||||
@@ -137,3 +154,30 @@ async fn run_server() {
|
|||||||
|
|
||||||
axum::serve(listener, app).await.expect("server error");
|
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());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user