From b55b7c34f87ac3a9a341d47bb1b502369780b448 Mon Sep 17 00:00:00 2001 From: goose Date: Fri, 3 Jul 2026 21:57:39 -0300 Subject: [PATCH] feat: rate limiting + E2E crypto lifecycle test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rate limiting (closes the last security gap #3): - New RateLimiter: in-memory fixed-window IP-based limiter (std::sync::Mutex HashMap, no new deps). Configurable via RATE_LIMIT_MAX (default 100) + RATE_LIMIT_WINDOW_SECS (default 60) env vars. - general_rate_limit_middleware now reads ClientIp from request extensions and rejects with 429 + Retry-After header when over the limit. Wired via from_fn_with_state in app.rs. Lived on AppState as Arc. - Deleted the dead auth_rate_limit_middleware (never wired). - 3 unit tests (allows up to N, independent IPs, window reset). E2E crypto lifecycle test: - Full zero-knowledge round-trip against jsdom's real Web Crypto: setup → encrypt → verify ciphertext → unlock with password → decrypt → recover via phrase → rewrap under new password → decrypt. Plus wrong-password/wrong-phrase failures and cross-user key isolation. - Fixed wrapDek/unwrapDek: base64-encode raw DEK bytes (was using TextDecoder which produced non-base64), and make unwrapped DEK extractable (needed for rewrapDek to export). Verified: backend 24 tests 0 warnings; frontend 24 tests, build clean. --- backend/src/app.rs | 7 +- backend/src/config/mod.rs | 18 ++++ backend/src/main.rs | 7 ++ backend/src/middleware/rate_limit.rs | 48 ++++++--- backend/src/security/mod.rs | 2 + backend/src/security/rate_limiter.rs | 86 +++++++++++++++ backend/tests/common/mod.rs | 4 + .../src/crypto/crypto.e2e.test.ts | 101 ++++++++++++++++++ web/normogen-web/src/crypto/keys.ts | 12 +-- 9 files changed, 262 insertions(+), 23 deletions(-) create mode 100644 backend/src/security/rate_limiter.rs create mode 100644 web/normogen-web/src/crypto/crypto.e2e.test.ts diff --git a/backend/src/app.rs b/backend/src/app.rs index 63b86a5..0644947 100644 --- a/backend/src/app.rs +++ b/backend/src/app.rs @@ -92,7 +92,7 @@ pub fn build_app(state: AppState) -> Router { public_routes .merge(protected_routes) - .with_state(state) + .with_state(state.clone()) .layer( ServiceBuilder::new() // Resolve the client IP once and stash it for handlers/middleware @@ -104,8 +104,9 @@ pub fn build_app(state: AppState) -> Router { .layer(axum::middleware::from_fn( middleware::security_headers_middleware, )) - // General rate limiting (currently a stub) - .layer(axum::middleware::from_fn( + // IP-based rate limiting (reads ClientIp from extensions). + .layer(axum::middleware::from_fn_with_state( + state.clone(), middleware::general_rate_limit_middleware, )) .layer(TraceLayer::new_for_http()) diff --git a/backend/src/config/mod.rs b/backend/src/config/mod.rs index edb6869..ead49a7 100644 --- a/backend/src/config/mod.rs +++ b/backend/src/config/mod.rs @@ -51,6 +51,9 @@ pub struct AppState { /// P0: persisted (hashed) refresh tokens, used for rotation, revocation, /// and surviving process restarts. pub refresh_token_repo: Option>, + + /// IP-based rate limiter (in-memory fixed-window). + pub rate_limiter: Arc, } #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -61,6 +64,13 @@ pub struct Config { pub encryption: EncryptionConfig, pub cors: CorsConfig, pub environment: Environment, + pub rate_limit: RateLimitConfig, +} + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +pub struct RateLimitConfig { + pub max_requests: u32, + pub window_secs: u64, } #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] @@ -174,6 +184,14 @@ impl Config { .collect(), }, environment, + rate_limit: RateLimitConfig { + max_requests: std::env::var("RATE_LIMIT_MAX") + .unwrap_or_else(|_| "100".to_string()) + .parse()?, + window_secs: std::env::var("RATE_LIMIT_WINDOW_SECS") + .unwrap_or_else(|_| "60".to_string()) + .parse()?, + }, }) } } diff --git a/backend/src/main.rs b/backend/src/main.rs index d3e35c5..fd59437 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -111,6 +111,12 @@ async fn main() -> anyhow::Result<()> { let interaction_service = Arc::new(services::InteractionService::new()); eprintln!("Interaction service initialized (Phase 2.8)"); + // IP-based rate limiter (in-memory fixed-window). + let rate_limiter = Arc::new(security::RateLimiter::new( + config.rate_limit.max_requests, + std::time::Duration::from_secs(config.rate_limit.window_secs), + )); + // Create application state let state = config::AppState { db, @@ -124,6 +130,7 @@ async fn main() -> anyhow::Result<()> { interaction_service: Some(interaction_service), token_version_cache, refresh_token_repo: Some(refresh_token_repo), + rate_limiter, }; eprintln!("Building router with security middleware..."); diff --git a/backend/src/middleware/rate_limit.rs b/backend/src/middleware/rate_limit.rs index b10f95e..0f676fa 100644 --- a/backend/src/middleware/rate_limit.rs +++ b/backend/src/middleware/rate_limit.rs @@ -1,20 +1,42 @@ -use axum::{extract::Request, http::StatusCode, middleware::Next, response::Response}; +use axum::{ + extract::{Request, State}, + http::StatusCode, + middleware::Next, + response::{IntoResponse, Response}, + Json, +}; -/// Middleware for general rate limiting -/// NOTE: Currently a stub implementation. TODO: Implement IP-based rate limiting +use crate::{config::AppState, middleware::ClientIp}; + +/// IP-based rate limiting. Reads the resolved client IP from request extensions +/// (populated by `client_ip_middleware`). Returns 429 Too Many Requests if the +/// IP has exceeded the configured limit within the window. pub async fn general_rate_limit_middleware( + State(state): State, req: Request, next: Next, -) -> Result { - // TODO: Implement proper rate limiting with IP-based tracking - // For now, just pass through - Ok(next.run(req).await) -} +) -> Result { + let ip = req + .extensions() + .get::() + .map(|c| c.as_str().to_string()) + .unwrap_or_else(|| "0.0.0.0".to_string()); + + if !state.rate_limiter.check(&ip) { + let retry_after = state.rate_limiter.window_secs(); + return Err(( + StatusCode::TOO_MANY_REQUESTS, + [ + ("Retry-After", retry_after.to_string()), + ("Content-Type", "application/json".to_string()), + ], + Json(serde_json::json!({ + "error": "rate limit exceeded", + "retry_after": retry_after, + })), + ) + .into_response()); + } -/// Middleware for auth endpoint rate limiting -/// NOTE: Currently a stub implementation. TODO: Implement IP-based rate limiting -pub async fn auth_rate_limit_middleware(req: Request, next: Next) -> Result { - // TODO: Implement proper rate limiting with IP-based tracking - // For now, just pass through Ok(next.run(req).await) } diff --git a/backend/src/security/mod.rs b/backend/src/security/mod.rs index e682707..01178e5 100644 --- a/backend/src/security/mod.rs +++ b/backend/src/security/mod.rs @@ -1,7 +1,9 @@ pub mod account_lockout; pub mod audit_logger; +pub mod rate_limiter; pub mod session_manager; pub use account_lockout::AccountLockout; pub use audit_logger::AuditLogger; +pub use rate_limiter::RateLimiter; pub use session_manager::SessionManager; diff --git a/backend/src/security/rate_limiter.rs b/backend/src/security/rate_limiter.rs new file mode 100644 index 0000000..cf670a7 --- /dev/null +++ b/backend/src/security/rate_limiter.rs @@ -0,0 +1,86 @@ +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; + +/// A fixed-window IP rate limiter. Thread-safe via a standard mutex (the +/// critical section is nanoseconds — increment + compare). +#[derive(Clone)] +pub struct RateLimiter { + inner: Arc>>, + max_requests: u32, + window: Duration, +} + +#[derive(Clone, Copy)] +struct WindowState { + count: u32, + started_at: Instant, +} + +impl RateLimiter { + pub fn new(max_requests: u32, window: Duration) -> Self { + Self { + inner: Arc::new(Mutex::new(HashMap::new())), + max_requests, + window, + } + } + + /// Check whether the given IP is allowed to make a request. Increments the + /// counter. Returns `true` if allowed, `false` if over the limit. + pub fn check(&self, ip: &str) -> bool { + let mut map = self.inner.lock().expect("rate limiter mutex poisoned"); + let now = Instant::now(); + + let state = map.entry(ip.to_string()).or_insert(WindowState { + count: 0, + started_at: now, + }); + + // Reset the window if it has elapsed. + if now.duration_since(state.started_at) >= self.window { + state.count = 0; + state.started_at = now; + } + + state.count += 1; + state.count <= self.max_requests + } + + /// The configured window duration (for the `Retry-After` response header). + pub fn window_secs(&self) -> u64 { + self.window.as_secs() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn allows_up_to_limit_then_denies() { + let limiter = RateLimiter::new(3, Duration::from_secs(60)); + assert!(limiter.check("1.2.3.4")); + assert!(limiter.check("1.2.3.4")); + assert!(limiter.check("1.2.3.4")); + assert!(!limiter.check("1.2.3.4")); // 4th denied + } + + #[test] + fn different_ips_are_independent() { + let limiter = RateLimiter::new(2, Duration::from_secs(60)); + assert!(limiter.check("1.1.1.1")); + assert!(limiter.check("1.1.1.1")); + assert!(!limiter.check("1.1.1.1")); // 1.1.1.1 exhausted + assert!(limiter.check("2.2.2.2")); // different IP still fine + } + + #[test] + fn window_reset_allows_again() { + let limiter = RateLimiter::new(1, Duration::from_millis(10)); + assert!(limiter.check("9.9.9.9")); + assert!(!limiter.check("9.9.9.9")); // over limit + std::thread::sleep(Duration::from_millis(15)); + assert!(limiter.check("9.9.9.9")); // window elapsed → allowed + } +} diff --git a/backend/tests/common/mod.rs b/backend/tests/common/mod.rs index 6370c75..d66556b 100644 --- a/backend/tests/common/mod.rs +++ b/backend/tests/common/mod.rs @@ -142,6 +142,10 @@ pub async fn app_for_test() -> Option<(Router, String)> { interaction_service: Some(interaction_service), token_version_cache, refresh_token_repo: Some(refresh_token_repo), + rate_limiter: std::sync::Arc::new(normogen_backend::security::RateLimiter::new( + 1000, + std::time::Duration::from_secs(60), + )), }; Some((build_app(state), db_name)) diff --git a/web/normogen-web/src/crypto/crypto.e2e.test.ts b/web/normogen-web/src/crypto/crypto.e2e.test.ts new file mode 100644 index 0000000..b160193 --- /dev/null +++ b/web/normogen-web/src/crypto/crypto.e2e.test.ts @@ -0,0 +1,101 @@ +import { describe, it, expect } from 'vitest'; +import { + setupEncryption, + unlockWithPassword, + unlockWithRecovery, + rewrapDek, + encryptJson, + decryptJson, + setEncKey, + clearEncKey, + hasEncKey, +} from './index'; + +describe('zero-knowledge crypto lifecycle', () => { + const password = 'my-secret-password'; + const recoveryPhrase = 'my-recovery-phrase'; + const sampleData = { name: 'Aspirin', dosage: '100mg', frequency: 'daily' }; + + it('full lifecycle: setup → encrypt → unlock → decrypt → recover → rewrap', async () => { + // 1. Setup (register): generate DEK, wrap under password + recovery. + const setup = await setupEncryption(password, recoveryPhrase); + expect(setup.dek).toBeDefined(); + expect(setup.passwordWrappedDek.data).not.toBe(''); + expect(setup.recoveryWrappedDek).toBeDefined(); + expect(setup.recoveryKekHash).toBeDefined(); + + // 2. Encrypt data with the DEK. + const encrypted = await encryptJson(sampleData, setup.dek); + // Verify it's ciphertext, not plaintext. + expect(encrypted.data).not.toContain('Aspirin'); + expect(encrypted.iv).not.toBe(''); + + // 3. Decrypt with the same DEK — round-trip. + const decrypted = await decryptJson(encrypted, setup.dek); + expect(decrypted.name).toBe('Aspirin'); + expect(decrypted.dosage).toBe('100mg'); + + // 4. Simulate page reload: DEK is lost. + clearEncKey(); + expect(hasEncKey()).toBe(false); + + // 5. Unlock with password (the unlock screen flow). + const unlockResult = await unlockWithPassword(password, setup.passwordWrappedDek); + setEncKey(unlockResult.dek); + expect(hasEncKey()).toBe(true); + + // 6. The unlocked DEK can decrypt the same data. + const decryptedAfterUnlock = await decryptJson( + encrypted, + unlockResult.dek, + ); + expect(decryptedAfterUnlock.name).toBe('Aspirin'); + + // 7. Recover via recovery phrase (forgot password flow). + clearEncKey(); + const recoveredDek = await unlockWithRecovery( + recoveryPhrase, + setup.recoveryWrappedDek!, + ); + const decryptedAfterRecovery = await decryptJson( + encrypted, + recoveredDek, + ); + expect(decryptedAfterRecovery.name).toBe('Aspirin'); + + // 8. Rewrap under a new password (post-recovery). + const newPassword = 'my-new-password'; + const rewrapped = await rewrapDek(recoveredDek, newPassword); + const unlockWithNew = await unlockWithPassword(newPassword, rewrapped); + const decryptedAfterRewrap = await decryptJson( + encrypted, + unlockWithNew.dek, + ); + expect(decryptedAfterRewrap.name).toBe('Aspirin'); + + clearEncKey(); + }); + + it('wrong password fails to unlock', async () => { + const setup = await setupEncryption(password); + await expect( + unlockWithPassword('wrong-password', setup.passwordWrappedDek), + ).rejects.toThrow(); + }); + + it('wrong recovery phrase fails to unlock', async () => { + const setup = await setupEncryption(password, recoveryPhrase); + await expect( + unlockWithRecovery('wrong-phrase', setup.recoveryWrappedDek!), + ).rejects.toThrow(); + }); + + it('data encrypted under one key cannot be decrypted by another', async () => { + const setupA = await setupEncryption('user-a-password'); + const setupB = await setupEncryption('user-b-password'); + const encrypted = await encryptJson(sampleData, setupA.dek); + + // User B's key cannot decrypt User A's data. + await expect(decryptJson(encrypted, setupB.dek)).rejects.toThrow(); + }); +}); diff --git a/web/normogen-web/src/crypto/keys.ts b/web/normogen-web/src/crypto/keys.ts index 4f2391d..25577a5 100644 --- a/web/normogen-web/src/crypto/keys.ts +++ b/web/normogen-web/src/crypto/keys.ts @@ -83,16 +83,14 @@ export async function generateDek(): Promise { ); } -/** Export a DEK to raw bytes, encrypt (wrap) it under a KEK, return base64. */ +/** Export a DEK to raw bytes, base64-encode, then encrypt (wrap) under a KEK. */ export async function wrapDek(dek: CryptoKey, kek: CryptoKey): Promise { const rawDek = await crypto.subtle.exportKey('raw', dek); - return encrypt( - new TextDecoder().decode(new Uint8Array(rawDek)), - kek, - ); + // Base64-encode the raw bytes so they survive the encrypt/decrypt string cycle. + return encrypt(base64(new Uint8Array(rawDek)), kek); } -/** Decrypt (unwrap) a wrapped DEK and import as a non-extractable AES-GCM key. */ +/** Decrypt (unwrap) a wrapped DEK and import as an AES-GCM key. */ export async function unwrapDek(payload: CipherPayload, kek: CryptoKey): Promise { const rawDekB64 = await decrypt(payload, kek); // The wrapped DEK was encrypted as a base64 string of the raw key bytes. @@ -101,7 +99,7 @@ export async function unwrapDek(payload: CipherPayload, kek: CryptoKey): Promise 'raw', rawDek as BufferSource, { name: 'AES-GCM' }, - false, // non-extractable in the session (can't be re-exported) + true, // extractable so rewrapDek can export it for re-wrapping ['encrypt', 'decrypt'], ); }