bug#31268: 'guix system vm-image' hangs

[ludo@gnu.org (Ludovic Courtès)]

[-- Attachment #1: Type: text/plain, Size: 2434 bytes --]

Hello!

(+Cc: Leo for randomness input.)

ludo@gnu.org (Ludovic Courtès) skribis:

> Clément Lassieur <clement@lassieur.org> skribis:
>
>> Mark H Weaver <mhw@netris.org> writes:
>
> [...]
>
>>> I suggest doing a git bisect on the 4.16.y branch of the linux-stable
>>> git repository, between the 4.16.3 and 4.16.4 tags.
>>
>> I did it, here is the result:
>
> Impressive, thanks a lot!  Did you have a script to do that or
> something?
>
>> cd8d7a5778a4abf76ee8fe8f1bfcf78976029f8d is the first bad commit
>> commit cd8d7a5778a4abf76ee8fe8f1bfcf78976029f8d
>> Author: Theodore Ts'o <tytso@mit.edu>
>> Date:   Wed Apr 11 13:27:52 2018 -0400
>>
>>     random: fix crng_ready() test
>>     
>>     commit 43838a23a05fbd13e47d750d3dfd77001536dd33 upstream.
>>     
>>     The crng_init variable has three states:
>>     
>>     0: The CRNG is not initialized at all
>>     1: The CRNG has a small amount of entropy, hopefully good enough for
>>        early-boot, non-cryptographical use cases
>>     2: The CRNG is fully initialized and we are sure it is safe for
>>        cryptographic use cases.
>>     
>>     The crng_ready() function should only return true once we are in the
>>     last state.  This addresses CVE-2018-1108.
>
> What happens I think is that libparted/fs/r/fat/fat.c:fat_create calls
> ‘generate_random_uint32’, which in turn indirectly calls
> ‘get_random_bytes’ from libuuid (package ‘e2fsprogs’).  And I suppose
> that’s where it blocks, even though it’s supposed to be using
> /dev/urandom, which is not supposed to block.

I was looking at the wrong code: we’re using libuuid from util-linux,
which in turn uses getrandom(2).  Since it doesn’t pass the
GRND_NONBLOCK flag, it ends up blocking forever because too little
entropy is available in the VM.

The following patches work around that:

  1. Parted now explicitly uses getrandom(2) with GRND_NONBLOCK instead
     of libuuid’s ‘uuid_generate’, which is good enough for this
     purpose.  I’ll submit it upstream.

  2. e2fsprogs is changed to use a libuuid that passes GRND_NONBLOCK.
     It does the job, but it’s quite inelegant.

Another approach I looked at was to seed the VM’s PRNG from /dev/hwrng,
which I thought was connected to the host via ‘virtio-rng-pci’, but I
get ENODEV while trying to read from /dev/hwrng in the guest like this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1008 bytes --]

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index b6561dfc7..3bfd6b4ca 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -319,6 +319,18 @@ the image."
 
            (set-path-environment-variable "PATH" '("bin" "sbin") inputs)
 
+           ;; Try writing from /dev/hwrng into /dev/urandom.
+           ;; It seems that the file /dev/hwrng always exists, even
+           ;; when there is no hardware random number generator
+           ;; available. So, we handle a failed read or any other error
+           ;; reported by the operating system.
+           (let ((buf (call-with-input-file "/dev/hwrng"
+                        (lambda (hwrng)
+                          (get-bytevector-n hwrng 512)))))
+             (call-with-output-file "/dev/urandom"
+               (lambda (urandom)
+                 (put-bytevector urandom buf))))
+
            (let* ((graphs     '#$(match inputs
                                    (((names . _) ...)
                                     names)))

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]


Am I missing something, Leo?

In short, we’re almost done with this bug, now we just need to make sure
we have a reasonable fix.

Thanks,
Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-gnu-e2fsprogs-Use-libuuid-with-non-blocking-random.patch --]
[-- Type: text/x-patch, Size: 3009 bytes --]

From 43357218f024b251fc2b741dc3e8bdc4c001051f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Tue, 22 May 2018 18:07:08 +0200
Subject: [PATCH 3/3] gnu: e2fsprogs: Use libuuid with non-blocking random.

Partly fixes <https://bugs.gnu.org/31268>.

* gnu/packages/linux.scm (util-linux/libuuid-non-blocking-random): New
variable.
(e2fsprogs)[inputs]: Use it instead of UTIL-LINUX.
---
 gnu/packages/linux.scm                          | 17 ++++++++++++++++-
 ...util-linux-libuuid-non-blocking-random.patch | 15 +++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/util-linux-libuuid-non-blocking-random.patch

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index bb4e00394..c93947e68 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -671,6 +671,20 @@ block devices, UUIDs, TTYs, and many other tools.")
                 (append (origin-patches (package-source util-linux))
                         (search-patches "util-linux-CVE-2018-7738.patch")))))))
 
+(define-public util-linux/libuuid-non-blocking-random
+  ;; Package variant where libuuid uses non-blocking (weak) random.  See
+  ;; <https://bugs.gnu.org/31268>.
+  (package
+    (inherit util-linux)
+    (name "util-linux-weak-random")
+    (source (let ((source (package-source util-linux)))
+              (origin
+                (inherit source)
+                (patches (append (search-patches
+                                  "util-linux-libuuid-non-blocking-random.patch")
+                                 (origin-patches source))))))
+    (properties '((hidden? . #t)))))
+
 (define-public ddate
   (package
     (name "ddate")
@@ -779,7 +793,8 @@ slabtop, and skill.")
               (base32
                "00ilv65dzcgiap435j89xk86shf7rrav3wsik7cahy789qijdcn9"))))
     (build-system gnu-build-system)
-    (inputs `(("util-linux" ,util-linux)))
+    ;; Arrange so that 'mke2fs' doesn't wait: <https://bugs.gnu.org/31268>.
+    (inputs `(("util-linux" ,util-linux/libuuid-non-blocking-random)))
     (native-inputs `(("pkg-config" ,pkg-config)
                      ("texinfo" ,texinfo)       ;for the libext2fs Info manual
 
diff --git a/gnu/packages/patches/util-linux-libuuid-non-blocking-random.patch b/gnu/packages/patches/util-linux-libuuid-non-blocking-random.patch
new file mode 100644
index 000000000..c0b3ea24d
--- /dev/null
+++ b/gnu/packages/patches/util-linux-libuuid-non-blocking-random.patch
@@ -0,0 +1,15 @@
+Change libuuid so that 'uuid_generate' is non-blocking.  This is needed when
+'mke2fs' is used during early boot in VMs with little entropy available.
+See <https://bugs.gnu.org/31268>.
+
+--- a/lib/randutils.c
++++ b/lib/randutils.c
+@@ -104,7 +104,7 @@ void random_get_bytes(void *buf, size_t nbytes)
+ 		int x;
+ 
+ 		errno = 0;
+-		x = getrandom(cp, n, 0);
++		x = getrandom(cp, n, GRND_NONBLOCK);
+ 		if (x > 0) {			/* success */
+ 		       n -= x;
+ 		       cp += x;
-- 
2.17.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0001-gnu-parted-Use-weak-non-blocking-randomness-for-FAT-.patch --]
[-- Type: text/x-patch, Size: 3406 bytes --]

From 3110c594afb8839ba81b528f316b6d6dce757e99 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Tue, 22 May 2018 17:34:47 +0200
Subject: [PATCH 1/3] gnu: parted: Use weak (non-blocking) randomness for FAT
 serial numbers.

Partly fixes <https://bugs.gnu.org/31268>.

* gnu/packages/patches/parted-non-blocking-random.patch: New file.
* gnu/packages/disk.scm (parted)[source]: Use it.
* gnu/local.mk (dist_patch_DATA): Add it.
---
 gnu/local.mk                                  |  1 +
 gnu/packages/disk.scm                         |  3 +-
 .../patches/parted-non-blocking-random.patch  | 39 +++++++++++++++++++
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/parted-non-blocking-random.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 20975306b..4222050b5 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -975,6 +975,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/p7zip-CVE-2017-17969.patch		\
   %D%/packages/patches/p7zip-remove-unused-code.patch		\
   %D%/packages/patches/password-store-gnupg-compat.patch	\
+  %D%/packages/patches/parted-non-blocking-random.patch		\
   %D%/packages/patches/patchelf-page-size.patch			\
   %D%/packages/patches/patchelf-rework-for-arm.patch		\
   %D%/packages/patches/patchutils-xfail-gendiff-tests.patch	\
diff --git a/gnu/packages/disk.scm b/gnu/packages/disk.scm
index d7bf72683..476b26acb 100644
--- a/gnu/packages/disk.scm
+++ b/gnu/packages/disk.scm
@@ -65,7 +65,8 @@
                                   version ".tar.xz"))
               (sha256
                (base32
-                "1r3qpg3bhz37mgvp9chsaa3k0csby3vayfvz8ggsqz194af5i2w5"))))
+                "1r3qpg3bhz37mgvp9chsaa3k0csby3vayfvz8ggsqz194af5i2w5"))
+              (patches (search-patches "parted-non-blocking-random.patch"))))
     (build-system gnu-build-system)
     (arguments
      `(#:phases
diff --git a/gnu/packages/patches/parted-non-blocking-random.patch b/gnu/packages/patches/parted-non-blocking-random.patch
new file mode 100644
index 000000000..e30d9f413
--- /dev/null
+++ b/gnu/packages/patches/parted-non-blocking-random.patch
@@ -0,0 +1,39 @@
+Explicitly use a non-blocking random primitive.  This is used primarily
+to compute FAT "serial numbers" in libparted/fs/r/fat/fat.c:fat_create,
+for which GRND_NONBLOCK is probably good enough.
+
+Assume the running kernel does support 'getrandom', which is the case on
+GuixSD.  See <https://bugs.gnu.org/31268>.
+
+--- a/libparted/labels/misc.h
++++ b/libparted/labels/misc.h
+@@ -17,7 +17,8 @@
+     along with this program.  If not, see <http://www.gnu.org/licenses/>. */
+ 
+ #include <inttypes.h>
+-#include <uuid/uuid.h>
++#include <sys/random.h>
++#include <errno.h>
+ 
+ /* hack: use the ext2 uuid library to generate a reasonably random (hopefully
+  * with /dev/random) number.  Unfortunately, we can only use 4 bytes of it.
+@@ -28,11 +29,17 @@ static inline uint32_t
+ generate_random_uint32 (void)
+ {
+        union {
+-               uuid_t uuid;
++               char uuid[4];
+                uint32_t i;
+        } uu32;
++       ssize_t ret;
+ 
+-       uuid_generate (uu32.uuid);
++       do
++         ret = getrandom (uu32.uuid, sizeof uu32, GRND_NONBLOCK);
++       while (ret == EAGAIN);
++
++       if (ret < sizeof uu32)
++         abort ();
+ 
+        return uu32.i > 0 ? uu32.i : 0xffffffff;
+ }
-- 
2.17.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0002-vm-Use-a-deterministic-file-system-UUID-in-shared-st.patch --]
[-- Type: text/x-patch, Size: 1769 bytes --]

From 8aa37a4124db90a9991485477d1af85677c7fa1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Tue, 22 May 2018 17:36:35 +0200
Subject: [PATCH 2/3] vm: Use a deterministic file system UUID in shared-store
 VMs.

* gnu/system/vm.scm (system-qemu-image/shared-store): Pass
 #:file-system-uuid to 'qemu-image'.
---
 gnu/system/vm.scm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gnu/system/vm.scm b/gnu/system/vm.scm
index 09a11af86..b6561dfc7 100644
--- a/gnu/system/vm.scm
+++ b/gnu/system/vm.scm
@@ -752,6 +752,13 @@ with the host.
 When FULL-BOOT? is true, return an image that does a complete boot sequence,
 bootloaded included; thus, make a disk image that contains everything the
 bootloader refers to: OS kernel, initrd, bootloader data, etc."
+  (define root-uuid
+    ;; Use a fixed UUID so that 'mke2fs' doesn't wait for strong randomness
+    ;; while generating a UUID.  See <https://bugs.gnu.org/31268>.
+    ;; XXX: Actually it doesn't help since 'mke2fs' calls 'uuid_generate'
+    ;; anyway.
+    (operating-system-uuid os 'dce))
+
   (mlet* %store-monad ((os-drv   (operating-system-derivation os))
                        (bootcfg  (operating-system-bootcfg os)))
     ;; XXX: When FULL-BOOT? is true, we end up creating an image that contains
@@ -763,6 +770,7 @@ bootloader refers to: OS kernel, initrd, bootloader data, etc."
                 #:bootloader (bootloader-configuration-bootloader
                               (operating-system-bootloader os))
                 #:disk-image-size disk-image-size
+                #:file-system-uuid root-uuid
                 #:inputs (if full-boot?
                              `(("bootcfg" ,bootcfg))
                              '())
-- 
2.17.0