picketlink/picketlink — pull request #184 by mposolda (2013-09-03 21:36:13+00:00)
Title: PLINK-258 PasswordCredentialEncoder.generateSalt() always generate same value 

To address the issue, I've added SecureRandomProvider interface and two implementations

1) AutoReseedSecureRandomProvider implementation, which returns SecureRandom generated with random seed. It also periodically replaces SecureRandom with new instance after each 5 minutes, so that it's not possible to guess seed value from previously generated salts (See http://www.cigital.com/justice-league-blog/2009/08/14/proper-use-of-javas-securerandom/ for background info).

This implementation should be safe, however random generation of seed is platform dependent and could be sometimes slow on some platforms (It's blocking operation, which initializes seed from system data). For this purpose, the generation of SecureRandom is asynchronously done in separate thread (we have something like this in GateIn portal as well), but it may still block the caller if he use getSecureRandom() before the initialization is done.

2) SimpleSecureRandomProvider implementation, which is quite similar to current implementation, so the seed is not random and is predictable (despite the fact that it's using same instance of SecureRandom provider, so it won't always generate same salt like currently). This implementation is not secure, however is fast, so it should be good for unit test purposes

I've changed PasswordCredentialHandler to read SecureRandomProvider from credentialHandlerProperties map and use secure version AutoReseedSecureRandomProvider by default.
I've also changed almost all unit tests to use SimpleSecureRandomProvider, so they are not blocked by possibly slow initialization of SecureRandom. I've just added one new unit test "RealSecureRandomProviderTestCase" to use secure AutoReseedSecureRandomProvider.

I've also changed AbstractIdentityStoreConfiguration.credentialHandlerProperties to not be unmodifiable map, so that if PasswordCredentialHandler needs to use AutoReseedSecureRandomProvider, it will add it back to the map, so it could be shared with next CredentialHandler instances (like TOTPCredentialHandler)

Let me know if you're happy with this approach or if you prefer to change something or do it differently