> I agree that PasswordManager would not be an ideal name, it's vague.
> However, HashManager and EncryptionManager reflect the one-way and two-way
> functions. I would be worried that we would be mixing concerns in a
> CredentialsManager. I am making my comments with my previous outline in
> mind. It might be useful if you provided bits of code to give a bit more
> context.
In attempting to put together an example, I think I found where there
might be a mental disconnect. Let's see if I can ask some more
questions that help me understand your use case.
What are the K (key) and PD (perData) values and why would they be
necessary? I don't understand their purpose at the moment.
When you use a cryptographically strong secure random number as the
salt, there is no need to 'mix in' additional data along with the data
being hashed - it is already secure enough for credentials hashing.
You would only really need to add additional data to the salt (as Mike
referenced in RFC 2898) to help distinguish the salt's purpose between
being used for password-based encryption (PBE) or Message
Authentication Codes (MAC).
At the moment (as I understand it) we're talking about using passwords
+ salts to support Shiro end-user authentication only - not PBE or MAC
scenarios - and adding data (non-random octets) to the salt is
unnecessary for this purpose. It's necessary for PBE and MAC usages
because those two mechanisms assume key sharing - where two different
parties might share the same key to perform either function. But for
password matching, it is expected that each user has their own
password and cryptographically-strong salt that is not shared with any
party.
The information used for Shiro to reconstitute the hash, such as # of
hash iterations is usually application wide and can be specified as a
configuration parameter in Shiro (e.g.
credentialsManager.numHashIterations = 2048). The only reason I can
see for storing it in the salt directly is that you might want the #
of hash iterations to be unique per user. Is this what you'd like to
support?
While this is possible, most applications that would ever change that
number end up doing it on a periodic basis system-wide, for example,
once a week or once a month for all accounts. I'm not sure if
per-user values would be 'worth it', but I'm totally open on this and
would love to see what you guys think.
>> Also, we already have Hash and CipherService concepts in Shiro (no
>> need for something like Encryptor that I can see) - the
>
> Can you explain your statement "no need for something like Encryptor that I
> can see"?
To me, Encryptor looked like the same thing as the CipherService in
practice. Is this not the case?
>> CredentialsManager would just sit a level above these and use them
>> both, probably along with a RandomNumberGenerator and tie all three
>> things together.
>
> Again, a bit more code would help here. :)
For example,
public interface HashedCredentialsManager {
HashedCredentials hashCredentials(ByteSource credentials);
boolean credentialsMatch(AuthenticationToken token,
AuthenticationInfo info);
}
public interface HashedCredentials {
Hash getCredentials();
ByteSource getSalt();
}
An implementation of this interface would simply use delegation - it
would would use an instance of RandomNumberGenerator to acquire a
salt, pair it with the credentials as the key, hash it using the
configured hash algorithm name, and return a HashedCredentials
instance.
The HashedCredentials instance would only be used so the caller could
acquire both the generated (hashed) credentials/password as well as
the salt so they can store both as necessary.
The 'credentialsMatch' method would just delegate to an internal
HashedCredentialsMatcher using the same hash algorithm.
Also note that the number of hash iterations doesn't need to be
'known' to anything except the HashedCredentialsManager
implementation, allowing for a single place for configuration.
But this begs the question - the above HashedCredentialsManager _is_ a
HashedCredentialsMatcher by the presence of the 'credentialsMatch'
method. Wouldn't it make more sense to just have
HashedCredentialsManager sub-interface CredentialsMatcher?
>> Finally, it doesn't make sense to me to have a 'key' attribute forced
>> upon a Hash interface. Hashes have no concept of a 'key' and that
>
> The key is critical since it is used to indicate what hash or encryption was
> used to obtain that bit of data.
Please see above. I'm assuming we're talking about credentials
matching still and not PBE or MACs.
Anyway, this is great discussion - I hope our mental models sync up :)
Best,
Les
