joscha / play-authenticate

An authentication plugin for Play Framework 2.x (Java)
http://joscha.github.com/play-authenticate/
Other
807 stars 366 forks source link

Re "You *SHOULD* provide your own implementation of this which implements your own security." #273

Open dan-lind opened 9 years ago

dan-lind commented 9 years ago

Inside the UsernamePasswordAuthUser.class you can find the following snippets of code related to creating and checking password hashes:

    /**
     * You *SHOULD* provide your own implementation of this which implements your own security.
     */
    protected String createPassword(final String clearString) {
        return BCrypt.hashpw(clearString, BCrypt.gensalt());
    }

    /**
     * You *SHOULD* provide your own implementation of this which implements your own security.
     */
    public boolean checkPassword(final String hashed, final String candidate) {
        if(hashed == null || candidate == null) {
            return false;
        }
        return BCrypt.checkpw(candidate, hashed);
    }

Isn't this code already following best practices, e.g. the OWASP Password Storage Guidelines found here: https://www.owasp.org/index.php/Password_Storage_Cheat_Sheet

What would be the main reasons to change this implementation?

albuch commented 9 years ago

I think the main reason at least for this specific example using bcrypt would be to adapt the number of rounds for the bcrypt hashing function so that it fits your security needs and the available resources of your production environment. The following question and answers should give you a hint: http://security.stackexchange.com/questions/17207/recommended-of-rounds-for-bcrypt