aces / Loris

LORIS is a web-accessible database solution for longitudinal multi-site studies.
GNU General Public License v3.0
143 stars 172 forks source link

Password requirements don't follow NIST standards #3314

Closed gdevenyi closed 4 years ago

gdevenyi commented 6 years ago

The password setting requirements violate best practices as recommended by NIST: https://pages.nist.gov/800-63-3/sp800-63b.html#appA

https://pages.nist.gov/800-63-3/sp800-63b.html#memsecret

gdevenyi commented 6 years ago

Verifiers SHOULD NOT impose other composition rules (e.g., requiring mixtures of different character types or prohibiting consecutively repeated characters) for memorized secrets. Verifiers SHOULD NOT require memorized secrets to be changed arbitrarily (e.g., periodically). However, verifiers SHALL force a change if there is evidence of compromise of the authenticator.

driusan commented 6 years ago

Hi @gdevenyi,

I think it's important to take the NIST guidelines as a whole, and not selectively apply them. They don't simply say "don't have any requirements other than an 8 character limit." Selectively applying the parts of the best practices which contradict and relax previously published best practices is without simultaneously implementing the other parts which mitigate the risks is likely worse than using an outdated password requirement scheme.

For instance, this part of the document you linked to is important:

When processing requests to establish and change memorized secrets, verifiers SHALL compare the prospective secrets against a list that contains values known to be commonly-used, expected, or compromised. For example, the list MAY include, but is not limited to:

    Passwords obtained from previous breach corpuses.
    Dictionary words.
    Repetitive or sequential characters (e.g. ‘aaaaaa’, ‘1234abcd’).
    Context-specific words, such as the name of the service, the username, and derivatives thereof.

If the chosen secret is found in the list, the CSP or verifier SHALL advise the subscriber that they need to select a different secret, SHALL provide the reason for rejection, and SHALL require the subscriber to choose a different value.

As is this:

Verifiers SHALL implement a rate-limiting mechanism that effectively limits the number of failed authentication attempts that can be made on the subscriber’s account as described in Section 5.2.2.

That said, as a user I agree that the password requirements implemented by LORIS are dated and need to be modernized, but in order to remain secure I think it's import that it be implemented alongside the other updated best practices, and not simply by removing the password strength requirements currently in place as your ticket seems to be requesting.

driusan commented 6 years ago

I went through the requirements and tried to enumerate them in an easier checklist.

The following MUST be done in order to be compliant with the best practices:

The following SHOULD be done, but aren't obligatory according to the standard:

gdevenyi commented 6 years ago

Sure, I'm totally okay with implementing the whole thing, but I highlighted specifically the content requirements because they are known to cause weaker passwords.

I use a password manager, so my passwords are long, random and can easily satisfy requirements. Other users might have a much harder time.

gdevenyi commented 6 years ago

Troy hunt offers an API for checking known bad passwords: https://haveibeenpwned.com/Passwords

johnsaigle commented 6 years ago

The pull requests just referenced address these issues mentioned above:

The following SHOULD be done, but aren't obligatory according to the standard:

driusan commented 5 years ago

The full updated list:

The following SHOULD be done, but aren't obligatory according to the standard:

johnsaigle commented 5 years ago

I just unchecked the ones relating to blacklisting. #3962 contained that functionality but has since been closed and not merged. #4411 now contains the blacklisting functionality.

johnsaigle commented 4 years ago

4411 was just merged for upcoming LORIS v. 22

Essentially all we need now is a visual indicator of password strength on the front-end.

Here's the list, updated again:

The following SHOULD be done, but aren't obligatory according to the standard:

gdevenyi commented 4 years ago

Essentially all we need now is a visual indicator of password strength on the front-end.

https://github.com/dropbox/zxcvbn

johnsaigle commented 4 years ago

We're using Zxcvbn on the back-end as of the PR I linked, but not yet in the front-end.

johnsaigle commented 4 years ago

Closing this because we have separate tickets for the remaining features:

5761 front-end strength estimator.

5293 improve unicode support.