Libguestfs - Nov 2013 - Re: make_random_password(): avoid modulo bias, and do not deplete system entropy (#9)

On Thu, Nov 14, 2013 at 01:48:17AM -0800, edwintorok
wrote:
> Following the link to builder.ml from your blogpost I noticed the
make_random_password () function, and I have some suggestions, well nitpicks
really. See the 2 commits from this pull request.
> 
> 1. Using Ocaml's buffered I/O means that one make_random_password()
call reads 64k bytes from /dev/urandom which drops the system's entropy
to minimum ~128 bits. The fix is to use unbuffered I/O from the Unix module.
> 
> 2. There is a modulo bias when generating the random password: [0,256) mod
60 won't give you uniformly distributed bytes. In this particular case
it probably doesn't matter, but you never know when someone copy+pastes
your code into their project thinking this is a proper way to generate random
passwords, so IMHO its best to avoid the modulo bias.
> See here for more details:
http://eternallyconfuzzled.com/arts/jsw_art_rand.aspx
> And see arc4random_uniform's implementation:
http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/crypt/arc4random.c?rev=1.26;content-type=text%2Fplain
> 
> 3. The generated password needs ~2^107 brute-force attempts (16 * log2(60)
+ log2(default_rounds=5000)), which is more than enough of course, but usually
128-bit strength is used for keys. A password length of 20 characters would
achieve that. My pull request doesn't include this change, its up to
you.
> You can merge this Pull Request by running:
> 
>   git pull https://github.com/edwintorok/libguestfs
make_random_password-fixes
> 
> Or you can view, comment on it, or merge it online at:
> 
>   https://github.com/libguestfs/libguestfs/pull/9
> 
> -- Commit Summary --
> 
>   * Do not use buffered reads on /dev/urandom
>   * Avoid modulo bias in random password generation
> 
> -- File Changes --
> 
>     M builder/builder.ml (22)
> 
> -- Patch Links --
> 
> https://github.com/libguestfs/libguestfs/pull/9.patch
> https://github.com/libguestfs/libguestfs/pull/9.diff
I'm posting this patch to the list for review.  Please follow
up there as we don't use github pull requests.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

On Thu, Nov 14, 2013 at 01:48:17AM -0800, edwintorok
wrote:
> Following the link to builder.ml from your blogpost I noticed the
> make_random_password () function, and I have some suggestions, well
> nitpicks really. See the 2 commits from this pull request.
>
> 1. Using Ocaml's buffered I/O means that one
> make_random_password() call reads 64k bytes from /dev/urandom which
> drops the system's entropy to minimum ~128 bits. The fix is to
> use unbuffered I/O from the Unix module.
>
> 2. There is a modulo bias when generating the random password:
> [0,256) mod 60 won't give you uniformly distributed bytes. In
> this particular case it probably doesn't matter, but you never
> know when someone copy+pastes your code into their project thinking
> this is a proper way to generate random passwords, so IMHO its best
> to avoid the modulo bias.  See here for more details:
> http://eternallyconfuzzled.com/arts/jsw_art_rand.aspx And see
> arc4random_uniform's implementation:
>
http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/crypt/arc4random.c?rev=1.26;content-type=text%2Fplain
Thanks.  I have modified the patches a fair bit because it turns out
there are 2 other places where we make the same mistake.  The result
is this:

https://github.com/libguestfs/libguestfs/commit/f013b15fa1daf623cbf39007182c44c734e9bc79
https://github.com/libguestfs/libguestfs/commit/6a1061663fa467d43777a8ed98f4f07750125012
> 3. The generated password needs ~2^107 brute-force attempts (16 *
> log2(60) + log2(default_rounds=5000)), which is more than enough of
> course, but usually 128-bit strength is used for keys. A password
> length of 20 characters would achieve that. My pull request
> doesn't include this change, its up to you.
It could be worth making this change.  On the other hand it can be
easily mitigated if people supply the password, and the 16 character
random password is hard enough to type as it is -- a nice example of
the trade-off between security vs usability :-(

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)