Authenticator.verifyPasswordStrength thinks "password123" is a good password

[meg23]

From cyounk...@gmail.com on August 04, 2009 11:11:31

In AuthenticatorTest.java there is this test:
try {
instance.verifyPasswordStrength("password", "password123");
fail();
}
catch ...

The test passes, indicating that verifyPasswordStrength raised an exception
about the password quality, but only because it is too similar to the
previous password (the first param). Passing in an empty string instead of
"password" will cause the test to fail, because in the default
implementation, "password123" will have a "strength" of 22. 11 chars * 2
charsets = strength of 22, which is > than the 16 limit. Can't we come up
with a better metric? A couple suggestions:

1. Make a verifyPasswordStrength(String) method that will only check the
   new password. Also, make generateStrongPassword call verifyPasswordStrength
   before returning output.
2. Add into the API doc that it is a good idea that the implementation
   compare the password to a dictionary list, and that the default
   implementation does not do this.
3. Can we change the "strength" algorithm to something a bit better? How
   about number of bits of entropy? First, check what charsets are being used.
   Add up the length of the charactersets in use, yielding the length of the
   whole charset of the password, N. The entropy per symbol is then log(N)
   base 2, and multiplied by the length of the password will give its entropy
   in bits.

This metric is less valid for human-generated passwords. An alternative is
to up the strength limit. What version of the product are you using? On what operating system? SVN revision 574 .

Original issue: http://code.google.com/p/owasp-esapi-java/issues/detail?id=26

[meg23]

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

[meg23]

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

Labels: -Release-2.1 Milestone-Release2.1

[meg23]

From kevin.w.wall@gmail.com 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.

[meg23]

From manico.james@gmail.com 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.

[meg23]

From manico.james@gmail.com on November 06, 2010 23:41:51

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

[meg23]

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

Labels: Component-Authenticator

[jeremiahjstacey]

@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?

[kwwall]

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.

[jeremiahjstacey]

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