Tom Collins wrote:

On Tuesday, November 4, 2003, at 05:40 AM, Nick Harring wrote:

Actually, this is already a right place to put this, which is in randltr. Oddly that's what's used for generating the salt, but not what's used for generating the password. Instead the password just uses an ugly rand call.
I'd change vgen_pass to do this:


for (i = 0; i < len; i++) {
     k = randltr();
     p[i] = gen_chars[k];
}
return p;


randltr selects from 64 valid salt characters. The password generator pulls from a larger selection of possible characters.

I'd even consider modifying the random password generator to not use letters that can be confused with each other (1/I/l and 0/O).

That'd be foolish in the extreme. If the user, or administrator, wants passwords that are "easy to remember" or "hard to confuse" then they should take the onerous burden on themselves. The random password function, by using the word random, is promising a certain functionality and what you propose doesn't deliver it. Narrowing the possible scope for each letter to 64 from some larger group but increasing the entropy that goes into selecting each character seems like a good idea to me.
The valid salt chars for DES, with which we must maintain backwards compatability, are [a-zA-Z0-9./]. The additional characters that vgen_pass() can be using are [EMAIL PROTECTED]&*()-_=+\|]}[{"';:><, by my count that's 29 new characters. A decently larger set, but not substantially so.
This means that an 8 character password, which is what gets created when vpopmail creates a random password for you, can be one of either 6.27710173538668e+57 or 9.71334446112865e+83 combinations, depending on how many characters you allow. I personally think we should lean towards higher entropy within a smaller field rather than dramatically lower entropy within a somewhat larger field. I do not, however, have any evidence to back this up at the moment. If you'd like me to back the above up, or withdraw it, based on an actual expert opinion I'd be more than happy to do some research this evening to see what the "pro's" say.
We could also increase the size of the MD5 salt, since we're allowed 8 characters plus the leading $1$ and an optional, terminating $. This'd be easy to do, backwards compatible, and ought to increase security to at least some extent.




Also, randltr is relying on something else to seed srand, which is really a bad idea. One mistake and suddenly everyone's vpopmail everywhere is seeding with 1. Oops.


randltr is only used by mkpasswd3() which seeds srandom. You need to be careful to seed rand/random only once.

There's no real reason that I can determine, from either the man pages, my experience, or the experience of the people around me that says you need to be careful to seed the random pool only once. It doesn't break anything. At worst its inefficient.
If you are so terribly worried you can waste 1 byte of memory with a flag indicating that srand has been called and thus not do it again. Since you've not yet done that, but instead relied on nobody calling mkpasswd3() AND vgen_pass() in a series, I'm guessing you weren't that worried.




I agree that we should use /dev/urandom (or /dev/random) if available. The code should read in enough bytes to generate an entire salt or random password (however the case may be). I'm willing to explore adding this to the next development cycle. Right now, I want to get a 5.3.30 release done (and maybe even call it 5.4.0-pre1) so we can have a stable release for people who've been waiting to upgrade from 5.2.2.

A stable release would be a Good Thing. I'm one of those people waiting to upgrade, however I'd consider this a fairly serious, albeit still theoretical, bug in the password generating and hashing functions.




I'll still say that I think this is overkill. I find it extremely unlikely that someone could determine the random password generated by vpopmail.

People found it extremely unlikely that anyone could brute force DES crypted passwords for a long time. Then someone did it and everyone moved to MD5. Remember, the crypted password used to be kept in /etc/passwd, which was world readable, since no one worried about it.
Shrugging off easily fixable bugs as theoretical keeps getting people into trouble, and yet everyone keeps trying to shrug them off. Even worse, shrugging them off publicly seems to invite people to prove you wrong.
As far as I'm concerned, when it comes to security the line between enough and overkill is very very blurry. I usually draw it when something is "measurably" secure AND the workload to continue making it more secure begins going up dramatically. In this case I just don't see the workload going up by very much, but the level of security would seem to be going up dramatically.




--
Tom Collins  -  [EMAIL PROTECTED]
Note: The Tom Logic offices will be closed October 23 to November 18.
QmailAdmin: http://qmailadmin.sf.net/  Vpopmail: http://vpopmail.sf.net/
Info on the Sniffter hand-held Network Tester: http://sniffter.com/

Cheers,
Nick Harring
Webley Systems



Reply via email to