View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001341 | Main CAcert Website | my account | public | 2014-11-19 20:46 | 2015-06-23 20:09 |
| Reporter | BenBE | Assigned To | BenBE | ||
| Priority | immediate | Severity | block | Reproducibility | always |
| Status | solved? | Resolution | fixed | ||
| Product Version | 2012 Q1 | ||||
| Target Version | 2014 Q4 | Fixed in Version | 2015 Q1 | ||
| Summary | 0001341: Rate limit for login attempts | ||||
| Description | Limit the number of login attempts per time interval that are allowed. | ||||
| Steps To Reproduce | Automate login brute force. -> No limit on tries per interval is present, thus any speed is fine. | ||||
| Additional Information | about one attempt every 5 seconds should be fine. | ||||
| Tags | No tags attached. | ||||
| Reviewed by | Ted, BenBE | ||||
| Test Instructions | Check that successive logins are limited to a low rate (like 1 login every 5 seconds) | ||||
|
|
Before the patch was applied, I tried to enter multiple wrong passwords to an account, fast after each other. I got no error message but that the PW were wrong and had no issue to log in with the correct PW, afterwards. After the patch was applied, I tried the same and got the message: "You hit the login rate limit of 1 login per 5 seconds." if I did this too fast. I could log in with the correct PW after that delay. -> ok Further observations: - I tried this with two accounts from the same computer at the same time, the limit did only count for one account at the same time. - [correct or wrong] PWs entered during the 5 seconds did not re-set the delay. Maybe there is room for improvement, but this patch seems to cover the issue of fast automated PW-attacks, while not hindering humans to enter real PWs (as those have to be typed in, anyway, to begin with) => OK |
|
|
I received a the confirmation of review of this patch by Ted: --- ich habe mir den Patch mal angeschaut. Ich habe ein bischen ein komisches Gefühl damit alle Accounts für eine Mail-Adresse zu behandeln (also auch die gelöschten, gesperrten und unverifizierten), aber es sollte eigentlich in Ordnung gehen. Hast Du diese Restriktionen gezielt rausgelassen? Ansonsten noch zwei Kleinigkeiten: * Im Projekt scheint NOW() üblicher zu sein als CURRENT_TIMESTAMP. Und obwohl letzteres eigentlich ANSI-Standard ist kenne ich mehr DBMS die NOW() unterstützen als CURRENT_TIMESTAMP... * Wäre es nicht präziser die neue DB-Spalte "lastFailedLogin" zu benennen? Sind beides keine kritischen Punkte. Auch ohne Änderungen hat der Patch meinen Segen. MfG Ted --- I've taken a look at the patch. I have a somewhat strange feeling to handle email addresses for all accounts (even deleted or blocked ones), but it should be OK. Have those restrictions been left out intentionally? Apart from this two more minor issues: * In the project it seems to more common to use NOW() instead of CURRENT_TIMESTAMP. And even as the latter actually is in the ANSI standard I know more DBMS that support NOW() instead of CURRENT_TIMESTAMP. * Wouldn't it be more precise to call the new column "lastFailedLogin"? Both aren't critical issues. Even without the changes the patch is fine with me. Regards, Ted --- BenBE: CARS for the translation. Regarding the two points mentioned in the review by Ted: 1. The choice of CURRENT_TIMESTAMP in this case was done by the patch author (felixd). No particular preference exists for either one. 2. When the original DB migration script was written the final semantics had yet to be discussed in the team. No update of the script (or patch) based on that was done in order to rename the column afterwards to avoid name mixups. From my side the patch is OK too. |
|
|
I try to login with out password => message "Falsche E-Mail-Adresse und/oder falsches Kennwort." => ok If I just retry id in a short timespan => message "You hit the login rate limit of 1 login per 5 seconds." =>ok If I wait some time and I try to login in again with wrong credentials I get again message "You hit the login rate limit of 1 login per 5 seconds." => false |
|
|
After fix has been applied: Login with false credentials => message "Login failed due to incorrect email address, wrong passphrase or account rate limit of one login per 5 seconds was hit." => ok Directly new attempt => same message => ok Waiting for some time and new attempt => same message => ok => ok |
|
|
The privacy issue mentioned by INOPIAE in ^5359 refers to different behaviour if an account exists or not. The mitigation now uses the same wording regardless of hitting the rate limit or using wrong credentials. This might be somewhat confusing (long error message with several reasons), but it avoids the information leak of primary email addresses. Review for modified patch -> OK Ted has been notified of this change for review/confirmation. |
|
|
Tested wrong username and got a generic error message => OK Tested right username/pw and got logged in => OK Tested right usernane/ wrong pw and got a generic error messgae => OK Tested right username/ wrong pw and right username/pw after that within 5 secounds and got a generic error message => OK => PASSED |
|
|
Reviewed changes to avoid leaking infos (commit 0e14ede2f690be0df938ef2e98b974f60882612f), changes are acceptable. Review is PASSED. |
|
|
The fix has been installed on the production server on March 13, 2015. During the installation of the fix, the database layout has been migrated to version 6 using the script contained in this fix. See also: https://lists.cacert.org/wws/arc/cacert-systemlog/2015-03/msg00000.html |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2014-11-19 20:46 | BenBE | New Issue | |
| 2014-11-19 20:46 | BenBE | Assigned To | => felixd |
| 2014-12-02 23:56 | BenBE | Status | new => fix available |
| 2015-01-13 20:25 | reinhardm | Assigned To | felixd => reinhardm |
| 2015-01-13 20:28 | reinhardm | Assigned To | reinhardm => felixd |
| 2015-03-11 21:18 | Eva | Note Added: 0005357 | |
| 2015-03-11 21:24 | BenBE | Reviewed by | => Ted, BenBE |
| 2015-03-11 21:24 | BenBE | Note Added: 0005358 | |
| 2015-03-11 21:24 | BenBE | Assigned To | felixd => INOPIAE |
| 2015-03-11 21:24 | BenBE | Priority | urgent => immediate |
| 2015-03-11 21:24 | BenBE | Severity | major => block |
| 2015-03-11 21:24 | BenBE | Status | fix available => needs review & testing |
| 2015-03-11 21:24 | BenBE | Status | needs review & testing => needs testing |
| 2015-03-11 21:25 | BenBE | Note Edited: 0005358 | |
| 2015-03-11 22:10 | INOPIAE | Note Added: 0005359 | |
| 2015-03-11 22:11 | INOPIAE | Note Edited: 0005359 | |
| 2015-03-11 22:14 | INOPIAE | Note Edited: 0005359 | |
| 2015-03-11 22:21 | INOPIAE | Note Edited: 0005359 | |
| 2015-03-11 22:34 | INOPIAE | Note Added: 0005360 | |
| 2015-03-11 22:54 | BenBE | Note Added: 0005361 | |
| 2015-03-11 22:54 | BenBE | Reviewed by | Ted, BenBE => BenBE |
| 2015-03-11 22:54 | BenBE | Status | needs testing => needs review |
| 2015-03-12 05:33 | INOPIAE | Note Edited: 0005360 | |
| 2015-03-12 11:19 | janmaco | Note Added: 0005362 | |
| 2015-03-12 22:07 | Ted | Note Added: 0005363 | |
| 2015-03-12 22:08 | Ted | Reviewed by | BenBE => Ted, BenBE |
| 2015-03-12 22:18 | BenBE | Assigned To | INOPIAE => BenBE |
| 2015-03-12 22:18 | BenBE | Status | needs review => ready to deploy |
| 2015-03-13 09:06 | wytze | Note Added: 0005364 | |
| 2015-03-13 09:06 | wytze | Status | ready to deploy => solved? |
| 2015-03-13 09:06 | wytze | Fixed in Version | => 2015 Q1 |
| 2015-03-13 09:06 | wytze | Resolution | open => fixed |
| 2015-04-01 11:51 | BenBE | View Status | private => public |
| 2015-06-23 20:09 | BenBE | Relationship added | related to 0001339 |
