bug#40405: System log files are world readable

[Diego Nicola Barbato <dnbarbato@posteo.de>]

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

Hey,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi,
>
> Diego Nicola Barbato <dnbarbato@posteo.de> skribis:
>
>> On Guix System the log files (in /var/log) generated by syslogd are
>> currently (commit 151f3d4) world readable.  They should probably only be
>> readable by root (for the same reason that dmesg can only be run by
>> root).
>>
>> It isn't possible to set the umask with fork-exec-constructor, is it?
>> Otherwise that might have been a simple solution.
>
> That would be a nice solution to implement in the Shepherd.  If you feel
> like giving it a try, that would be great!

I've attached two patches for the Shepherd.  The first one makes sure
that 'exec-command' creates log files with mode #o640 (I thought about
making it a parameter instead of hard coding it, but I doubt it would be
very useful).  The second one makes it possible to set the umask with
'exec-command', 'fork+exec-command', and 'make-forkexec-constructor'.  I
wasn't quite sure how to avoid a collision with the procedure umask
(would `((@ (guile) umask) umask)' have been ok?) so I named the
parameter file-creation-mask.

I haven't tested the changes.  What would be a straight forward way to
do that on Guix?  Looking at the documentation it doesn't seem possible
to swap out the shepherd package of the %shepherd-root-service with
'modify-services'. 

[...]

Regards,

Diego


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-service-Create-log-files-as-non-world-readable.patch --]
[-- Type: text/x-patch, Size: 1050 bytes --]

From 43c9ded791ce5b480504ce3528ee34578168f90e Mon Sep 17 00:00:00 2001
From: Diego Nicola Barbato <dnbarbato@posteo.de>
Date: Tue, 7 Apr 2020 13:58:28 +0200
Subject: [PATCH 1/2] service: Create log files as non-world-readable.

* modules/shepherd/service.scm (exec-command): Create log-file with file
  permissions #o640.
---
 modules/shepherd/service.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index fc82cc4..9a4a5d9 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -808,7 +808,7 @@ false."
              ;; Redirect stout and stderr to use LOG-FILE.
              (catch-system-error (close-fdes 1))
              (catch-system-error (close-fdes 2))
-             (dup2 (open-fdes log-file (logior O_CREAT O_WRONLY O_APPEND)) 1)
+             (dup2 (open-fdes log-file (logior O_CREAT O_WRONLY O_APPEND) #o640) 1)
              (dup2 1 2))
            (lambda (key . args)
              (format (current-error-port)
-- 
2.26.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-service-Add-file-creation-mask-to-make-forkexec-cons.patch --]
[-- Type: text/x-patch, Size: 6532 bytes --]

From e491436967a912e6e7372f582b3bf5c9784b8209 Mon Sep 17 00:00:00 2001
From: Diego Nicola Barbato <dnbarbato@posteo.de>
Date: Tue, 7 Apr 2020 13:38:47 +0200
Subject: [PATCH 2/2] service: Add #:file-creation-mask to
 'make-forkexec-constructor'.

* modules/shepherd/service.scm (exec-command): Add #:file-creation-mask
  parameter and honor it.
  (fork+exec-command): Add #:file-creation-mask parameter and pass it to
  exec-command.
  (make-forkexec-constructor): Add #:file-creation-mask parameter and pass it
  to fork+exec-command.
* doc/shepherd.texi (Service De- and Constructors): Adjust accordingly.
---
 doc/shepherd.texi            |  9 +++++++--
 modules/shepherd/service.scm | 22 ++++++++++++++++------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/doc/shepherd.texi b/doc/shepherd.texi
index 3e61f5d..659eb82 100644
--- a/doc/shepherd.texi
+++ b/doc/shepherd.texi
@@ -896,10 +896,12 @@ execution of the @var{command} was successful, @code{#t} if not.
   [#:pid-file #f] [#:pid-file-timeout %pid-file-timeout] @
   [#:log-file #f] @
   [#:directory (default-service-directory)] @
+  [#:file-creation-mask #f] @
   [#:environment-variables (default-environment-variables)]
 Return a procedure that forks a child process, closes all file
 descriptors except the standard output and standard error descriptors, sets
-the current directory to @var{directory}, changes the environment to
+the current directory to @var{directory}, sets the umask to
+@var{file-creation-mask} unless it is @code{#f}, changes the environment to
 @var{environment-variables} (using the @code{environ} procedure), sets the
 current user to @var{user} and the current group to @var{group} unless they
 are @code{#f}, and executes @var{command} (a list of strings.)  The result of
@@ -935,13 +937,16 @@ procedures.
   [#:group #f] @
   [#:log-file #f] @
   [#:directory (default-service-directory)] @
+  [#:file-creation-mask #f] @
   [#:environment-variables (default-environment-variables)]
 @deffnx {procedure} fork+exec-command @var{command} @
   [#:user #f] @
   [#:group #f] @
   [#:directory (default-service-directory)] @
+  [#:file-creation-mask #f] @
   [#:environment-variables (default-environment-variables)]
-Run @var{command} as the current process from @var{directory}, and with
+Run @var{command} as the current process from @var{directory}, with
+@var{file-creation-mask} if it's true, and with
 @var{environment-variables} (a list of strings like @code{"PATH=/bin"}.)
 File descriptors 1 and 2 are kept as is or redirected to @var{log-file}
 if it's true, whereas file descriptor 0
diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index 9a4a5d9..d90b55b 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -771,12 +771,14 @@ daemon writing FILE is running in a separate PID namespace."
                        (group #f)
                        (log-file #f)
                        (directory (default-service-directory))
+                       (file-creation-mask #f)
                        (environment-variables (default-environment-variables)))
-  "Run COMMAND as the current process from DIRECTORY, and with
-ENVIRONMENT-VARIABLES (a list of strings like \"PATH=/bin\".)  File
-descriptors 1 and 2 are kept as is or redirected to LOG-FILE if it's true,
-whereas file descriptor 0 (standard input) points to /dev/null; all other file
-descriptors are closed prior to yielding control to COMMAND.
+  "Run COMMAND as the current process from DIRECTORY, with FILE-CREATION-MASK
+if it's true, and with ENVIRONMENT-VARIABLES (a list of strings like
+\"PATH=/bin\").  File descriptors 1 and 2 are kept as is or redirected to
+LOG-FILE if it's true, whereas file descriptor 0 (standard input) points to
+/dev/null; all other file descriptors are closed prior to yielding control to
+COMMAND.
 
 By default, COMMAND is run as the current user.  If the USER keyword
 argument is present and not false, change to USER immediately before
@@ -840,6 +842,9 @@ false."
            (print-exception (current-error-port) #f key args)
            (primitive-exit 1))))
 
+     (when file-creation-mask
+       (umask file-creation-mask))
+
      ;; As the last action, close file descriptors.  Doing it last makes
      ;; "error in the finalization thread: Bad file descriptor" issues
      ;; unlikely on 2.2.
@@ -871,6 +876,7 @@ false."
                             (group #f)
                             (log-file #f)
                             (directory (default-service-directory))
+                            (file-creation-mask #f)
                             (environment-variables
                              (default-environment-variables)))
   "Spawn a process that executed COMMAND as per 'exec-command', and return
@@ -886,6 +892,7 @@ its PID."
                       #:group group
                       #:log-file log-file
                       #:directory directory
+                      #:file-creation-mask file-creation-mask
                       #:environment-variables environment-variables)
         pid)))
 
@@ -903,7 +910,8 @@ its PID."
     (case-lambda*
      "Return a procedure that forks a child process, closes all file
 descriptors except the standard output and standard error descriptors, sets
-the current directory to @var{directory}, changes the environment to
+the current directory to @var{directory}, sets the umask to
+@var{file-creation-mask} unless it is @code{#f}, changes the environment to
 @var{environment-variables} (using the @code{environ} procedure), sets the
 current user to @var{user} and the current group to @var{group} unless they
 are @code{#f}, and executes @var{command} (a list of strings.)  The result of
@@ -918,6 +926,7 @@ start."
                (user #f)
                (group #f)
                (directory (default-service-directory))
+               (file-creation-mask #f)
                (environment-variables (default-environment-variables))
                (pid-file #f)
                (pid-file-timeout %pid-file-timeout)
@@ -944,6 +953,7 @@ start."
                                         #:group group
                                         #:log-file log-file
                                         #:directory directory
+                                        #:file-creation-mask file-creation-mask
                                         #:environment-variables
                                         environment-variables)))
             (if pid-file
-- 
2.26.0