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 std::sync::Arc;
|
||||||
|
|
||||||
use axum::extract::State;
|
use axum::extract::State;
|
||||||
|
use axum::http::HeaderMap;
|
||||||
use axum::routing::post;
|
use axum::routing::post;
|
||||||
use axum::{Json, Router};
|
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
|
/// Extract a coarse client identity from the request headers (IP via
|
||||||
/// X-Forwarded-For fallback). Used as the rate-limit key.
|
/// 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
|
headers
|
||||||
.get("x-forwarded-for")
|
.get("x-forwarded-for")
|
||||||
.and_then(|v| v.to_str().ok())
|
.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.
|
/// 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
|
!state
|
||||||
.auth_rate_limiter
|
.auth_rate_limiter
|
||||||
.check_rate_limit(
|
.check_rate_limit(
|
||||||
@@ -79,7 +80,7 @@ fn rate_limited(state: &ApiState, headers: &axum::http::HeaderMap) -> bool {
|
|||||||
#[tracing::instrument(skip(state))]
|
#[tracing::instrument(skip(state))]
|
||||||
pub async fn login_handler(
|
pub async fn login_handler(
|
||||||
State(state): State<Arc<ApiState>>,
|
State(state): State<Arc<ApiState>>,
|
||||||
headers: axum::http::HeaderMap,
|
headers: HeaderMap,
|
||||||
Json(req): Json<LoginRequest>,
|
Json(req): Json<LoginRequest>,
|
||||||
) -> Result<Json<AuthResponse>, ApiError> {
|
) -> Result<Json<AuthResponse>, ApiError> {
|
||||||
// Rate-limit login attempts (brute-force protection).
|
// Rate-limit login attempts (brute-force protection).
|
||||||
@@ -156,7 +157,7 @@ pub async fn login_handler(
|
|||||||
#[tracing::instrument(skip(state))]
|
#[tracing::instrument(skip(state))]
|
||||||
pub async fn register_handler(
|
pub async fn register_handler(
|
||||||
State(state): State<Arc<ApiState>>,
|
State(state): State<Arc<ApiState>>,
|
||||||
headers: axum::http::HeaderMap,
|
headers: HeaderMap,
|
||||||
Json(req): Json<RegisterRequest>,
|
Json(req): Json<RegisterRequest>,
|
||||||
) -> Result<Json<AuthResponse>, ApiError> {
|
) -> Result<Json<AuthResponse>, ApiError> {
|
||||||
// Rate-limit registration (abuse protection).
|
// 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
|
// Load existing users
|
||||||
let users_path = state.store_base_dir.join("users.json");
|
let users_path = state.store_base_dir.join("users.json");
|
||||||
let mut users: std::collections::HashMap<String, String> = if users_path.exists() {
|
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
|
// Persist
|
||||||
users.insert(req.username.clone(), hash);
|
users.insert(req.username.clone(), hash);
|
||||||
let content = serde_json::to_string_pretty(&users)
|
let content = serde_json::to_string_pretty(&users)
|
||||||
@@ -239,7 +248,7 @@ pub async fn register_handler(
|
|||||||
#[tracing::instrument(skip(state))]
|
#[tracing::instrument(skip(state))]
|
||||||
pub async fn refresh_handler(
|
pub async fn refresh_handler(
|
||||||
State(state): State<Arc<ApiState>>,
|
State(state): State<Arc<ApiState>>,
|
||||||
headers: axum::http::HeaderMap,
|
headers: HeaderMap,
|
||||||
Json(req): Json<RefreshRequest>,
|
Json(req): Json<RefreshRequest>,
|
||||||
) -> Result<Json<AuthResponse>, ApiError> {
|
) -> Result<Json<AuthResponse>, ApiError> {
|
||||||
// Rate-limit refresh attempts.
|
// Rate-limit refresh attempts.
|
||||||
|
|||||||
@@ -23,6 +23,7 @@
|
|||||||
use std::fmt;
|
use std::fmt;
|
||||||
use std::future::Future;
|
use std::future::Future;
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
use std::sync::Arc;
|
||||||
|
|
||||||
use zesdex_application::ports::{PasswordService, TokenService};
|
use zesdex_application::ports::{PasswordService, TokenService};
|
||||||
|
|
||||||
@@ -208,6 +209,10 @@ pub struct ApiState {
|
|||||||
/// Shared sliding-window limiter for auth endpoints (login/register/refresh).
|
/// Shared sliding-window limiter for auth endpoints (login/register/refresh).
|
||||||
pub auth_rate_limiter: zesdex_infrastructure::middleware::rate_limit::RateLimiter,
|
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.
|
/// LLM provider client for chat completions.
|
||||||
pub llm_client: zesdex_infrastructure::llm::provider::LlmClient,
|
pub llm_client: zesdex_infrastructure::llm::provider::LlmClient,
|
||||||
}
|
}
|
||||||
@@ -288,6 +293,7 @@ impl ApiState {
|
|||||||
|
|
||||||
let token_service = JwtTokenService::new(&jwt_secret);
|
let token_service = JwtTokenService::new(&jwt_secret);
|
||||||
let auth_rate_limiter = zesdex_infrastructure::middleware::rate_limit::RateLimiter::new();
|
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(
|
let llm_client = zesdex_infrastructure::llm::provider::LlmClient::new(
|
||||||
llm_api_key.into(),
|
llm_api_key.into(),
|
||||||
llm_model.into(),
|
llm_model.into(),
|
||||||
@@ -304,6 +310,7 @@ impl ApiState {
|
|||||||
password_service: Argon2PasswordService,
|
password_service: Argon2PasswordService,
|
||||||
token_service,
|
token_service,
|
||||||
auth_rate_limiter,
|
auth_rate_limiter,
|
||||||
|
users_lock,
|
||||||
llm_client,
|
llm_client,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user