ESAPI
Authenticator.verifyPasswordStrength thinks "password123" is a good password
From [email protected] 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:
- Make a verifyPasswordStrength(String) method that will only check the new password. Also, make generateStrongPassword call verifyPasswordStrength before returning output.
- 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.
- 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
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 [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 [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 [email protected] on November 06, 2010 23:41:51
Labels: -Milestone-Release2.1 Milestone-Release2.0
@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?
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.
