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:
asepharyana
2026-08-27 22:09:22 +07:00
parent 6db00b2266
commit 884b19ccb5
2 changed files with 28 additions and 12 deletions
+21 -12
View File
@@ -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.
+7
View File
@@ -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,
} }
} }