Increase salt-length

[GoogleCodeExporter]

I think the length of the used salt is pretty short (6 chars if I remember 
right).
I don't see any good reason why we use such a short salt and would propose to 
use a longer one (e.g. 32 chars).
I mean longer salts don't necessarily mean higher security, but as they are 
more or less for free, I don't see a good reason against using a longer one. 
Okay, cookies get some bytes larger and therefore http-requests, but come on... 
We don't even use lots of images or such, so we don't have a lot of HTTP 
requests anyway.

Any comments?

(This issue has been mentioned first in issue #170.)

Original issue reported on code.google.com by crazy4ch...@gmail.com on 9 Feb 2013 at 9:25

[GoogleCodeExporter]

As far as I know, the only good reason to use a longer salt (since we store it 
on the client anyway) is to keep the hashed 'salt+pass' string longer enough 
not to be found in precalculated hash/rainbow tables.

Given that we don't require a minimum password length, anything above 16 
characters should give us a few years of relative safety :-p  32 is definitely 
better, but I wouldn't overdo it and go for 20 or 24.

Original comment by dreadnaut on 10 Feb 2013 at 2:05

[GoogleCodeExporter]

Original comment by dreadnaut on 10 Feb 2013 at 2:25

• Added labels: Security

[GoogleCodeExporter]

Completely agree.

I used 32 as an example as lots of simple salt-generation algorithms use an md5 
of some random input. Of course MD5 hashs are in [0-9A-F]{32}, which means less 
possibilities (3,4e38) than alphanumeric [0-9A-Za-z]{22}, which makes 2,7e39. 
So the md5-approach wastes 10 characters of salt length.
And of course using other characters like special chars is a very good idea for 
salts and would easily make a 20 chars salt as good as a 32 char md5-salt (or 
even better).

So I'd say 20 is a good length given the fact that we are not securing fort 
knox here.

Original comment by crazy4ch...@gmail.com on 10 Feb 2013 at 3:51

[GoogleCodeExporter]

Something like this then?

$set = 'A....Za....z0....9';
$setLast = strlen($set) - 1;
$salt = '';
for ($saltSize = 20; $saltSize > 0; $saltSize--)
  $salt .= $set[mt_rand(0, $setLast)];

Original comment by dreadnaut on 11 Feb 2013 at 1:12

• Changed state: Accepted

[GoogleCodeExporter]

Yeah, this should do fine.

Original comment by crazy4ch...@gmail.com on 15 Feb 2013 at 2:37

[GoogleCodeExporter]

Patch against r338 attached.

Original comment by dreadnaut on 15 Feb 2013 at 11:16

Attachments:

• newsalt-r338.diff

[GoogleCodeExporter]

Looks good. Especially that you created a new static method that takes saltSize 
as parameter. If we continue like that, phpLiteAdmin might become a well 
oo-designed software one day ;-)

Please commit it.

Original comment by crazy4ch...@gmail.com on 16 Feb 2013 at 11:25

• Changed state: Started

[GoogleCodeExporter]

This issue was closed by revision r340.

Original comment by dreadnaut on 16 Feb 2013 at 11:49

• Changed state: Fixed

[GoogleCodeExporter]

One day :)

Committed as r340!

Original comment by dreadnaut on 16 Feb 2013 at 11:49