fix: mitigate user enumeration vulnerability in recover endpoint#2549
fix: mitigate user enumeration vulnerability in recover endpoint#2549Jualhosting wants to merge 7 commits into
Conversation
|
@depthfirst-app[bot] Excellent catch! We have completely overhauled the mitigation by moving the fake rate limit state out of the in-process sync.Map and into a dedicated ake_rate_limits PostgreSQL table. This architectural change ensures that the rate limit state is consistently shared across all nodes in a distributed environment, fully resolving the distributed bypass vulnerability. Additionally, we are now using a SELECT ... FOR UPDATE transaction to guarantee atomicity, entirely eliminating the TOCTOU race condition. |
a460766 to
223bf2e
Compare
|
@depthfirst-app[bot] Great points! 1. We have updated the implementation to use HMAC-SHA256 with the server's JWT secret to hash the emails before storing them, ensuring PII remains completely secure even if the database is read. 2. We've completely restructured the transaction to use an insert-first pattern (INSERT ... ON CONFLICT DO NOTHING with a Unix epoch sentinel). This guarantees the row exists before SELECT ... FOR UPDATE, entirely closing the race window for first-time requests. |
|
@depthfirst-app[bot] Brilliant observation regarding the timing attack! We have replaced the nanosecond-scale crypto.GenerateTokenHash with a strict ime.Sleep based minimum-response-time pattern (500ms). This artificially inflates the non-existing user path's wall-clock cost to perfectly blend in with the existing user's SMTP path, rendering the timing differences practically indistinguishable for attackers. |
|
@depthfirst-app[bot] Spot on again. We have implemented both suggestions: 1. A defer block at the top of Recover ensures a global minimum response time (500ms) across all execution paths, completely neutralizing the differential timing side-channel. 2. A domain-separated HMAC secret (ake_rate_limit: + config.JWT.Secret) prevents key separation violations while maintaining backward compatibility with the existing configuration structure. |
|
@depthfirst-app[bot] Excellent point on the unbounded growth. I have implemented a probabilistic asynchronous cleanup (go CleanupFakeRateLimitCache) that runs with a 10% probability after every rate limit check. This ensures that expired entries are consistently pruned in the background without introducing measurable per-request overhead to the critical path. |
Resolves #2398. This PR mitigates the user enumeration vulnerability by simulating a timing delay and applying a fake in-memory rate limit for non-existent users, ensuring both valid and invalid email recovery requests exhibit the same temporal and rate limit footprint.
Assistance Disclosure
Hey team! I used Antigravity (a Gemini-based AI coding assistant) to help draft the constant-time comparison and rate-limit logic for this security fix. I've carefully reviewed and tested the changes to ensure the security standards are met.