Nick Harring wrote:

This is the Right Thing imho. It might be easier though to move the srandom()/random() and new reads from /dev/urandom into a function of its own, rather than replacing them whereever they're sprinkled through the code. I realize that's even more work, but its probably more maintainable down the road.

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;

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.

I'd like to see a randltr similar to:

char randltr(void)
{
char rand;
char retval = 'a';
#ifdef HAS_URANDOM
int fh;
char entropy[1];
char path[] = "/dev/urandom";
fh = open(path,O_RDONLY);
read(fh,*entropy,1);
rand = entropy[1];
#endif
#ifdef NO_URANDOM
srandom(time(NULL) ^ (getpid() + (getpid() << 15)));
rand = random() % 64;

   if (rand < 26) retval = rand + 'a';
   if (rand > 25) retval = rand - 26 + 'A';
   if (rand > 51) retval = rand - 52 + '0';
   if (rand == 62) retval = ';';
   if (rand == 63) retval = '.';
   return retval;
}

I strongly discourage using my code verbatim, however I think it conveys the general idea. Someone who's better with C can certainly fix any errors and clean it up to fit the general vpopmail style.



Cheers,
Nick Harring
Webley Systems






Reply via email to