fix(api): cegah race condition pada register users.json (TOCTOU)
- Tambah users_lock (Mutex) di ApiState untuk serialisasi read-modify-write users.json pada endpoint register; lock hanya dipegang selama operasi file sinkron (tidak pernah lintas .await, menjaga future tetap Send) - Hash password dihitung sebelum lock sehingga request concurrent tidak saling blokir selama hashing Argon2
This commit is contained in:
@@ -15,6 +15,7 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use axum::extract::State;
|
||||
use axum::http::HeaderMap;
|
||||
use axum::routing::post;
|
||||
use axum::{Json, Router};
|
||||
|
||||
@@ -39,7 +40,7 @@ pub fn router() -> Router<Arc<ApiState>> {
|
||||
|
||||
/// Extract a coarse client identity from the request headers (IP via
|
||||
/// X-Forwarded-For fallback). Used as the rate-limit key.
|
||||
fn client_id(headers: &axum::http::HeaderMap) -> String {
|
||||
fn client_id(headers: &HeaderMap) -> String {
|
||||
headers
|
||||
.get("x-forwarded-for")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
@@ -50,7 +51,7 @@ fn client_id(headers: &axum::http::HeaderMap) -> String {
|
||||
}
|
||||
|
||||
/// Enforce the auth rate limit, returning `true` if the request is allowed.
|
||||
fn rate_limited(state: &ApiState, headers: &axum::http::HeaderMap) -> bool {
|
||||
fn rate_limited(state: &ApiState, headers: &HeaderMap) -> bool {
|
||||
!state
|
||||
.auth_rate_limiter
|
||||
.check_rate_limit(
|
||||
@@ -79,7 +80,7 @@ fn rate_limited(state: &ApiState, headers: &axum::http::HeaderMap) -> bool {
|
||||
#[tracing::instrument(skip(state))]
|
||||
pub async fn login_handler(
|
||||
State(state): State<Arc<ApiState>>,
|
||||
headers: axum::http::HeaderMap,
|
||||
headers: HeaderMap,
|
||||
Json(req): Json<LoginRequest>,
|
||||
) -> Result<Json<AuthResponse>, ApiError> {
|
||||
// Rate-limit login attempts (brute-force protection).
|
||||
@@ -156,7 +157,7 @@ pub async fn login_handler(
|
||||
#[tracing::instrument(skip(state))]
|
||||
pub async fn register_handler(
|
||||
State(state): State<Arc<ApiState>>,
|
||||
headers: axum::http::HeaderMap,
|
||||
headers: HeaderMap,
|
||||
Json(req): Json<RegisterRequest>,
|
||||
) -> Result<Json<AuthResponse>, ApiError> {
|
||||
// Rate-limit registration (abuse protection).
|
||||
@@ -176,6 +177,21 @@ pub async fn register_handler(
|
||||
));
|
||||
}
|
||||
|
||||
// Compute the password hash first (async, no lock held).
|
||||
let hash = state
|
||||
.password_service
|
||||
.hash(&req.password)
|
||||
.await
|
||||
.map_err(|e| ApiError::Internal(format!("Password hashing failed: {e}")))?;
|
||||
|
||||
// Serialize read-modify-write of users.json to avoid losing concurrent
|
||||
// registers (TOCTOU race). The lock is held only across the sync
|
||||
// file operations — never across an `.await` (keeps the future `Send`).
|
||||
let _users_guard = state
|
||||
.users_lock
|
||||
.lock()
|
||||
.map_err(|_| ApiError::Internal("users lock poisoned".into()))?;
|
||||
|
||||
// Load existing users
|
||||
let users_path = state.store_base_dir.join("users.json");
|
||||
let mut users: std::collections::HashMap<String, String> = if users_path.exists() {
|
||||
@@ -194,13 +210,6 @@ pub async fn register_handler(
|
||||
));
|
||||
}
|
||||
|
||||
// Hash the password
|
||||
let hash = state
|
||||
.password_service
|
||||
.hash(&req.password)
|
||||
.await
|
||||
.map_err(|e| ApiError::Internal(format!("Password hashing failed: {e}")))?;
|
||||
|
||||
// Persist
|
||||
users.insert(req.username.clone(), hash);
|
||||
let content = serde_json::to_string_pretty(&users)
|
||||
@@ -239,7 +248,7 @@ pub async fn register_handler(
|
||||
#[tracing::instrument(skip(state))]
|
||||
pub async fn refresh_handler(
|
||||
State(state): State<Arc<ApiState>>,
|
||||
headers: axum::http::HeaderMap,
|
||||
headers: HeaderMap,
|
||||
Json(req): Json<RefreshRequest>,
|
||||
) -> Result<Json<AuthResponse>, ApiError> {
|
||||
// Rate-limit refresh attempts.
|
||||
|
||||
@@ -23,6 +23,7 @@
|
||||
use std::fmt;
|
||||
use std::future::Future;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
use zesdex_application::ports::{PasswordService, TokenService};
|
||||
|
||||
@@ -208,6 +209,10 @@ pub struct ApiState {
|
||||
/// Shared sliding-window limiter for auth endpoints (login/register/refresh).
|
||||
pub auth_rate_limiter: zesdex_infrastructure::middleware::rate_limit::RateLimiter,
|
||||
|
||||
/// Serializes read-modify-write of `users.json` so concurrent register
|
||||
/// requests cannot lose writes (TOCTOU race).
|
||||
pub users_lock: Arc<std::sync::Mutex<()>>,
|
||||
|
||||
/// LLM provider client for chat completions.
|
||||
pub llm_client: zesdex_infrastructure::llm::provider::LlmClient,
|
||||
}
|
||||
@@ -288,6 +293,7 @@ impl ApiState {
|
||||
|
||||
let token_service = JwtTokenService::new(&jwt_secret);
|
||||
let auth_rate_limiter = zesdex_infrastructure::middleware::rate_limit::RateLimiter::new();
|
||||
let users_lock = Arc::new(std::sync::Mutex::new(()));
|
||||
let llm_client = zesdex_infrastructure::llm::provider::LlmClient::new(
|
||||
llm_api_key.into(),
|
||||
llm_model.into(),
|
||||
@@ -304,6 +310,7 @@ impl ApiState {
|
||||
password_service: Argon2PasswordService,
|
||||
token_service,
|
||||
auth_rate_limiter,
|
||||
users_lock,
|
||||
llm_client,
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user