shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan D. Cabrera" <>
Subject Re: Password hashing - getting the salt from AuthenticationInfo
Date Sat, 23 Oct 2010 15:24:27 GMT

On Oct 19, 2010, at 8:17 PM, Les Hazlewood wrote:

> Hi folks,
> I've committed a good bit of code to trunk that allows for safe
> acquisition of a salt from AuthenticationInfo when hashing passwords.
> I also created a nice little RandomNumberGenerator abstraction which
> can be used for generating random (and secure) salts, initialization
> vectors, or any other type of cryptographic seed data.  Shiro
> end-users can use this in their own apps for user-account salt
> creation.  This is _much_ better and safer than using any
> account-related data (e.g. username or something else) as the salt.
> Also, existing AuthenticationInfo implementations have been updated to
> support this.
> I've written a few test cases to verify that the new behavior works
> and that I've retained backwards compatibility (although it is highly
> recommended to use the new approach since user-submission-derived
> salts are dangerous).  See the HashedCredentialsMatcherTest
> 'testSaltedAuthenticationInfo' test case to see a good/common example
> of how the new salt support would work in a typical application.
> Anyway, feedback is welcome.  If I don't hear anything, I'll resolve
> the issue and consider it finished for 1.1 (issue: Here's the issue:

This is great.  I also needed this functionality.

Some comments.  IMO, Shiro uses inheritance too much.  It's a brittle practice that I think
we're now starting to see the cracks; e.g. the conversion to use StringBuilder.  

It seems that  are some hashing implementation details that have leaked into the abstract
methods, i.e. hashing iterations. 

I'm not a fan of mixing data with code that manipulates it, e.g. the Hash hierarchy.  Just
a personal preference.  Things start to end up looking like swiss army knives and Hash seems
to be a good example.

Just my 2 cents.  Thanks for getting this set up!


View raw message