From 79e8defa97b974b1c3f63701c0a3411a958c5499 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?T=C3=B6r=C3=B6k=20Edwin?= <edwin@etorok.net>
Date: Thu, 14 Nov 2013 11:13:50 +0200
Subject: [PATCH] Avoid modulo bias in random password generation

Char.code (input_char chan) mod nr_chars has modulo bias because
the original interval is not a multiple of the destination interval,
i.e. 256 mod nr_chars != 0.

One way to fix this is to keep generating random numbers until they fall outside
the interval where modulo bias occurs, that is accept only c=[256 % nr_chars, 256).
That interval maps back to [0, nr_chars), and has a length of
(256 - 256 % nr_chars), which is a multiple of nr_chars.

RWMJ:
 - Modify the code so it goes into a utility library.
 - Use the same code across virt-builder and virt-sysprep.

(cherry picked from commit 6a1061663fa467d43777a8ed98f4f07750125012)
---
 builder/builder.ml | 11 +----------
 mllib/password.ml  | 10 +---------
 mllib/urandom.ml   | 21 +++++++++++++++++++++
 mllib/urandom.mli  |  4 ++++
 4 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/builder/builder.ml b/builder/builder.ml
index 84aa869..15f920c 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -454,16 +454,7 @@ let main () =
        *)
       let chars =
         "ABCDEFGHIJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz0123456789" in
-      let nr_chars = String.length chars in
-
-      let chan = open_in "/dev/urandom" in
-      let buf = String.create 16 in
-      for i = 0 to 15 do
-        buf.[i] <- chars.[Char.code (input_char chan) mod nr_chars]
-      done;
-      close_in chan;
-
-      buf
+      Urandom.urandom_uniform 16 chars
     in
 
     let root_password =
diff --git a/mllib/password.ml b/mllib/password.ml
index 50c0683..a7a97b6 100644
--- a/mllib/password.ml
+++ b/mllib/password.ml
@@ -57,7 +57,6 @@ and read_password_from_file filename =
 
 (* Permissible characters in a salt. *)
 let chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789./"
-let nr_chars = String.length chars
 
 let rec set_linux_passwords ~prog ?password_crypto g root passwords =
   let crypto =
@@ -95,14 +94,7 @@ let rec set_linux_passwords ~prog ?password_crypto g root passwords =
  *)
 and encrypt password crypto =
   (* Get random characters from the set [A-Za-z0-9./] *)
-  let salt =
-    let chan = open_in "/dev/urandom" in
-    let buf = String.create 16 in
-    for i = 0 to 15 do
-      buf.[i] <- chars.[Char.code (input_char chan) mod nr_chars]
-    done;
-    close_in chan;
-    buf in
+  let salt = Urandom.urandom_uniform 16 chars in
   let salt =
     (match crypto with
     | `MD5 -> "$1$"
diff --git a/mllib/urandom.ml b/mllib/urandom.ml
index 3df9b7a..9b613e8 100644
--- a/mllib/urandom.ml
+++ b/mllib/urandom.ml
@@ -46,3 +46,24 @@ let urandom_bytes n =
   done;
   close fd;
   ret
+
+(* Return a random number uniformly distributed in [0, upper_bound)
+ * avoiding modulo bias.
+ *)
+let rec uniform_random read upper_bound =
+  let c = read () in
+  if c >= 256 mod upper_bound then c mod upper_bound
+  else uniform_random read upper_bound
+
+let urandom_uniform n chars =
+  assert (n > 0);
+  let nr_chars = String.length chars in
+  assert (nr_chars > 0);
+
+  let ret = String.make n ' ' in
+  let fd = open_urandom_fd () in
+  for i = 0 to n-1 do
+    ret.[i] <- chars.[uniform_random (read_byte fd) nr_chars]
+  done;
+  close fd;
+  ret
diff --git a/mllib/urandom.mli b/mllib/urandom.mli
index a7d7808..ffc77dd 100644
--- a/mllib/urandom.mli
+++ b/mllib/urandom.mli
@@ -20,3 +20,7 @@
 
 val urandom_bytes : int -> string
 (** Read N bytes from /dev/urandom and return it as a binary string. *)
+
+val urandom_uniform : int -> string -> string
+(** [urandom_uniform n chars] returns [n] bytes, uniformly
+    distributed from the sets of characters [chars]. *)
-- 
1.8.3.1