From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Subject: bug#29773: urandom-seed-service should run earlier in the boot process Date: Wed, 20 Dec 2017 11:19:36 +0100 Message-ID: <87tvwlzop3.fsf@gnu.org> References: <20171219191348.GA19177@jasmine.lan> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:52283) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eRbTq-0006W5-7R for bug-guix@gnu.org; Wed, 20 Dec 2017 05:20:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eRbTm-0006OU-Hy for bug-guix@gnu.org; Wed, 20 Dec 2017 05:20:06 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:34479) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eRbTm-0006OF-9j for bug-guix@gnu.org; Wed, 20 Dec 2017 05:20:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eRbTm-0006rS-2a for bug-guix@gnu.org; Wed, 20 Dec 2017 05:20:02 -0500 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <20171219191348.GA19177@jasmine.lan> (Leo Famulari's message of "Tue, 19 Dec 2017 14:13:48 -0500") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Leo Famulari Cc: 29773@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hello, Leo Famulari skribis: > In some cases, the applications require some random data before any > services are started, during activation. For example, our OpenSSH > service generates its host keys during activation. And even if it > generated host keys during the start of the OpenSSH service, that > service does not depend on urandom-seed-service. [0] > > In systemd, there is an abstract sysinit "target" that basically serves > as a checkpoint. All the lower-level system initialization is required > before the sysinit.target is met, and the rest of the services depend on > sysinit. The random seeding is part of sysinit. I've reproduced a graph > of this in [1]. There=E2=80=99s a =E2=80=98user-processes=E2=80=99 service that serves a si= milar purpose. With the attached patches =E2=80=98urandom-seed=E2=80=99 becomes a dependen= cy of =E2=80=98user-processes=E2=80=99, meaning that daemons & co. start after =E2=80=98urandom-seed=E2=80=99. WDYT? > In practice, I'm not sure if it matters. I'd appreciate if GuixSD users > could check /var/log/messages for warnings like this one and report > them: > > random: application: uninitialized urandom read (16 bytes read)=20 I don=E2=80=99t have any of these. I guess this is most likely to happen w= hen running =E2=80=98ssh-keygen=E2=80=99 on startup, which isn=E2=80=99t the ca= se on my machine. Ludo=E2=80=99. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0002-services-urandom-seed-Become-a-dependency-of-user-pr.patch >From 5895acdbc345572434d9efae5cf5cdd11e4c1a07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Wed, 20 Dec 2017 11:09:03 +0100 Subject: [PATCH 2/3] services: urandom-seed: Become a dependency of 'user-processes'. This ensures that 'urandom-seed' is started before programs that rely on sources of randomness. Fixes . Reported by Leo Famulari . * gnu/services/base.scm (urandom-seed-shepherd-service): Change 'requirement' to (file-systems). (urandom-seed-service-type): Extend USER-PROCESSES-SERVICE-TYPE. --- gnu/services/base.scm | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/gnu/services/base.scm b/gnu/services/base.scm index 481439d4f..cc59ec573 100644 --- a/gnu/services/base.scm +++ b/gnu/services/base.scm @@ -529,7 +529,7 @@ in KNOWN-MOUNT-POINTS when it is stopped." (list (shepherd-service (documentation "Preserve entropy across reboots for /dev/urandom.") (provision '(urandom-seed)) - (requirement '(user-processes)) + (requirement '(file-systems)) (start #~(lambda _ ;; On boot, write random seed into /dev/urandom. (when (file-exists? #$%random-seed-file) @@ -590,7 +590,13 @@ in KNOWN-MOUNT-POINTS when it is stopped." (service-type (name 'urandom-seed) (extensions (list (service-extension shepherd-root-service-type - urandom-seed-shepherd-service))) + urandom-seed-shepherd-service) + + ;; Have 'user-processes' depend on 'urandom-seed'. + ;; This ensures that user processes and daemons don't + ;; start until we have seeded the PRNG. + (service-extension user-processes-service-type + (const '(urandom-seed))))) (description "Seed the @file{/dev/urandom} pseudo-random number generator (RNG) with the value recorded when the system was last shut -- 2.15.1 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-services-user-processes-service-type-can-now-be-exte.patch >From 8d0714bdb038e525880aed9de29e78af8c021efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Wed, 20 Dec 2017 11:05:11 +0100 Subject: [PATCH 1/3] services: 'user-processes-service-type' can now be extended. * gnu/services/base.scm (user-processes-shepherd-service): New procedure, taken from former 'user-processes-service-type'. Add REQUIREMENTS argument; remove GRACE-DELAY argument. (user-processes-service-type): Redefine in terms of 'service-type'. (user-processes-service): Remove. (file-system-service-type): Extend USER-PROCESSES-SERVICE-TYPE. * gnu/system.scm (essential-services): Use USER-PROCESSES-SERVICE-TYPE directly. --- gnu/services/base.scm | 236 +++++++++++++++++++++++++++----------------------- gnu/system.scm | 2 +- 2 files changed, 130 insertions(+), 108 deletions(-) diff --git a/gnu/services/base.scm b/gnu/services/base.scm index a3654fd4d..481439d4f 100644 --- a/gnu/services/base.scm +++ b/gnu/services/base.scm @@ -57,7 +57,7 @@ file-system-service-type user-unmount-service swap-service - user-processes-service + user-processes-service-type host-name-service console-keymap-service %default-console-font @@ -162,6 +162,129 @@ ;;; ;;; Code: + + +;;; +;;; User processes. +;;; + +(define %do-not-kill-file + ;; Name of the file listing PIDs of processes that must survive when halting + ;; the system. Typical example is user-space file systems. + "/etc/shepherd/do-not-kill") + +(define (user-processes-shepherd-service requirements) + "Return the 'user-processes' Shepherd service with dependencies on +REQUIREMENTS (a list of service names). + +This is a synchronization point used to make sure user processes and daemons +get started only after crucial initial services have been started---file +system mounts, etc. This is similar to 'target' in systemd." + (define grace-delay + ;; Delay after sending SIGTERM and before sending SIGKILL. + 4) + + (list (shepherd-service + (documentation "When stopped, terminate all user processes.") + (provision '(user-processes)) + (requirement requirements) + (start #~(const #t)) + (stop #~(lambda _ + (define (kill-except omit signal) + ;; Kill all the processes with SIGNAL except those listed + ;; in OMIT and the current process. + (let ((omit (cons (getpid) omit))) + (for-each (lambda (pid) + (unless (memv pid omit) + (false-if-exception + (kill pid signal)))) + (processes)))) + + (define omitted-pids + ;; List of PIDs that must not be killed. + (if (file-exists? #$%do-not-kill-file) + (map string->number + (call-with-input-file #$%do-not-kill-file + (compose string-tokenize + (@ (ice-9 rdelim) read-string)))) + '())) + + (define (now) + (car (gettimeofday))) + + (define (sleep* n) + ;; Really sleep N seconds. + ;; Work around . + (define start (now)) + (let loop ((elapsed 0)) + (when (> n elapsed) + (sleep (- n elapsed)) + (loop (- (now) start))))) + + (define lset= (@ (srfi srfi-1) lset=)) + + (display "sending all processes the TERM signal\n") + + (if (null? omitted-pids) + (begin + ;; Easy: terminate all of them. + (kill -1 SIGTERM) + (sleep* #$grace-delay) + (kill -1 SIGKILL)) + (begin + ;; Kill them all except OMITTED-PIDS. XXX: We would + ;; like to (kill -1 SIGSTOP) to get a fixed list of + ;; processes, like 'killall5' does, but that seems + ;; unreliable. + (kill-except omitted-pids SIGTERM) + (sleep* #$grace-delay) + (kill-except omitted-pids SIGKILL) + (delete-file #$%do-not-kill-file))) + + (let wait () + ;; Reap children, if any, so that we don't end up with + ;; zombies and enter an infinite loop. + (let reap-children () + (define result + (false-if-exception + (waitpid WAIT_ANY (if (null? omitted-pids) + 0 + WNOHANG)))) + + (when (and (pair? result) + (not (zero? (car result)))) + (reap-children))) + + (let ((pids (processes))) + (unless (lset= = pids (cons 1 omitted-pids)) + (format #t "waiting for process termination\ + (processes left: ~s)~%" + pids) + (sleep* 2) + (wait)))) + + (display "all processes have been terminated\n") + #f)) + (respawn? #f)))) + +(define user-processes-service-type + (service-type + (name 'user-processes) + (extensions (list (service-extension shepherd-root-service-type + user-processes-shepherd-service))) + (compose concatenate) + (extend append) + + ;; The value is the list of Shepherd services 'user-processes' depends on. + ;; Extensions can add new services to this list. + (default-value '()) + + (description "The @code{user-processes} service is responsible for +terminating all the processes so that the root file system can be re-mounted +read-only, just before rebooting/halting. Processes still running after a few +seconds after @code{SIGTERM} has been sent are terminated with +@code{SIGKILL}."))) + ;;; ;;; File systems. @@ -349,7 +472,11 @@ FILE-SYSTEM." (list (service-extension shepherd-root-service-type file-system-shepherd-services) (service-extension fstab-service-type - identity))) + identity) + + ;; Have 'user-processes' depend on 'file-systems'. + (service-extension user-processes-service-type + (const '(file-systems))))) (compose concatenate) (extend append) (description @@ -389,111 +516,6 @@ file systems, as well as corresponding @file{/etc/fstab} entries."))) in KNOWN-MOUNT-POINTS when it is stopped." (service user-unmount-service-type known-mount-points)) -(define %do-not-kill-file - ;; Name of the file listing PIDs of processes that must survive when halting - ;; the system. Typical example is user-space file systems. - "/etc/shepherd/do-not-kill") - -(define user-processes-service-type - (shepherd-service-type - 'user-processes - (lambda (grace-delay) - (shepherd-service - (documentation "When stopped, terminate all user processes.") - (provision '(user-processes)) - (requirement '(file-systems)) - (start #~(const #t)) - (stop #~(lambda _ - (define (kill-except omit signal) - ;; Kill all the processes with SIGNAL except those listed - ;; in OMIT and the current process. - (let ((omit (cons (getpid) omit))) - (for-each (lambda (pid) - (unless (memv pid omit) - (false-if-exception - (kill pid signal)))) - (processes)))) - - (define omitted-pids - ;; List of PIDs that must not be killed. - (if (file-exists? #$%do-not-kill-file) - (map string->number - (call-with-input-file #$%do-not-kill-file - (compose string-tokenize - (@ (ice-9 rdelim) read-string)))) - '())) - - (define (now) - (car (gettimeofday))) - - (define (sleep* n) - ;; Really sleep N seconds. - ;; Work around . - (define start (now)) - (let loop ((elapsed 0)) - (when (> n elapsed) - (sleep (- n elapsed)) - (loop (- (now) start))))) - - (define lset= (@ (srfi srfi-1) lset=)) - - (display "sending all processes the TERM signal\n") - - (if (null? omitted-pids) - (begin - ;; Easy: terminate all of them. - (kill -1 SIGTERM) - (sleep* #$grace-delay) - (kill -1 SIGKILL)) - (begin - ;; Kill them all except OMITTED-PIDS. XXX: We would - ;; like to (kill -1 SIGSTOP) to get a fixed list of - ;; processes, like 'killall5' does, but that seems - ;; unreliable. - (kill-except omitted-pids SIGTERM) - (sleep* #$grace-delay) - (kill-except omitted-pids SIGKILL) - (delete-file #$%do-not-kill-file))) - - (let wait () - ;; Reap children, if any, so that we don't end up with - ;; zombies and enter an infinite loop. - (let reap-children () - (define result - (false-if-exception - (waitpid WAIT_ANY (if (null? omitted-pids) - 0 - WNOHANG)))) - - (when (and (pair? result) - (not (zero? (car result)))) - (reap-children))) - - (let ((pids (processes))) - (unless (lset= = pids (cons 1 omitted-pids)) - (format #t "waiting for process termination\ - (processes left: ~s)~%" - pids) - (sleep* 2) - (wait)))) - - (display "all processes have been terminated\n") - #f)) - (respawn? #f))))) - -(define* (user-processes-service #:key (grace-delay 4)) - "Return the service that is responsible for terminating all the processes so -that the root file system can be re-mounted read-only, just before -rebooting/halting. Processes still running GRACE-DELAY seconds after SIGTERM -has been sent are terminated with SIGKILL. - -The returned service will depend on 'file-systems', meaning that it is -considered started after all the auto-mount file systems have been mounted. - -All the services that spawn processes must depend on this one so that they are -stopped before 'kill' is called." - (service user-processes-service-type grace-delay)) - ;;; ;;; Preserve entropy to seed /dev/urandom on boot. diff --git a/gnu/system.scm b/gnu/system.scm index 7466ed780..df89ca06d 100644 --- a/gnu/system.scm +++ b/gnu/system.scm @@ -449,7 +449,7 @@ a container or that of a \"bare metal\" system." (other-fs (non-boot-file-system-service os)) (unmount (user-unmount-service known-fs)) (swaps (swap-services os)) - (procs (user-processes-service)) + (procs (service user-processes-service-type)) (host-name (host-name-service (operating-system-host-name os))) (entries (operating-system-directory-base-entries os #:container? container?))) -- 2.15.1 --=-=-=--