Re: [PATCH 1/3] Use LFSR113 PRNG for mutt's internal random needs

[Kevin J. McCarthy]

On Sun, May 24, 2020 at 04:59:24PM -0400, Remco Rijnders wrote:

By implementing the LFSR113 function by Pierre L'Ecuyer mutt gets a fast
and high quality PRNG that, given the same seeds, results in the same
output no matter the environment mutt is running on.

More comments below.

I'm not knowledgeable about PRNGs. The generation code below looks the same as other LFSR113 generators I searched around for, but I can't vouch for how "good" the generator itself is. Still, I'm inclined to apply the patch (with fixes) so please chime in if you think this is a bad idea.

diff --git a/configure.ac b/configure.ac
index 7906ce35..0f65fc56 100644
--- a/configure.ac
+++ b/configure.ac
@@ -34,7 +34,6 @@ AC_PROG_CPP
AC_PROG_MAKE_SET
AC_PROG_INSTALL
AC_PROG_MKDIR_P
-AC_PROG_RANLIB
AC_CHECK_TOOL(AR, ar, ar)

Ranlib is a library tool, and is still used (in some way) in the intl and m4 directories. Regardless, this kind of change should not be in this commit (unless it was a mistaken assumption that it meant "random library" or something).

diff --git a/mutt_random.c b/mutt_random.c

+/* Initialize the four seeds for our PRNG algorithm */
+void mutt_srandom(void)
+{
+  struct timeval tv;
+  gettimeofday(&tv, NULL);
+  /* POSIX.1-2008 states that seed is 'unsigned' without specifying its width.
+   * Use as many of the lower order bits from the current time of day as the 
seed.
+   * If the upper bound is truncated, that is fine.
+   *
+   * tv_sec is integral of type integer or float.  Cast to 'u_int32_t' before
+   * bitshift in case it is a float.
+   */
+  z1 = ((u_int32_t) tv.tv_sec << 20) | tv.tv_usec;
+  z2 = getpid();
+  z3 = getppid();
+  z4 = (intptr_t) &z4;
+}

Comments on the seed choices would be quite welcome. This first three seem okay to me, but the fourth choice is probably a bit weak. I don't know that it will vary much.

diff --git a/mutt_random.h b/mutt_random.h

+#include <sys/time.h>
+#include <sys/types.h>
+#include <string.h>
+#include <unistd.h>

Mutt traditionally puts these inside the .c file. So I'd prefer to see them moved there.

+extern void mutt_to_base64 (unsigned char*, const unsigned char*, size_t, 
size_t);

Include "mutt.h" inside mutt_random.c instead.

+void mutt_srandom(void);
+u_int32_t mutt_random32(void);
+void mutt_base64_random96(char output_B64[static 17]);
+
+u_int32_t z1, z2, z3, z4;

--
Kevin J. McCarthy
GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA

z1-z4 should be moved inside mutt_random.c and declared static. The latest gcc won't even compile duplicate global variable definitions.

signature.asc
Description: PGP signature