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".

diff --git a/main.c b/main.c
index 74e27067e0100e183ead02f184d301f9c473efd0..355cdc3a238162baa95e0ed85952c55c29bd4c1d 100644 (file)
--- 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 123967883d3c73c698a3d547e628169f27b22077..ece1b4c01b21f208c54f37f458d4425511394085 100644 (file)
--- 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 99de867dcf02a08463f81ba3a7f8a0da8d10a582..3fb5cbd5c47c485f248507b17183d598436deadc 100644 (file)
--- 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 4d7a4e8c7217b2a79bf893dc052f5d4ef411a3ff..efd9c5a176d04d2dd78ee8eb7f90a7054d18fe8a 100644 (file)
--- 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 69f523d7a5bc5d4e2efcd5b25cbb8c3372dab03d..da99f82739d7887f13d9dfa992578f26640e879d 100644 (file)
--- 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 f6bf7b908743e592a74805f94f5a25a1b7384a18..a57ef9ba75f6c3ccc54d5b4675d33b062c22e72a 100644 (file)
--- 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);