New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lock down accounts by IP after N failed attemps at logging in #13472
Conversation
Haven't tested, but wondering if using "Webcron" (calling the archiver with token_auth in url) will still work if the according account got locked down. |
Just tested it, but it works as it is only an API test. But realizing now that the UI is sending API requests so there is pretty much no way to keep the UI working for a logged in user when the IP got blocked by a different user. Will need to block the entire UI as it current behaviour it would only block partial UI and look broken. |
"CurrentlyBlockedIPs": "Currently blocked IPs", | ||
"IPsAlwaysBlocked": "These IPs are always blocked", | ||
"UnblockAllIPs": "Unblock all currently blocked IPs", | ||
"CurrentlyBlockedIPsUnblockInfo": "You can unblock IPs that are currently blocked so they can log in again in case they were falsely flagged and need to be able to log in again.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
think the last part could be changed to just "in case they were incorrectly flagged"
expect.screenshot("bruteforcelog_blockedapi").to.be.capture(function (page) { | ||
page.load(apiAuthUrl); | ||
}, done); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one could be tested in a system test w/ Http::sendRequest()
, in case you don't want to keep the screenshot around.
Reviewed code, haven't tested locally though |
@tsteur In case this isn't already included (I didn't check), I think it would be worth it to extend this feature to the password reset feature. Currently an attacker can try out unlimited password resets, which will send unlimited emails to the Matomo user and if they accidently click on the link in any email, the attacker has immediate full access to the instance (which is another security issue in itself, that maybe should be fixed in one of the next releases (#11071)) |
Not sure @Findus23 I can add a simple block to password reset page if user is already blocked because of too many failed logins. I reckon in case you locked yourself out it may be good to reach the password reset page though. Once the lock is expired, you can simply log in with the new password... Maybe create a new issue for this, but don't think it has too much of a priority if we should also count the number of resets etc. In theory, just one reset is enough though to possibly trick a user, or one reset every hour... or something. I don't think it adds much to security and rather #11071 can be looked at to be improved eventually. |
@tsteur I think the main reason why we should limit the password resets is that attackers can create a lot of chaos by requesting hundreds of password resets per minute an spamming the users mailbox, overloading the mailserver or even damaging the spam ranking. |
Can you create a new issue for this? |
@diosmosis fyi fixed the tests |
As discussed in #2888 a quite simple implementation that should do the job. Heaps more could be done as mentioned in the comments but might not be needed.
It took me couple hours to understand all the authentication/login flows as there are lots of different components now such as Auth, SessionAuth, SessionInitializer, Access, etc. and it wasn't clear where to put things to make it work universally while still being clear about when it happens etc.
Also had to add couple of events eg for failed logins. The most important part is really to register a failed login attempt when there is one (we often authenticate eg API requests etc even though the user didn't, or we always authenticate first against session then regular auth adapter etc which makes things complicated). And it is important to check whether IP is blocked when trying to attempt to log in. All those parts should be covered.
When an IP is blocked, I did not want to block any already logged in users in the UI. This could be annoying when a company shares the same IP and it would otherwise even block the super user.
The superuser has a diagnostic page to unblock all users and a console command. Can also document in a FAQ how to do it through the SQL database directly.
For system settings and diagnostics page screens see screenshot tests.
Functionality wise you specify two values: X amount of login retries allowed in Y amount of minutes. If you have more than X amount of logins within Y amount of minutes, you are blocked and cannot log in. Even when successful token provided for example. You can only log in again, when some time has passed and you had less than X amount of login retries within the last Y amount of minutes. There is no specific lock out time that you can configure so if people lock themselves out they can use UI again after eg 30 min or 60min. By default we allow 20 logins within 60 minutes. When sharing an IP within an office those 20 logins could be reached within an hour but should be maybe fine. Hard to find the right balance here.
You can also whitelist and blacklist specific IPs.
fixes #2888
I'll try to write some more tests for the different login failures once the PR is reviewed.