On 24.1.2014 12:55, Bruno Oliveira wrote:
Let me check if I'm misreading you, are you considering to make the salt static? If the answer is yes, the salt would be predictable, no? Not sure if I'm following you on this.
No, not salt of course. I meant to have just SecureRandom instance declared to be static (or maybe not necessarilly static but shared somehow via something like Picketlink is doing with SecureRandomProvider or we are doing in GateIn). The expensive part is initial generation of seed for SecureRandom and if this would need to be done during each encoding/validation, you will likely see performance issues. Especially in production environments with many logins/password validations.

You can easily try to run the code snippet below couple of times on your machine and maybe you will see what I meant.

Marek


On Fri, Jan 24, 2014 at 7:49 AM, Marek Posolda <mposolda@redhat.com> wrote:
Hi Bruno,

that's great improvement. I just want to point the creation of SecureRandom during each password encoding/validation as it's platform dependent and quite expensive. We already have issues with this in GateIn. For example running this on my laptop:

    public static void main(String[] args) throws NoSuchAlgorithmException {
        SecureRandom.getInstance("SHA1PRNG").nextBytes(new byte[16]);
    }

is sometimes quick, but sometimes it took about 30 seconds to finish (depends on available random entropy from /dev/urandom on linux laptops). So IMO will be better to have it declared as static variable?

We already discussed and solved similar issue with picketlink team some time ago. In the end Picketlink is using SecureRandomProvider abstraction for retrieve SecureRandom instance. And default implementation of this interface is good compromise between security and performance.

Marek


On 22.1.2014 13:55, Bruno Oliveira wrote:
Good morning guys, as a suggestion to improve the way how the passwords have been stored in nowadays I did some changes to support PBKDF2[1] (we have been doing the same thing on AeroGear for mobile devices), into this way is possible to prevent rainbow tables and brute force attacks like HashCat does for example.

I'm completely fine on adding bcrypt as long as we include some KDF, I just didn't that because I would like to hear some feedback before move forward, not sure if makes sense but my suggestion is to remove SHA-* encoders because they can be easily broken and replace by the support for PBKDF2 and bcrypt only.

What do you think? Let me know if I should move forward or that doesn't fit.

[1] - https://github.com/keycloak/keycloak/pull/171






--

-- 
"The measure of a man is what he does with power" - Plato
-
@abstractj
-
Volenti Nihil Difficile