Use cryptography secure random generator for vibe.http.session

[ilya-stromberg]

Please, use cryptography secure random generator for Session m_id field, not a uniform(0, 255). It creates unsecured session id, and bad guy can predict next session id if he know current one. So, he can hack all sessions. I have a very simple cryptography secure random generator based on "/dev/urandom" (Linux/POSIX only). So, I can add pull request if you want. Also, OpenSSL have cryptography secure random generator, you can use it.

[mihails-strasuns]

There have been series of random number generator security related patches if Phobos few months ago. Are you sure we still need to resort to external stuff (contrary to simply using different RNG from Phobos)? Anyway, OS-specific solutions are hardly acceptable for vibe.d

[ilya-stromberg]

Yes, I am sure.

By default, uniform use MersenneTwisterEngine:

alias MersenneTwisterEngine!(uint, 32, 624, 397, 31, 0x9908b0df, 11, 7, 0x9d2c5680, 15, 0xefc60000, 18) Mt19937;
alias Mt19937 Random;
auto uniform(string boundaries = "[)", T1, T2)
(T1 a, T2 b) if (!is(CommonType!(T1, T2) == void))
{
    return uniform!(boundaries, T1, T2, Random)(a, b, rndGen);
}

the Mersenne Twister is NOT cryptography secure: "The algorithm in its native form is not suitable for cryptography (unlike Blum Blum Shub). Observing a sufficient number of iterations (624 in the case of MT19937, since this is the size of the state vector from which future iterations are produced) allows one to predict all future iterations." http://en.wikipedia.org/wiki/Mersenne_twister

There have been series of random number generator security related patches if Phobos few months ago.

I didn't know about this. What exactly was added?

Anyway, OS-specific solutions are hardly acceptable for vibe.d

As I said, we can use OpenSSL.

[ilya-stromberg]

I look at the CryptGenRandom function on Windows. It should be easy to use it. So, I can create a cross-platform pull if you want.

[s-ludwig]

There have been some bad news about the Windows RNG if I remember right. How about combining the platform generator with the OpenSSL one (XOR)? If we make it secure, let's try to minimize the chance that some implementation hole affects the outcome.

BTW regarding security, the highly naiive generateSimplePasswordHash really needs to be deprecated/removed.

[ilya-stromberg]

There have been some bad news about the Windows RNG if I remember right.

Yes, Windows RNG have problems with Windows 2000 and Windows XP. Fixed for Windows XP Service Pack 3 and Windows Vista: "A 2007 paper from Hebrew University suggested security problems in the Windows 2000 implementation of CryptGenRandom (assuming the attacker has control of the machine). Microsoft later acknowledged that the same problems exist in Windows XP, but not in Vista. Microsoft released a fix for the bug with Windows XP Service Pack 3 in mid-2008." http://en.wikipedia.org/wiki/CryptGenRandom

How about combining the platform generator with the OpenSSL one (XOR)?

Yes, it should work. Another idea - use a cryptography secure hash function for RNG result (like SHA-256 or maybe SHA-1, but not MD5). We can combine both XOR and hash methods (only for real paranoiacs).

[ilya-stromberg]

Add pull #352 for cryptography secure random generator support. If it's OK, I will fix vibe.http.session.

[ilya-stromberg]

Add a hash-based cryptographically secure random number generator. I use SHA1 because std.digest doesn't provide any alternatives. It should be secure enough. @s-ludwig, you can implement OpenSSL-based random number generator yourself if you want to do it. I never used OpenSSL, sorry.

[s-ludwig]

@ilya-stromberg: I would appreciate if you'd adjust the session code, but let's defer that until after merging #352. I'll look into the OpenSSL stuff at some point, but currently my time is severely lacking (I've never used OpenSSL much apart from the SSL/TLS code, too).

[ilya-stromberg]

@s-ludwig, can we use Tango library? I will be happy to use Whirlpool or SHA-512, but for vibe.http.session it's not so critical.

BTW regarding security, the highly naiive generateSimplePasswordHash really needs to be deprecated/removed.

We can use Whirlpool or SHA-512 with salt (yes, std.digest looks like bad joke). I'll be happy to avoid SHA-1 for this purpose because it was hacked in theory.

Another idea: we can use scrypt function. We can take bindings to the Tarsnap library here: https://github.com/BitPuffin/scrypt.d It's the most security algorithm for passwords that I know. But in that case we can have distribution problems, especially for Windows.

Any ideas?

[s-ludwig]

Regarding Tango, there should not be any additional dependencies apart from Phobos and C bindings. OpenSSL could also be used for SHA-512, but it would be ideal to get an implementation into Phobos instead.

Instead of just a secure hash function for generateSimplePasswordHash, at least something like HMAC should be used to increase the computational costs, because even with a secure hash function is used, it may be too easy to perform a brute force plain text attack.

scrypt would be nice, but having an additional external dependency is indeed an issue. It's important not to complicate the overall installation process. I wonder how difficult it is to port only the scrypt part of libtarsnap to D...

[ilya-stromberg]

but it would be ideal to get an implementation into Phobos instead.

I know, but haven't got time to do it.

at least something like HMAC should be used to increase the computational costs

HMAC should be easy to implement, but it's almost same to the H(H(password) + salt) where salt has digest length (512 bit for SHA-512). Probably, it's better to use something like this:

key = H(password + salt)
foreach(i; 1..65536)
{
   key = H(key + password + salt)
}

it may be too easy to perform a brute force plain text attack

Not so easy if you use secure hash function and long salt. But MD5 can be cracked less than 1 minute due algorithm issues. That's why I don't want to use SHA-1: they have a lot of common.

I wonder how difficult it is to port only the scrypt part of libtarsnap to D

Don't know. We need: SHA-256, HMAC and Mixing Function based on Salsa20/8. Tango has SHA-256 and Salsa20/20, so probably we need just combine it.

[s-ludwig]

Sorry, I confused HMAC with PBKDF2. But regarding brute force crack, it is still easy with a plain salt+hash if the password is relatively short, because the hash function is so fast to compute (using HW support at least).

The libtarsnap code looks at least like it will be well portable due to its plain C nature and from a quick look, it seems like everything except SHA-256 is contained in the crypto_scrypt_nosse.c file (340 lines of code).

[ilya-stromberg]

Yes, PBKDF2 is better than HMAC, but I don't know any D implementation. So, if we haven't got any D PBKDF2 implementation it's, probably, better to port scrypt.

[ilya-stromberg]

@s-ludwig, I add fix for vibe.http.session in #364. Is it OK?

[ilya-stromberg]

@s-ludwig, have we got any other places were std.random.uniform was used?

[s-ludwig]

Only for the simplePasswordHash stuff (which obviously has worse issues) and for BsonObjectID, which has no security concerns.