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:
> https://issues.apache.org/jira/browse/SHIRO-186)

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!


Regards,
Alan

Reply via email to