-
-
Notifications
You must be signed in to change notification settings - Fork 668
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
2.2.1 seems too busy for a single requirement #1763
Comments
I'm glad to see this requirement being discussed and refined. I want to share some ideas about it.
|
It all should be reviewed with knowledge, that we require MFA (from level 2):
This should put everything to different perspective. |
I provide my views and understandings.
We need to watch it separately - 2.1.7 and 2.1.14 blocks users to use it, which makes it even more suspicious, when someone start to use those "can not be used" passwords for guessing. So it makes sense to block them.
L2 for 1.14.8 is taken based on "blackbox" logic, that for being able verify the correct IP, verifier probably need more access than just blackbox test. With guessing password - it needs to block repeatitive attempts from one source (IP). The side effect here can be, that if source IP is taken incorrectly, it blocks too much.
Personally I'm not sure that this ever increasing is actually needed. The point for the requirement is to take down online guessing vector, if it is already few attempts per time X, it is too slow to cause the damage. If an attacker is really into it, he/she can find another IP for next few attempts. The question itself is valid and I asked the same last time, when current 2.2.1 was developed: #906 (comment)
We have requirements to cover this part, like:
(related issue #1272)
There should be strong authentication for unlocking. Also we have requirement like:
If there is way to lockout accounts, it's easy to lockout entire userbase. I don't think it is a good idea. Basically the requirement itself is against this idea:
For lockouts we need to keep in mind MFA point (#1763 (comment)) - even if attacker is able to guess the password, it still do not provide access to the application. |
I have some doubts about it. There are situations to consider, like when a user forgets their password and attempts to guess from a list of their commonly used passwords (even though using the same password for multiple apps isn't secure, some users still do it).
It's great to have this requirement in place. When the requirement is met in the app, it ensures that there's no chance to block the admin account. However, what if, in the process of achieving 100% coverage, this requirement isn't met, and a mechanism for blocking accounts is developed? In that case, it could be a good idea to include a comment about adding a workaround for admin account.
MFA is still not required for L1 applications. |
Let's also keep in mind that numerical-based multi-factor can be
brute-forced. We're seeing a lot of attacks that successfully pop
6-digit SMS MFA codes with relative ease.
|
Ok, I reviewed the useful thoughts from @belalena and @elarlang. You can skip to the tl;dr below My rough thoughts were as follows:
Long story short I hate this requirement, it is far too long and far too specific in a way that I don't think is helpful for implementation. The original point from NIST was for rate limiting. This current requirement spills into some specific requirements that I want to get rid of and also adaptive responses which NIST does include but I would prefer to rely on the other monitoring controls discussed here for this aspect. I think we need to have some sort of requirement for this as it is an important area but it needs to be shorter and less prescriptive. tl;dr As such, I would simplify to:
|
Direction is good - just giving principles without sending the "cheat sheet content" in the requirement, but I think there are 2 separate requirements:
|
Is then 2nd one important enough to deserve its own requirement? |
yes |
So like: Verify that malicious users cannot lock out legitimate users and admins through excessive incorrect login attempts? |
Let's keep the discussion over lockout in a separate issue: #2134 and keep focus here to find the wording for current 2.2.1 |
So as I noted here, I would simplify 2.2.1 to:
|
Credential stuffing and brute force are attacker actions to do. Account lockout is (maybe bad) developer choice when such attack is detected - I think those should not be put in the list like those are all the same thing. One is cause and other is impact. |
Are
Ok, check out #2148 |
Current proposal:
It is a bit hard to read, 4 times "and" in it. Maybe it should be split to different sentences to provide easier reading.
Hiting some limit is a security event by default and I think it is covered by current 7.2.1 (and it some point is not, it should belong there):
|
Here are two suggestions. One with logging: Verify that the application prevents credential stuffing and password brute-force attacks by using rate limiting, anti-automation measures, or adaptive response controls. Ensure that these security measures are included in the security logging and alerting system. One without logging Verify that the application prevents credential stuffing and password brute-force attacks by implementing rate limiting, anti-automation measures, or adaptive response controls. |
Current requirement has
We should keep in mind, that the requirement must be clear for how to test it. If it does not provide clear limits itself, it must ask it to be decided with security decision documentation requirements. |
I like it Josh |
I think the requirement can still be misinterpreted - I use brackets to highlight it:
Proposal direction:
But this one is not PR-ready as well:
This is probably one of the most tested requirements for simple applications, we need to be clear here. |
I don't think we can be too prescriptive here about the rate limiting. I think the deciding factor is whether it makes the attack impractical or not. @elarlang |
If it is wide opened it can not be implemented and tested. On option is to use the same way as we did with session - decide your limits, document them, implement them. |
Ok @elarlang so how about:
|
There seems to be too much going on in this requirement, I think it needs to be trimmed down or split up or something....
The text was updated successfully, but these errors were encountered: