From 1db190cb66339cd2c052ab2f868410455e215b8a Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Fri, 30 Dec 2022 19:46:09 +0100 Subject: [PATCH] Rewrite csrand_interval() as a wrapper around csrand_uniform() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old code didn't produce very good random numbers. It had a bias. And that was from performing some unnecessary floating-point calculations that overcomplicate the problem. Cc: "Jason A. Donenfeld" Cc: Cristian Rodríguez Cc: Adhemerval Zanella Cc: Björn Esser Cc: Yann Droneaud Cc: Joseph Myers Cc: Sam James Signed-off-by: Alejandro Colomar --- lib/prototypes.h | 1 + libmisc/csrand.c | 10 ++++++++++ libmisc/salt.c | 27 --------------------------- 3 files changed, 11 insertions(+), 27 deletions(-) diff --git a/lib/prototypes.h b/lib/prototypes.h index 722d6d98..03870ad7 100644 --- a/lib/prototypes.h +++ b/lib/prototypes.h @@ -359,6 +359,7 @@ extern void pw_free (/*@out@*/ /*@only@*/struct passwd *pwent); /* csrand.c */ unsigned long csrand (void); unsigned long csrand_uniform (unsigned long n); +unsigned long csrand_interval (unsigned long min, unsigned long max); /* remove_tree.c */ extern int remove_tree (const char *root, bool remove_root); diff --git a/libmisc/csrand.c b/libmisc/csrand.c index 0cc99972..f06b9579 100644 --- a/libmisc/csrand.c +++ b/libmisc/csrand.c @@ -85,3 +85,13 @@ csrand_uniform(unsigned long n) return r; } + + +/* + * Return a uniformly-distributed CS random value in the interval [min, max]. + */ +unsigned long +csrand_interval(unsigned long min, unsigned long max) +{ + return csrand_uniform(max - min + 1) + min; +} diff --git a/libmisc/salt.c b/libmisc/salt.c index c3f3d23a..103fb1cf 100644 --- a/libmisc/salt.c +++ b/libmisc/salt.c @@ -89,9 +89,6 @@ #if !USE_XCRYPT_GENSALT static /*@observer@*/const char *gensalt (size_t salt_size); #endif /* !USE_XCRYPT_GENSALT */ -#if defined(USE_SHA_CRYPT) || defined(USE_BCRYPT) -static unsigned long csrand_interval (unsigned long min, unsigned long max); -#endif /* USE_SHA_CRYPT || USE_BCRYPT */ #ifdef USE_SHA_CRYPT static /*@observer@*/unsigned long SHA_get_salt_rounds (/*@null@*/const int *prefered_rounds); static /*@observer@*/void SHA_salt_rounds_to_buf (char *buf, unsigned long rounds); @@ -106,30 +103,6 @@ static /*@observer@*/void YESCRYPT_salt_cost_to_buf (char *buf, unsigned long co #endif /* USE_YESCRYPT */ -#if defined(USE_SHA_CRYPT) || defined(USE_BCRYPT) -/* - * Return a random number between min and max (both included). - * - * It favors slightly the higher numbers. - */ -static unsigned long csrand_interval (unsigned long min, unsigned long max) -{ - double drand; - long ret; - - drand = (double) (csrand () & RAND_MAX) / (double) RAND_MAX; - drand *= (double) (max - min + 1); - /* On systems were this is not random() range is lower, we favor - * higher numbers of salt. */ - ret = (long) (max + 1 - drand); - /* And we catch limits, and use the highest number */ - if ((ret < min) || (ret > max)) { - ret = max; - } - return ret; -} -#endif /* USE_SHA_CRYPT || USE_BCRYPT */ - #ifdef USE_SHA_CRYPT /* Return the the rounds number for the SHA crypt methods. */ static /*@observer@*/unsigned long SHA_get_salt_rounds (/*@null@*/const int *prefered_rounds)