-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bad random session ids due to bad Rand::random randomness #69
Comments
Keep in mind that randomness comes with entropy, and it is hardly retrived in a computer system. The bits of randomness are not in par with bits of cryptography strength, and don't need to be. They just need to be unguessable! Notice: This message is digitally signed, its source and integrity are verifiable. On Sep 17, 2013, at 4:16 PM, SvenRtbg notifications@github.com wrote:
|
If the basic randomness can only generate 31 bits of different values, Even though the hashing hides that very poor randomness, you will still Compare that to the possible 1,3407807929942597099574024998206e+154 Another reference, which is talking about the cracking of 56-bit keys I am a little bit concerned about your answer here. 31 bits of Am 17.09.2013 23:29, schrieb AbiusX:
|
Well you need to bruteforce a weak randomness on the server, not on your own supercomputer :D That is the difference. Notice: This message is digitally signed, its source and integrity are verifiable. On Sep 17, 2013, at 5:43 PM, SvenRtbg notifications@github.com wrote:
|
So what would be lost if we would actually use some maximum amount of Am 17.09.2013 23:44, schrieb AbiusX:
|
Hello Sven, Now the test cases you sent obviously fails and proves the point...so
On Tue, Sep 17, 2013 at 5:46 PM, SvenRtbg notifications@github.com wrote:
Regards, |
SHA512 produces a hash string of 128 hex characters. On hex character represents 4 bits, so 128 hex characters are 64 bytes. So SHA512 can have the whole range of 64 bytes of different values - which is about 2^(64*8) or 2^512. What to do? First, get a clear picture of what this function should return: It sounds like "random string of length X" - what does this include?
For optimal randomness, the return string should contain as many different characters as possible. But it should also return a feasible selection of characters for a purpose like random password generation. But even if it will only output hex, this is fine, but should be taken into account by the developer using it, because he must then ask for more characters to get the same amount of possible different values. Now the random generation: I'd really just copy the approach from the password API I linked. That part will actually produce any number of random characters - and if used directly by |
Those links maybe useful |
Added testcase for random string generation. (Issue #69)
I think the was that session ids are generated is not very random. And this is concealed because of a layer of function calls that all look very good. Please follow me on the code path:
Let's start at Session::newSession https://github.com/OWASP/phpsec/blob/master/libs/session/session.php#L101
As you can see, there is a call to the function
random(32)
, which promises to deliver 32 random characters.That function https://github.com/OWASP/phpsec/blob/master/libs/core/random.php#L25 is only a simple proxy and forwards the call to Rand::randStr(32).
Now that function https://github.com/OWASP/phpsec/blob/master/libs/core/random.php#L109 actually does something: It grabs some randomness, hashes it into a string of finite length (which would lead to the whole functions so far to NOT deliver e.g. a random string of 2048 characters without hint or error - bad in itself, but not my point).
Ok, randomness... We do see a call to Rand::randRange() https://github.com/OWASP/phpsec/blob/master/libs/core/random.php#L87 with default parameters. This function does some checks, probably reverses
$min
and$max
, grabs some randomness and then simply scales that into the range given. The range for default parameters is $min=0, $max = 2^31. So whatever the random generator delivers, it will here be reduced to 31 bits of randomness.And the random number generator is in Rand::random() https://github.com/OWASP/phpsec/blob/master/libs/core/random.php#L47 itself has no parameters. In an ideal situation, the very good random function
openssl_random_pseudo_bytes()
would be asked for FOUR bytes of randomness, which equals 32 bits. In absence of this function, the very bad random generatormt_rand()
would only issue 31 bits of randomness.All in all, the session ids generated by this code will only contain 31 bits of randomness, packed into a nice looking substring of SHA512, suggesting much more randomness being included (SHA512 itself is 128 hex chars long, leaving room for 64 bytes of randomness instead of only 4 bytes).
Even the PHP default session id hash has
/* maximum 15+19+19+10 bytes */
= 63 bytes of pseudo-randomness and not-that-easy-to-guess information - and this is criticized as "attackable" by some. (see https://github.com/php/php-src/blob/master/ext/session/session.c#L309)What are good sources of randomness? I think a good example is given in the random salt generation of the password_hash library: https://github.com/ircmaxell/password_compat/blob/master/lib/password.php#L84
Plenty of sources are being asked, in the hope to find some good randomness:
mcrypt_create_iv()
,openssl_random_pseudo_bytes()
,fopen('/dev/urandom', 'r')
- and only then, as a last resort,mt_rand()
with as many calls as there should be bytes in the salt string.The text was updated successfully, but these errors were encountered: