Remove minor biasing problem from crypto_pseudo_rand_int

svn:r799

2 files changed, 16 insertions, 8 deletions

diff --git a/src/common/crypto.c b/src/common/crypto.c
index afec91d22..551af8964 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -16,6 +16,7 @@
 #include <stdlib.h>
 #include <assert.h>
 #include <stdio.h>
+#include <limits.h>
 
 #include "crypto.h"
 #include "../or/or.h"
@@ -1008,14 +1009,21 @@ void crypto_pseudo_rand(unsigned int n, unsigned char *to)
   }
 }
 
-int crypto_pseudo_rand_int(int max) {
+int crypto_pseudo_rand_int(unsigned int max) {
   unsigned int val;
-  crypto_pseudo_rand(sizeof(val), (unsigned char*) &val);
-  /* Bug: Low values are _slightly_ favored over high values because
-   * ((unsigned)-1)%max != max-1 .  This shouldn't matter if max is
-   * significantly smaller than ((unsigned)-1).
-   **/
-  return val % max;
+  unsigned int cutoff;
+  assert(max < UINT_MAX);
+
+  /* We ignore any values that are >= 'cutoff,' to avoid biasing the
+   * distribution with clipping at the upper end of unsigned int's
+   * range.
+   */
+  cutoff = UINT_MAX - (UINT_MAX%max);
+  while(1) {
+    crypto_pseudo_rand(sizeof(val), (unsigned char*) &val);
+    if (val < cutoff)
+      return val % max;
+  }
 }
 
 /* errors */
diff --git a/src/common/crypto.h b/src/common/crypto.h
index ab5422d8c..0d8257fd9 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -101,7 +101,7 @@ int crypto_SHA_digest(unsigned char *m, int len, unsigned char *digest);
 int crypto_seed_rng();
 int crypto_rand(unsigned int n, unsigned char *to);
 void crypto_pseudo_rand(unsigned int n, unsigned char *to);
-int crypto_pseudo_rand_int(int max);
+int crypto_pseudo_rand_int(unsigned int max);
 
 /* errors */
 char *crypto_perror();