Authenticator.verifyPasswordStrength thinks "password123" is a good password about esapi-java-legacy HOT 9 OPEN

From chrisisbeef on October 28, 2009 22:02:53

This algo needs to be addressed for ESAPI 2.1 or 3.0. At this point in time, I would
assume that not many users will be using the reference implementation of the
Authenticator since most applications already have some Authentication framework in
place (be it a home-grown or pre-packaged solution.)

At the point that we address this, a good deal of the functionality that is in the
FileBasedAuthenticator needs to be moved to a base class that the user can extend to
provide a good RI of these functions without forcing the user to either use the
FileBasedAuthenticator or Copy/Paste the RI code into their own wrapper class around
their existing user authentication classes.

Status: Accepted
Owner: chrisisbeef
Labels: Security Release-2.1

from esapi-java-legacy.

From chrisisbeef on October 28, 2009 22:11:22

Labels: -Release-2.1 Milestone-Release2.1

from esapi-java-legacy.

From [email protected] on May 08, 2010 18:59:41

Agree that this whole algorithm needs to be addressed. The best approach would be to
use something like the Crack library (libcrack.so) that is commonly used with PAM on
*nix systems. There is a FOSS port of this library to Java available at SourceForge
done back in Jan 2000 and not touched since. It is part of the "Solinger Java
Utilities Project". See http://sourceforge.net/projects/solinger/files/Java%20CrackLib/ for details.
However, if we want to be serious about providing a decent reference model for
verifying password strength, we really need something similar to Crack.

from esapi-java-legacy.

From [email protected] on November 06, 2010 23:41:42

In FileBasedAuthenticator, I just noticed there were several additions
to createUser() and changePassword() where a check to see if the provided
password was the acct name. E.g., in changePassword(),

verifyPasswordIsNotAccountName(accountName,newPassword);    // Added
verifyPasswordStrength(currentPassword, newPassword);
user.setLastPasswordChangeTime(new Date());

So, my question is "why is this check not part of the password strength
by default?". In other words, if your password is the same as your
account name, I'd pretty much say that your password strength should
be zero and an AuthenticationCredentialsException should be thrown.

I'd propose moving a check like this into the the public
verifyPasswordStrength() method so every time there is an occassion
to check the password strength, one doesn't need to remember to call
verifyPasswordIsNotAccountName(). (I mean, is there ever a good
reason as to why we would want the password to be set to the user
account name?) However, to do this, we would either have to change
the signature of verifyPasswordStrength() to include a User object
or assume that it is called on the current User object. (E.g., the
reference FileBasedAuthenticator class could call this.getCurrentUser()
to check if the password matched the account name without causing
the signature to be changed.) But the downside of using the current
user rather than changing the signature is that this would not do the
right thing for Authenticator.createUser().

Alternately, we could generalize it even more by separating this out of
the Authenticator class altogether and creating a PasswordPolicy class
that could be used to check against the same thing. It could still be
called from the reference FileBasedAuthenticator methods createUser()
and changePassword(), but would just take a slightly different form.

from esapi-java-legacy.

From [email protected] on November 06, 2010 23:41:51

Labels: -Milestone-Release2.1 Milestone-Release2.0

from esapi-java-legacy.

From chrisisbeef on November 20, 2010 13:16:06

Labels: Component-Authenticator

from esapi-java-legacy.

@kwwall , while we were working to resolve the intermittent test failures From the FileBasedAuthenticator it was suggested on a couple of occasions that the implementation may not be as "production worthy" as we'd like. I believe at one point we were discussing either deprecating the impl or moving it into test scope to discourage its use. With that in mind, is this issue work that we want to maintain?

from esapi-java-legacy.

We either need to fix it or deprecate it. Ideally, when you deprecate something, we ought to replace it with something (better). Generally an annotation like:
            @deprecated Sorry; you're SOL
isn't going to be appreciated and IMO, that's the message you are sending if you don't have some alternative to suggest. (Note the alternative doesn't have to be in ESAPI, but needs to be a FOSS Java solution.)

That said, I do have a few ideas, but I'd prefer to wait to implement them after we kick out this release.

from esapi-java-legacy.

Sounds good. I'll try to follow up with you after the release.

from esapi-java-legacy.