From a0a9560258cef3fa7dcd16e5f24eb087867641a0 Mon Sep 17 00:00:00 2001
From: Miroslav Lichvar <mlichvar@redhat.com>
Date: Tue, 23 Nov 2021 13:17:26 +0100
Subject: [PATCH] util: reset GetRandom functions in helpers after fork

Close /dev/urandom and drop cached getrandom() data after forking helper
processes to avoid them getting the same sequence of random numbers
(e.g. two NTS-KE helpers generating cookies with identical nonces).
arc4random() is assumed to be able to detect forks and reseed
automatically.

This is not strictly necessary with the current code, which does not use
the GetRandom functions before the NTS-KE helper processes are forked,
but that could change in future.

Also, call the reset function before exit to close /dev/urandom in order
to avoid valgrind reporting the file object as "still reachable".
---
 main.c           |  2 ++
 nts_ke_server.c  |  4 ++++
 privops.c        |  2 ++
 test/unit/util.c | 10 ++++++++++
 util.c           | 35 ++++++++++++++++++++++++++---------
 util.h           |  4 ++++
 6 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/main.c b/main.c
index 74e27067..355cdc3a 100644
--- a/main.c
+++ b/main.c
@@ -141,6 +141,8 @@ MAI_CleanupAndExit(void)
   HSH_Finalise();
   LOG_Finalise();
 
+  UTI_ResetGetRandomFunctions();
+
   exit(exit_status);
 }
 
diff --git a/nts_ke_server.c b/nts_ke_server.c
index 12396788..ece1b4c0 100644
--- a/nts_ke_server.c
+++ b/nts_ke_server.c
@@ -669,6 +669,8 @@ run_helper(uid_t uid, gid_t gid, int scfilter_level)
   CNF_Finalise();
   LOG_Finalise();
 
+  UTI_ResetGetRandomFunctions();
+
   exit(0);
 }
 
@@ -710,6 +712,8 @@ NKS_PreInitialise(uid_t uid, gid_t gid, int scfilter_level)
 
     is_helper = 1;
 
+    UTI_ResetGetRandomFunctions();
+
     snprintf(prefix, sizeof (prefix), "nks#%d:", i + 1);
     LOG_SetDebugPrefix(prefix);
     LOG_CloseParentFd();
diff --git a/privops.c b/privops.c
index 99de867d..3fb5cbd5 100644
--- a/privops.c
+++ b/privops.c
@@ -662,6 +662,8 @@ PRV_StartHelper(void)
         close(fd);
     }
 
+    UTI_ResetGetRandomFunctions();
+
     /* ignore signals, the process will exit on OP_QUIT request */
     UTI_SetQuitSignalsHandler(SIG_IGN, 1);
 
diff --git a/test/unit/util.c b/test/unit/util.c
index 4d7a4e8c..efd9c5a1 100644
--- a/test/unit/util.c
+++ b/test/unit/util.c
@@ -670,6 +670,10 @@ test_unit(void)
     UTI_GetRandomBytesUrandom(buf, j);
     if (j && buf[j - 1] % 2)
       c++;
+    if (random() % 10000 == 0) {
+      UTI_ResetGetRandomFunctions();
+      TEST_CHECK(!urandom_file);
+    }
   }
   TEST_CHECK(c > 46000 && c < 48000);
 
@@ -678,6 +682,12 @@ test_unit(void)
     UTI_GetRandomBytes(buf, j);
     if (j && buf[j - 1] % 2)
       c++;
+    if (random() % 10000 == 0) {
+      UTI_ResetGetRandomFunctions();
+#if HAVE_GETRANDOM
+      TEST_CHECK(getrandom_buf_available == 0);
+#endif
+    }
   }
   TEST_CHECK(c > 46000 && c < 48000);
 
diff --git a/util.c b/util.c
index 69f523d7..da99f827 100644
--- a/util.c
+++ b/util.c
@@ -1402,29 +1402,32 @@ UTI_DropRoot(uid_t uid, gid_t gid)
 
 #define DEV_URANDOM "/dev/urandom"
 
+static FILE *urandom_file = NULL;
+
 void
 UTI_GetRandomBytesUrandom(void *buf, unsigned int len)
 {
-  static FILE *f = NULL;
-
-  if (!f)
-    f = UTI_OpenFile(NULL, DEV_URANDOM, NULL, 'R', 0);
-  if (fread(buf, 1, len, f) != len)
+  if (!urandom_file)
+    urandom_file = UTI_OpenFile(NULL, DEV_URANDOM, NULL, 'R', 0);
+  if (fread(buf, 1, len, urandom_file) != len)
     LOG_FATAL("Can't read from %s", DEV_URANDOM);
 }
 
 /* ================================================== */
 
 #ifdef HAVE_GETRANDOM
+
+static unsigned int getrandom_buf_available = 0;
+
 static void
 get_random_bytes_getrandom(char *buf, unsigned int len)
 {
   static char rand_buf[256];
-  static unsigned int available = 0, disabled = 0;
+  static unsigned int disabled = 0;
   unsigned int i;
 
   for (i = 0; i < len; i++) {
-    if (!available) {
+    if (getrandom_buf_available == 0) {
       if (disabled)
         break;
 
@@ -1433,10 +1436,10 @@ get_random_bytes_getrandom(char *buf, unsigned int len)
         break;
       }
 
-      available = sizeof (rand_buf);
+      getrandom_buf_available = sizeof (rand_buf);
     }
 
-    buf[i] = rand_buf[--available];
+    buf[i] = rand_buf[--getrandom_buf_available];
   }
 
   if (i < len)
@@ -1460,6 +1463,20 @@ UTI_GetRandomBytes(void *buf, unsigned int len)
 
 /* ================================================== */
 
+void
+UTI_ResetGetRandomFunctions(void)
+{
+  if (urandom_file) {
+    fclose(urandom_file);
+    urandom_file = NULL;
+  }
+#ifdef HAVE_GETRANDOM
+  getrandom_buf_available = 0;
+#endif
+}
+
+/* ================================================== */
+
 int
 UTI_BytesToHex(const void *buf, unsigned int buf_len, char *hex, unsigned int hex_len)
 {
diff --git a/util.h b/util.h
index f6bf7b90..a57ef9ba 100644
--- a/util.h
+++ b/util.h
@@ -224,6 +224,10 @@ extern void UTI_GetRandomBytesUrandom(void *buf, unsigned int len);
    generating long-term keys */
 extern void UTI_GetRandomBytes(void *buf, unsigned int len);
 
+/* Close /dev/urandom and drop any cached data used by the GetRandom functions
+   to prevent forked processes getting the same sequence of random numbers */
+extern void UTI_ResetGetRandomFunctions(void);
+
 /* Print data in hexadecimal format */
 extern int UTI_BytesToHex(const void *buf, unsigned int buf_len, char *hex, unsigned int hex_len);
 
-- 
2.47.3