[PATCH 0/1] Improvements to urandom-seed service

[PATCH 0/1] Improvements to urandom-seed service

[Leo Famulari]

I read more on the subject of seeding /dev/urandom [0] and I found that
our service should be improved.

We should "refresh" the seed unconditionally in 'start', after we use it
to seed /dev/urandom [1]. This way, if there is no clean shut down, the
next boot does not re-use the same seed. At first boot, this "refreshed"
seed may not be of great quality, since we have not seeded /dev/urandom
yet, but it's better than the possibility of a 2nd boot with no seeding
at all.

This is recommended in the example in random(4) and the Linux code
comments [2]. I missed this before.

Currently, we make sure the seed exists with appropriate permissions
during activation.

If we refresh the seed in 'start', we can ensure it exists before
refreshing it. Since 'stop' also creates the seed file, we might as well
remove the activation code entirely... right? In that case, we also need
to do mkdir-p in 'stop', to be sure.

Your feedback is requested!

[0]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c#n159

[1] This does not happen if the seed file does not exist. With this
patch, the seed file will not exist at first boot until 'start' has
completed.

[2]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/char/random.c#n202

Leo Famulari (1):
  services: urandom-seed: Refresh seed at boot.

 gnu/services/base.scm | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

-- 
2.8.3

[PATCH 1/1] services: urandom-seed: Refresh seed at boot.

[Leo Famulari]

* gnu/services/base.scm (urandom-seed-shepherd-service): Refresh the random
seed unconditionally at boot. Ensure directory structure for %random-seed-file
exists when shutting down.
(%urandom-seed-activation): Remove variable.
(urandom-seed-service-type): Remove deleted variable from list of extensions.
---
 gnu/services/base.scm | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index b8e4741..2780d12 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -431,15 +431,6 @@ stopped before 'kill' is called."
 (define %random-seed-file
   "/var/lib/random-seed")
 
-(define %urandom-seed-activation
-  ;; Activation gexp for the urandom seed
-  #~(begin
-      (use-modules (guix build utils))
-
-      (mkdir-p (dirname #$%random-seed-file))
-      (close-port (open-file #$%random-seed-file "a0b"))
-      (chmod #$%random-seed-file #o600)))
-
 (define (urandom-seed-shepherd-service _)
   "Return a shepherd service for the /dev/urandom seed."
   (list (shepherd-service
@@ -454,6 +445,18 @@ stopped before 'kill' is called."
                           (call-with-output-file "/dev/urandom"
                             (lambda (urandom)
                               (dump-port seed urandom))))))
+                    ;; Immediately refresh the seed in case the system doesn't
+                    ;; shut down cleanly.
+                    (call-with-input-file "/dev/urandom"
+                      (lambda (urandom)
+                        (let ((previous-umask (umask #o077))
+                              (buf (make-bytevector 512)))
+                          (mkdir-p (dirname #$%random-seed-file))
+                          (get-bytevector-n! urandom buf 0 512)
+                          (call-with-output-file #$%random-seed-file
+                            (lambda (seed)
+                              (put-bytevector seed buf)))
+                          (umask previous-umask))))
                     #t))
          (stop #~(lambda _
                    ;; During shutdown, write from /dev/urandom into random seed.
@@ -462,6 +465,7 @@ stopped before 'kill' is called."
                        (lambda (urandom)
                          (let ((previous-umask (umask #o077)))
                            (get-bytevector-n! urandom buf 0 512)
+                           (mkdir-p (dirname #$%random-seed-file))
                            (call-with-output-file #$%random-seed-file
                              (lambda (seed)
                                (put-bytevector seed buf)))
@@ -475,9 +479,7 @@ stopped before 'kill' is called."
   (service-type (name 'urandom-seed)
                 (extensions
                  (list (service-extension shepherd-root-service-type
-                                          urandom-seed-shepherd-service)
-                       (service-extension activation-service-type
-                                          (const %urandom-seed-activation))))))
+                                          urandom-seed-shepherd-service)))))
 
 (define (urandom-seed-service)
   (service urandom-seed-service-type #f))
-- 
2.8.3

Re: [PATCH 0/1] Improvements to urandom-seed service

[Ludovic Courtès]

Leo Famulari <leo@famulari.name> skribis:

> I read more on the subject of seeding /dev/urandom [0] and I found that
> our service should be improved.
>
> We should "refresh" the seed unconditionally in 'start', after we use it
> to seed /dev/urandom [1]. This way, if there is no clean shut down, the
> next boot does not re-use the same seed. At first boot, this "refreshed"
> seed may not be of great quality, since we have not seeded /dev/urandom
> yet, but it's better than the possibility of a 2nd boot with no seeding
> at all.
>
> This is recommended in the example in random(4) and the Linux code
> comments [2]. I missed this before.

OK, makes sense.

> Currently, we make sure the seed exists with appropriate permissions
> during activation.
>
> If we refresh the seed in 'start', we can ensure it exists before
> refreshing it. Since 'stop' also creates the seed file, we might as well
> remove the activation code entirely... right?

Right.

> In that case, we also need to do mkdir-p in 'stop', to be sure.

Can’t hurt.  ;-)

> * gnu/services/base.scm (urandom-seed-shepherd-service): Refresh the random
> seed unconditionally at boot. Ensure directory structure for %random-seed-file
> exists when shutting down.
> (%urandom-seed-activation): Remove variable.
> (urandom-seed-service-type): Remove deleted variable from list of extensions.

LGTM.

Thanks!

Ludo’.