CAcert Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0001341Main CAcert Websitemy accountpublic2014-11-19 20:462015-06-23 20:09
Assigned ToBenBE 
PlatformOSOS Version
Product Version2012 Q1 
Target Version2014 Q4Fixed in Version2015 Q1 
Summary0001341: Rate limit for login attempts
DescriptionLimit the number of login attempts per time interval that are allowed.
Steps To ReproduceAutomate login brute force. -> No limit on tries per interval is present, thus any speed is fine.
Additional Informationabout one attempt every 5 seconds should be fine.
TagsNo tags attached.
Reviewed byTed, BenBE
Test InstructionsCheck that successive logins are limited to a low rate (like 1 login every 5 seconds)
Attached Files

- Relationships
related to 0001339solved?BenBE Account Pwnage using OTP hash 

-  Notes
Eva (updater)
2015-03-11 21:18

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
BenBE (updater)
2015-03-11 21:24
edited on: 2015-03-11 21:25

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.

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
 * 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.

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.

INOPIAE (updater)
2015-03-11 22:10
edited on: 2015-03-11 22:21

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

INOPIAE (updater)
2015-03-11 22:34
edited on: 2015-03-12 05:33

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

BenBE (updater)
2015-03-11 22:54

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.
janmaco (updater)
2015-03-12 11:19

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

Ted (administrator)
2015-03-12 22:07

Reviewed changes to avoid leaking infos (commit 0e14ede2f690be0df938ef2e98b974f60882612f), changes are acceptable.

Review is PASSED.
wytze (developer)
2015-03-13 09:06

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: [^]

- Issue History
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 View Revisions
2015-03-11 22:10 INOPIAE Note Added: 0005359
2015-03-11 22:11 INOPIAE Note Edited: 0005359 View Revisions
2015-03-11 22:14 INOPIAE Note Edited: 0005359 View Revisions
2015-03-11 22:21 INOPIAE Note Edited: 0005359 View Revisions
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 View Revisions
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

Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker