Bug 53785 - Modern password hashing for built-in Realms
Summary: Modern password hashing for built-in Realms
Status: RESOLVED WONTFIX
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: unspecified
Hardware: All All
: P2 enhancement with 1 vote (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-27 22:20 UTC by david
Modified: 2012-08-28 20:45 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description david 2012-08-27 22:20:40 UTC
Password-based authentication for the built-in realms can currently use three digestion algorithms from the java.security.MessageDigest class (SHA, MD2, or MD5).  All of these are out of date*, and each Realm implementation does its own comparison of the password to the saved digest.

[*It's not clear whether Java's SHA is SHA-1 or one of the SHA-2 algorithms.  SHA-1 is obsolete;  SHA-2, potentially less so.]

I recently created my own custom Realm in order to support bcrypt.  While I do not claim that bcrypt is the right algorithm for everyone, it is a much better default than the current built-in options-- so Tomcat should offer it.

However, rather than being a general purpose hash function, bcrypt a one-way hash designed for passwords.  The salt is built into the hash in such a way that it can't be extracted.  That is to say, you can't say:

    if ( bcrypt.hash(password1) == bcrypt.hash(password2) )
        log("Passwords match");

because every time you hash a password, you get a different result.  This is a security feature, since novices won't mismanage the salt.  Instead, you call:

    String hash = BCrypt.hashpw("hello"); // To hash, not to check

    if (BCrypt.checkpw(passwordFromLoginForm, savedPasswordHash))
        log("Passwords match");

This example uses the JBCrypt implementation at http://www.mindrot.org/projects/jBCrypt/

Like I said, I don't think BCrypt is the right solution for every user. See http://www.unlimitednovelty.com/2012/03/dont-use-bcrypt.html and http://security.stackexchange.com/questions/4781/do-any-security-experts-recommend-bcrypt-for-password-storage

Also note that NIST will recommend a new secure hashing algorithm soon ( http://csrc.nist.gov/groups/ST/hash/timeline.html ) although that will be a general purpose cryptographic hash function, not an out-of-the-box password hash format like bcrypt.


Instead, I propose that we make three (or four) changes:

1. Update all applicable subclasses of RealmBase to call a new method, RealmBase.checkDigest(String credentials, String savedHash), instead of each implementation doing a string comparison against the realms.

2. Implement RealmBase.checkDigest with the following rules:
   a.  If digest == null, implement the current string comparison.
   b.  If digest is "SHA", "MD2", or "MD5", compare with the current algorithm.
   c.  If digest is the name of a Java class, try calling checkPassword(credentials, savedHash) on the class, both as a static method and on an instance created with no constructor arguments.
 
3. (Depending on legal issues) Bundle Tomcat with JBCrypt, thus providing a secure hash out of the box.

4. Write unit tests and documentation and update Tomcat 7+ with the new code.  Of course, we could jump ahead and implement this in Tomcat 8, since this is a public API change.

I will check with my boss to see if I can take the time to implement this.  Of course, I'd prefer to get feedback before I go ahead with it.
Comment 1 david 2012-08-27 22:22:30 UTC
Should have said under step 3:  "Bundle BCrypt with Tomcat" rather than the other way around.
Comment 2 Mark Thomas 2012-08-27 22:42:00 UTC
Tomcat supports the use of any MessageDigest provided by the JRE. Additional algorithms may be supported by adding 3rd party security providers to the JRE.

FYI:
- as far as the Sun JRE is concerned, SHA is an alias for SHA-1.
- the MessageDigests supported by the latest Sun JDKs for Java 5 to Java 7 are:
MD2, MD5, SHA-1, SHA-256, SHA-384, SHA-512

One open source provider is BouncyCastle. It certainly provides additional digests although I haven't investigated how secure they are.

I do not see the point in adding bloat to Tomcat to provide a feature that the JRE already provides.
Comment 3 david 2012-08-28 14:45:33 UTC
I think you miss one important point, namely that Tomcat only supports bidirectional hashes, whereas modern password hash functions are one-way.

One workaround is for the user to provide salt (which currently isn't possible--see Bug 51966), but that is more error prone and arguably less secure than having the salt baked into the password algorithm.

Using a different MessageDigest does not fix this.


But as a bare minimum, could we at least change the documentation to direct novice users toward SHA-256 or better, since it currently implies that SHA, MD2, and MD5 are the only options?
Comment 4 Konstantin Kolinko 2012-08-28 18:04:52 UTC
> Tomcat only supports bidirectional hashes

I am sure that you are wrong in the above statement. If I missed something, please define what you mean by "bidirectional".

All cryptographic hash functions supported by Tomcat are one-way (as a hash function should be).
Comment 5 Christopher Schultz 2012-08-28 20:27:51 UTC
Ignoring several of David's foolish statements (one-way hashing versus bi-directional, bcrypt using non-retrievable salts, etc.), I do think this enhancement request has merit.

David is right that merely using a "better" hash algorithm (i.e. SHA-65536) isn't the same as using a so-called "password-hashing algorithm" like bcrypt, scrypt, etc.: those algorithms use built-in random salts and iterations of the base algorithm (which is something like SHA-256) and do require more than just "hashing" the incoming password and comparing it to the stored hash.

One could also argue that this kind of thing can be implemented outside of Tomcat by merely writing a custom Realm, which is certainly true, but the existing API doesn't lend itself to extensibility (other than to the realms currently built-into Tomcat). I think there are some changes we could make to the code to allow a bcrypt-based implementation to bemuch easier to built.

For instance: instead of a method that hashes the attempted-password, retrieves the known-hashed-password and comparing the two, perhaps we could have a method that retrieves the known-hashed-password and then calls a method to "hashAndCompare" or something similar. This would allow a bcrypt-based implementation to merely override this method and use the bcrypt tools to compare (for some definition of "compare") an attempted-password directly to the known-hashed-password while the "standard" implementation could perform the existing MessageDigest-based hashing and String-comparison.

This then brings up the fact that RealmBase seems like the most reasonable place to do all of this, except that nobody really wants to extend RealmBase because the real action is in the realm implementations (DataSourceRealm, JNDIRealm, etc.). If the "hashing stuff" could be isolated from RealmBase, then we could make it pluggable such that users could use a DataSourceRealm or JNDIRealm but also plug-in a bcrypt implementation for the password-hashing work.

I'd like to reopen this unless there are any strong objections.
Comment 6 david 2012-08-28 20:31:56 UTC
> > Tomcat only supports bidirectional hashes
>
> I am sure that you are wrong in the above statement. If I missed something, 
> please define what you mean by "bidirectional".
>
> All cryptographic hash functions supported by Tomcat are one-way (as a hash 
> function should be).

Bidirectional in the sense that the same function is used for password checking and password hashing.

I am making a distinction between the underlying general-purpose hash algorithm and the password hashing algorithm.  Although password hashing algorithms are built on top of general-purpose hash algorithms, they may include additional steps, such as salting.

A password API which includes salt is unidirectional in the sense that the same function is not used both for encryption and decryption.  Thus it is not possible to do a string comparison between two hashes in order to validate a password, as is done by Tomcat.

Since Tomcat does password checking like this:

  validated = (digest(credentials).equals(dbCredentials));

you cannot use a system such as bcrypt (or any API with built-in salt) which has a separate function for validation.

Mind you, bcrypt is built on top of Blowfish, but from a user's perspective it is more than just a front end to Blowfish.  A bcrypt hash includes the Blowfish hash, the salt, and the cost (log of number of iterations)-- all of which are required for validation.

The cost is a core feature of bcrypt:  the user can specify how expensive password checking is, and therefore how long brute force password guessing will take.

There is another password hashing algorithm, scrypt, which defines cost in terms of time and memory.  You'll also hear about PBKDF2 as well, although it is not so much an API as recommended procedures for password encryption (described in RFC 2898).


The point is that to do proper password checking you can't just do this (in JDBCRealm):

  validated = (digest(credentials).equals(dbCredentials));

as it doesn't work with salt;  nor can you do this:

  validated = (digest(credentials, salt).equals(digest(dbCredentials, salt)));

as it doesn't work with a cost function; and while you could do this:

  validated = (digest(credentials, salt, cost).equals(digest(dbCredentials, salt, cost)));

that's just getting silly-- and it won't work anyway since PBKDF2 requires that you specify an additional algorithm.  It's a lot more general and future-proof to just do this instead:

  validated = validate(credentials, dbCredentials);
Comment 7 Mark Thomas 2012-08-28 20:45:58 UTC
(In reply to comment #5)
> This then brings up the fact that RealmBase seems like the most reasonable
> place to do all of this, except that nobody really wants to extend RealmBase
> because the real action is in the realm implementations (DataSourceRealm,
> JNDIRealm, etc.). If the "hashing stuff" could be isolated from RealmBase,
> then we could make it pluggable such that users could use a DataSourceRealm
> or JNDIRealm but also plug-in a bcrypt implementation for the
> password-hashing work.
> 
> I'd like to reopen this unless there are any strong objections.

I have no issue with refactoring to make Realms easier to extend (for better digests or for any other reason) subject to the usual issues regarding backwards compatibility.

I not so keen on including jBCrypt (or any other library) without a much stronger demand for it.