Yes, as you've noticed, and as Kalle mentioned, this was an oversight
in the API back in the pre 1.0 days - it is not safe to obtain a salt
based on what the end-user submits (i.e. what is represented by an
AuthenticationToken).  System that obtain salts this way susceptible
to inference attacks.  Instead, it is *much* safer to obtain the salt
from the stored account data (what is represented by an
AuthenticationInfo instance). And even then, a salt should be ideally
created from a cryptographically-strong random number generator, and
not by using account data (password, username, whatever).

Here is how I've solved it in my applications, and I hope to get some
time to contribute this back to the project - it is a much cleaner
solution, and very similar to Kalle's.

First, because we want the salt to come from the stored account data
it makes much more sense for the salt to be represented in the
AuthenticationInfo returned from the Realm.  So, I created an
AuthenticationInfo sub-interface:

/**
 * An AuthenticationInfo instance that represents a salt used to hash
the stored credentials.
 */
public interface SaltedAuthenticationInfo extends AuthenticationInfo {

    /**
     * Returns the salt used to hash the credentials.
     *
     * @return the salt used to hash the credentials.
     */
    Object getSalt();
}

An instance of this interface is returned from my Realm's
doGetAuthenticationInfo implementation.  Bringing this in to Shiro's
API should work fine (or it can be a standalone interface,
'SaltSource' or something like that, but since Shiro has no need to
reference a salt outside of the AuthenticationInfo context, I thought
a sub-interface would be fine).

Next, I created a subclass of the HashedCredentialsMatcher I'm using.
In this case, I'm using the Sha512CredentialsMatcher:

public class SaltedSha512CredentialsMatcher extends Sha512CredentialsMatcher {

    /**
     * This implementation acquires the <tt>token</tt>'s credentials
     * (via {...@link #getCredentials(AuthenticationToken) 
getCredentials(token)})
     * and then the <tt>account</tt>'s credentials
     * (via {...@link
#getCredentials(org.apache.shiro.authc.AuthenticationInfo)
getCredentials(account)}) and then passes both of
     * them to the {...@link #equals(Object,Object)
equals(tokenCredentials, accountCredentials)} method for equality
     * comparison.
     *
     * @param token the <tt>AuthenticationToken</tt> submitted during
the authentication attempt.
     * @param info  the <tt>AuthenticationInfo</tt> stored in the
system matching the token principal.
     * @return <tt>true</tt> if the provided token credentials are
equal to the stored account credentials,
     *         <tt>false</tt> otherwise
     */
    @Override
    public boolean doCredentialsMatch(AuthenticationToken token,
AuthenticationInfo info) {
        Object tokenCredentials;
        if (isHashSalted()) {
            tokenCredentials = hashProvidedCredentials(token, info);
        } else {
            tokenCredentials = getCredentials(token);
        }
        Object accountCredentials = getCredentials(info);
        return equals(tokenCredentials, accountCredentials);
    }

    private Object hashProvidedCredentials(AuthenticationToken token,
AuthenticationInfo info) {
        Object salt;
        if (info instanceof SaltedAuthenticationInfo) {
            salt = ((SaltedAuthenticationInfo) info).getSalt();
        } else {
            //for backwards compatibility:
            salt = getSalt(token);
        }
        return hashProvidedCredentials(token.getCredentials(), salt,
getHashIterations());
    }
}

This logic should make its way into the base HashedCredentialsMatcher
implementation.

Finally, an instance of this CredentialsMatcher is configured on my Realm.

I'll try to contribute this to the codebase sometime this coming week,
after some brief discussion on the dev list.

I hope this helps!

Cheers,

Les

Reply via email to