On Tue, Oct 19, 2010 at 8:17 PM, Les Hazlewood <[email protected]> wrote: > I've committed a good bit of code to trunk that allows for safe > acquisition of a salt from AuthenticationInfo when hashing passwords.
The new SaltedAuthenticationInfo interface is an interesting take on this. With it, we are rolling the responsibility of storing and retrieving the salt on users. The implementation I'm currently using was based on adding getSalt(AuthenticationInfo token) to a HashedCredentialsMatcher and let it decode the salt from the credentials. The advantage is that there's no need for new AuthenticationInfo interface and that is easy to provide a default implementation. Obviously the new interface is more flexible but I wonder whether anybody ever needs that flexibility. Since you always need the salt and the hashed password together, you can just as well store them together (like Unix-style passwords). Take it as a general comment - the differences are minor, I'm not too particular on any given approach and we can still provide a default implementation that for example assumes a fixed-with salt stored with the hash. > recommended to use the new approach since user-submission-derived > salts are dangerous). See the HashedCredentialsMatcherTest I wouldn't go as far as saying that static salt is dangerous - any salt is better than no salt - but since random per-user-salt doesn't cost any extra and is much more secure, it doesn't make sense to use anything else. > 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) Perhaps just for symmetry, SimpleAuthenticationInfo.setSalt() should be renamed to setCredentialsSalt(). Obviously it's not in the interface, but the getter is called getCredentialsSalt(). Kalle
