Topics

Review PasswordHash / DatabaseIdentityStore


Rudy De Busscher
 

Copied some parts out of the other thread as it becomes too unreadable.

  • remove the "plumbing" that passees the properties down to the hash implementation, in case we want to restore the properties. But they are not currently passed in to the PasswordHash.

Even if the standard Pbkdf2PasswordHashImpl doesn't use it, I think it doesn't hurt much to pass them in anyway.

The parameters are required. if the PasswordHash interface doesn't have methods with the parameters passed along, there is no implementation ever that can use them!

  • Added support for multiple flavors of PBKDF2 (i.e., "with" different Hmac algorithms). Now supports all SHA1/2 variants, but not MD5 (which doesn't appear to be available, or at least not enabled by default, in JDK8)
Ok

I would also consider removing SHA1. Just as MD5, it is no longer considered as really safe.

That said, if there's no time, there's no time. Or in other words, it is what it is ;)


But it needs to be useable. Otherwise, it is better to deliver nothing than something which isn't working properly and usable!
If the perception of Java EE security API is not good during the first months, it will never be used by anyone, even if we come up with a better version later on.

Sorry to be this hard, but that is the truth.


A small detail about the code, I would make it more Java 8 alike ( with diamond operator, foreach method, ....)

If you like, I can make a PR for that.

Rudy


Will Hopkins
 

Thanks, Rudy, I responded to your comments in the original thread.

On 07/23/2017 08:08 AM, Rudy De Busscher wrote:
Copied some parts out of the other thread as it becomes too unreadable.

  • remove the "plumbing" that passees the properties down to the hash implementation, in case we want to restore the properties. But they are not currently passed in to the PasswordHash.

Even if the standard Pbkdf2PasswordHashImpl doesn't use it, I think it doesn't hurt much to pass them in anyway.

The parameters are required. if the PasswordHash interface doesn't have methods with the parameters passed along, there is no implementation ever that can use them!

  • Added support for multiple flavors of PBKDF2 (i.e., "with" different Hmac algorithms). Now supports all SHA1/2 variants, but not MD5 (which doesn't appear to be available, or at least not enabled by default, in JDK8)
Ok

I would also consider removing SHA1. Just as MD5, it is no longer considered as really safe.

That said, if there's no time, there's no time. Or in other words, it is what it is ;)


But it needs to be useable. Otherwise, it is better to deliver nothing than something which isn't working properly and usable!
If the perception of Java EE security API is not good during the first months, it will never be used by anyone, even if we come up with a better version later on.

Sorry to be this hard, but that is the truth.


A small detail about the code, I would make it more Java 8 alike ( with diamond operator, foreach method, ....)

If you like, I can make a PR for that.

Rudy

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803