search

data-govt-nz / ckanext-security

A CKAN extension to hold various security improvements for CKAN
GNU Affero General Public License v3.0
25 stars 31 forks source link

Update/brute force lockout by account #30

Closed markstuart closed 4 years ago

markstuart commented 4 years ago

This PR represents a move away from tracking brute force login attempts based on IP address, to tracking per username instead.

During security auditing, it was found that brute force attacks could be continued if the IP address appeared to change (via manipulation of request headers).

Now if a username fails login more than 10 times in the configured period (15 mins by default), the account will be locked, regardless of what IP address triggered the attempts.

This PR also fixes an issue where the 2 factor authentication changes in PR #28 bypassed the brute force lockout mechanism as the username/password checking is now happening in an ajax controller action, not using the built in authentication mechanisms until both username/password and 2 factor auth have been validated.

ThrawnCA commented 4 years ago

I'm not sure that this is a good idea. Yes, it would be possible for an attacker to continue a brute force attempt by switching IPs. However, that also means that it's very difficult for an attacker to lock out the legitimate user. I think that's a reasonable trade-off.

How about making this configurable, so each site can decide whether it's more concerned about password breaking or about denial of service?

markstuart commented 4 years ago

How about making this configurable, so each site can decide whether it's more concerned about password breaking or about denial of service?

That sounds like a good approach @ThrawnCA, I'll look at doing that.