basquetWi + New ticket
movilba MOVILBA-740

Throttle de login por IP sola: una direccion compartida bloquea a todos sus usuarios

Done low rmresearcher-mba-l-cc

El throttle de login por IP cuenta intentos fallidos por ip_address SOLA, sin el email, asi que una direccion compartida bloquea a todos sus usuarios juntos. rate-limiter.ts:60-83 cuenta login_attempts fallidos por ipAddress y devuelve ip_throttled al llegar a GLOBAL_RATE_LIMIT_IP_MAX_ATTEMPTS, que global-settings.ts:50-54 fija en 10 en una ventana de 15 minutos. MEDIDO EN PROD, read-only, 2026-10-08: login_attempts tiene 808 intentos desde 2026-04-01, 270 ip_address distintas, 0 NULL - es decir XFF[0] si varia por llamador y no colapsa en el hop interno. El maximo real de fallidos en cualquier ventana rodante de 15 minutos, en toda la historia, es 11, de 100.66.2.251 el 2026-05-18, contra un umbral de 10: el bloqueo se alcanzo una vez. Direcciones genuinamente compartidas hay una: 10.236.5.3 con 9 emails distintos, 54 intentos y 10 fallidos; la segunda es 10.78.163.58 con 3 emails y 2 fallidos. NO ES UNA REGRESION DE v5.6.199. Mida los dos lados: antes del deploy ya habia 270 ips distintas, asi que el cambio de rate-limiter.ts:24 a leer x-client-ip primero no colapso los buckets. El efecto real del arreglo de 735 es un APRIETE: los llamadores con XFF malformado ahora caen en una direccion validada o en unknown en vez de en lo que viniera. El riesgo de lockeo compartido es PREEXISTENTE y raro, no nuevo. DECISION PARA ELAZAR, tres caminos y ninguno es obvio: (A) keyear por email+IP, que preserva la defensa contra password spraying de un email y pierde la defensa contra barrido de muchos emails desde una IP; (B) confiar en un conteo de hops configurado y tomar el XFF correspondiente, que es exacto pero pide saber cuantos proxies hay delante y GCABA no se consulta; (C) sacar la dimension IP y dejar solo el throttle por email, que es el mas simple y el que menos defensa deja. Audit no elige.

Sub-tickets

No sub-tickets.
+ Add sub-ticket

Activity

  • audit-mba-l-cc created · 1w ago
  • coder03-mba-l-cc assigned · 1w ago
    researcher-mba-l-cc / solo
  • coder03-mba-l-cc statusChanged · 1w ago
    coder03-mba-l-cc 19:2xZ - assigned to the researcher lane. The DB side is already measured (login_attempts: 805 attempts / 270 distinct ip_address since 2026-04-01; 10.236.5.3 genuinely shared, 9 emails, 54 attempts, 10 failures; max 11 failures in any rolling 15-minute window in the whole history, against a threshold of 10). What is MISSING is the code surface: which call paths key a bucket on IP alone, where the threshold is read from, and what the blast radius is per path. That is a read-only code census, no DB write.
  • researcher-mba-l-cc note · 1w ago
    2026-10-08T16:18:43-03:00 / 19:18:43Z researcher-mba-l-cc. Code census for 740 - every call path keying a bucket on IP alone, read-only, no DB re-measurement (reusing coder03's 805/270/10.236.5.3/11-vs-10 figures as given). FOUR independent IP-only buckets found, three different storage mechanisms, no shared abstraction: 1. LOGIN (credentials provider only) - src/lib/auth.ts:168 calls checkRateLimit(email, clientIp) -> src/lib/rate-limiter.ts:65-83 `ip_throttled` branch: SELECT count(*) FROM login_attempts WHERE ip_address=X AND success=false AND created_at>=now-window, across ALL emails on that IP. Threshold GLOBAL_RATE_LIMIT_IP_MAX_ATTEMPTS=10, window GLOBAL_RATE_LIMIT_WINDOW_MINUTES=15 (global-settings.ts:51,54). DB-backed (login_attempts table) - persists across restarts, shared across all POD replicas, unlike the other three. clientIp comes from extractClientIp() (rate-limiter.ts:23-30): x-client-ip || x-forwarded-for[0] || x-real-ip, caller-supplied headers, no isIP validation, no proxy-level stripping at this layer (contrast with proxy.ts:148 which does validate via node:net isIP for its own ipAddress var - these are two different extraction paths, not shared code). OIDC/Keycloak login does NOT go through authorize() and is NOT subject to this bucket at all - matches the 4-Keycloak/15-credentials split already observed in DEV traffic. 2. PASSWORD RESET - src/app/api/auth/reset-password/route.ts:73-74 reuses the SAME checkRateLimit() / same login_attempts-backed bucket as #1. Does not create a second blast radius, but inherits #1's: an IP already ip_throttled from failed logins also gets 429 on reset requests. Route's own comment (lines 69-72) only addresses NOT writing new rows (avoiding a reset-triggered lockout), it does not address that the READ side still throttles by raw shared IP. 3. /api/health - src/proxy.ts:159-162, key `health:${ipAddress||"unknown"}`, in-process Map (src/lib/rate-limit.ts:40,55-91), threshold 60/min (HEALTH_RATE_LIMIT, rate-limit.ts:119 - hardcoded here, NOT in global-settings.ts, inconsistent with pattern #1/#4). ipAddress here IS validated via node:net isIP (proxy.ts:144-148) before use - this path is NOT exposed to the same spoofable-header risk as #1. Per-replica counter, resets on every deploy/restart (file's own header comment, rate-limit.ts:5-9). A caller with no valid IP shares one global "unknown" bucket with every other no-IP caller. 4. /api/log-error - src/app/api/log-error/route.ts:25-39,56-59, OWN third implementation (separate in-process Map literal in the route file itself, not rate-limit.ts's buckets, not rate-limiter.ts's DB table), key = raw extractClientIp() result (same un-validated extraction as #1, falls back to literal string 'unknown' on no IP). Threshold GLOBAL_LOG_ERROR_RATE_MAX=20/min, window GLOBAL_LOG_ERROR_RATE_WINDOW_MS=60000 (global-settings.ts:59-60). Per-replica, resets on deploy. CONFIRMED NOT IN SCOPE (checked, not just assumed): - src/lib/parque-api.ts:101 and src/lib/rendiciones-api.ts:235 call rateLimit() keyed on `principal.keyId` (the resolved API key), never IP - and both 401 before ever reaching that call if the key is missing/invalid, so no bucket exists for an unauthenticated caller on either path. - src/app/api/v1/exports/vehiculos/route.ts:34 calls extractClientIp() but the file's own comment (lines 29-32) states it is for attribution/correlation only - grep confirms no rateLimit/checkRateLimit call anywhere in that file. THRESHOLD SOURCE, per path: #1/#2 and #4 live in global-settings.ts (GLOBAL_RATE_LIMIT_* / GLOBAL_LOG_ERROR_RATE_*); #3's HEALTH_RATE_LIMIT is a literal const inside rate-limit.ts itself, not centralized with the others. NOT COVERED by this census (read-only code mapping only): no re-measurement of the DB-side blast radius (coder03's 805/270/10.236.5.3/11-vs-10 figures stand as given, not re-derived here); whether NextAuth's own internals apply any additional IP throttling on the OIDC/Keycloak path (not in this repo, not inspected); any edge/ingress-level limiting at GCABA's reverse proxy or dev01/prod nginx (explicitly out of scope per rate-limit.ts's own header comment); whether x-forwarded-for/x-client-ip can be spoofed past whatever sits in front of each environment (already a named trust limit elsewhere, not re-tested here). No fix proposed - per assignment, that decision is Elazar's.
  • researcher-mba-l-cc completed · 1w ago
    2026-10-08T18:44:47-03:00 / 21:44:47Z researcher-mba-l-cc. Census already posted this session (19:19:15Z note) - status had stayed todo despite the work being complete. Closing now, no new measurement. Shape: 4 independent IP-only buckets, 3 storage mechanisms, no shared abstraction - (1) login via rate-limiter.ts, DB-backed login_attempts, threshold 10/15min from global-settings.ts, unvalidated header extraction; (2) password-reset reuses bucket 1 fully; (3) /api/health, in-process Map, validated IP, hardcoded threshold 60/min, resets on restart; (4) /api/log-error, its own separate in-process Map, unvalidated extraction, 20/min from global-settings.ts, resets on restart. Confirmed out of scope: parque-api/rendiciones-api key on API-key not IP; exports/vehiculos extracts IP for attribution only, no rate-limit call. No fix proposed - the A/B/C decision is Elazar's per the item's own assignment.
bug
1w ago by audit-mba-l-cc
1w ago
2026-10-08 21:44