Acknowledgement sent
to Diego Nicola Barbato <dnbarbato@posteo.de>:
New bug report received and forwarded. Copy sent to bug-guix@gnu.org.
(Fri, 03 Apr 2020 13:20:02 GMT) (full text, mbox, link).
Hey Guix,
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.
Regards,
Diego
Information forwarded
to bug-guix@gnu.org: bug#40405; Package guix.
(Fri, 03 Apr 2020 13:35:01 GMT) (full text, mbox, link).
Subject: Re: bug#40405: System log files are world readable
Date: Fri, 03 Apr 2020 15:34:17 +0200
Diego Nicola Barbato <dnbarbato@posteo.de> writes:
> Hey Guix,
>
> 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?
^^^^^^^^^^^^^^^^^^^^^
That should be 'make-forkexec-constructor'. Sorry for the noise.
> Otherwise that might have been a simple solution.
>
> Regards,
>
> Diego
Added tag(s) security.
Request was from Ludovic Courtès <ludo@gnu.org>
to control@debbugs.gnu.org.
(Sun, 05 Apr 2020 21:33:06 GMT) (full text, mbox, link).
Information forwarded
to bug-guix@gnu.org: bug#40405; Package guix.
(Sun, 05 Apr 2020 22:13:02 GMT) (full text, mbox, link).
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!
In the meantime, the patch below fixes the syslogd problem. Also
attached is a patch for the accounting database, though that one is
questionable.
Thoughts?
Thanks,
Ludo’.
diff --git a/gnu/services.scm b/gnu/services.scm
index 7941cd3af0..d631e8dd32 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -528,15 +528,20 @@ ACTIVATION-SCRIPT-TYPE."
(use-modules (gnu build activation)
(guix build utils))
+ (define (ensure-file-exists file)
+ (let ((port (open-file file "a0")))
+ (chmod port #o640)
+ (close-port port)))
+
;; Make sure the user accounting database exists. If it
;; does not exist, 'setutxent' does not create it and
;; thus there is no accounting at all.
- (close-port (open-file "/var/run/utmpx" "a0"))
+ (ensure-file-exists "/var/run/utmpx")
;; Same for 'wtmp', which is populated by mingetty et
;; al.
(mkdir-p "/var/log")
- (close-port (open-file "/var/log/wtmp" "a0"))
+ (ensure-file-exists "/var/log/wtmp")
;; Set up /run/current-system. Among other things this
;; sets up locales, which the activation snippets
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 8d9a563e2b..e59b6fea80 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1436,10 +1436,17 @@ Service Switch}, for an example."
(documentation "Run the syslog daemon (syslogd).")
(provision '(syslogd))
(requirement '(user-processes))
- (start #~(make-forkexec-constructor
- (list #$(syslog-configuration-syslogd config)
- "--rcfile" #$(syslog-configuration-config-file config))
- #:pid-file "/var/run/syslog.pid"))
+ (start #~(let ((fork (make-forkexec-constructor
+ (list #$(syslog-configuration-syslogd config)
+ "--rcfile"
+ #$(syslog-configuration-config-file config))
+ #:pid-file "/var/run/syslog.pid")))
+ (lambda ()
+ ;; Set the umask such that file permissions are #o640.
+ (let ((mask (umask #o137))
+ (pid (fork)))
+ (umask mask)
+ pid))))
(stop #~(make-kill-destructor))))))
;; Snippet adapted from the GNU inetutils manual.
Information forwarded
to bug-guix@gnu.org: bug#40405; Package guix.
(Mon, 06 Apr 2020 22:08:02 GMT) (full text, mbox, link).
Subject: Re: bug#40405: System log files are world readable
Date: Tue, 07 Apr 2020 00:07:14 +0200
Hi,
Ludovic Courtès <ludo@gnu.org> skribis:
> In the meantime, the patch below fixes the syslogd problem. Also
> attached is a patch for the accounting database, though that one is
> questionable.
I pushed the syslog bits along with a test as commit
d7113bb655ff80a868a9e624c913f9d23e6c63ad. (I think already
world-readable files will remain world-readable though?)
The main remaining issue here is log files created by
‘fork+exec-command’. We’ll have to address that in the Shepherd proper,
I think.
Ludo’.
Information forwarded
to bug-guix@gnu.org: bug#40405; Package guix.
(Tue, 07 Apr 2020 00:51:01 GMT) (full text, mbox, link).
Cc: 40405@debbugs.gnu.org, Diego Nicola Barbato <dnbarbato@posteo.de>
Subject: Re: bug#40405: System log files are world readable
Date: Tue, 7 Apr 2020 02:49:58 +0200
Hi Ludo,
On +2020-04-07 00:07:14 +0200, Ludovic Courtès wrote:
> Hi,
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
> > In the meantime, the patch below fixes the syslogd problem. Also
> > attached is a patch for the accounting database, though that one is
> > questionable.
>
> I pushed the syslog bits along with a test as commit
> d7113bb655ff80a868a9e624c913f9d23e6c63ad. (I think already
> world-readable files will remain world-readable though?)
>
Could build daemons do some kind of maintenance rebuild to chmod them?
And maybe be scheduled to monitor new files for other mistakes as well?
Meanwhile, could a superuser chmod them without affecting hashes?
(curious as to whether permission bits escape hashing).
> The main remaining issue here is log files created by
> ‘fork+exec-command’. We’ll have to address that in the Shepherd proper,
> I think.
>
> Ludo’.
>
>
>
--
Regards,
Bengt Richter
Information forwarded
to bug-guix@gnu.org: bug#40405; Package guix.
(Tue, 07 Apr 2020 07:31:02 GMT) (full text, mbox, link).
Cc: 40405@debbugs.gnu.org, Diego Nicola Barbato <dnbarbato@posteo.de>
Subject: Re: bug#40405: System log files are world readable
Date: Tue, 07 Apr 2020 09:30:29 +0200
Hi,
Bengt Richter <bokr@bokr.com> skribis:
> On +2020-04-07 00:07:14 +0200, Ludovic Courtès wrote:
>> Hi,
>>
>> Ludovic Courtès <ludo@gnu.org> skribis:
>>
>> > In the meantime, the patch below fixes the syslogd problem. Also
>> > attached is a patch for the accounting database, though that one is
>> > questionable.
>>
>> I pushed the syslog bits along with a test as commit
>> d7113bb655ff80a868a9e624c913f9d23e6c63ad. (I think already
>> world-readable files will remain world-readable though?)
>>
>
> Could build daemons do some kind of maintenance rebuild to chmod them?
> And maybe be scheduled to monitor new files for other mistakes as well?
Yes, we could do that, I just haven’t checked if this is necessary or
thought about how to do it.
> Meanwhile, could a superuser chmod them without affecting hashes?
Definitely. (There’s no “hashing” involved for /var/log.)
Ludo’.
Information forwarded
to bug-guix@gnu.org: bug#40405; Package guix.
(Wed, 08 Apr 2020 12:34:01 GMT) (full text, mbox, link).
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
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
Information forwarded
to bug-guix@gnu.org: bug#40405; Package guix.
(Wed, 08 Apr 2020 19:50:02 GMT) (full text, mbox, link).
Subject: Re: bug#40405: System log files are world readable
Date: Wed, 08 Apr 2020 21:49:08 +0200
Diego Nicola Barbato <dnbarbato@posteo.de> skribis:
> 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.
Sounds good to me.
> 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'.
Both patches LGTM, but you could add a couple of tests in the Shepherd
itself before testing it on Guix.
The tests/*.sh are simple shell scripts. You could perhaps create a new
one there, run shepherd with a toy service that uses #:log-file and
creates files, and then ensure that the log file is #o640 and that
#:file-creation-mask is honored.
Does that make sense?
Then, to test it on Guix, you can run “make dist” in the Shepherd and
change the ‘shepherd’ package so that its ‘source’ points to that
tarball. You run ‘guix system vm gnu/system/examples/bare-bones.tmpl’,
boot that, and ensure everything’s OK.
Thanks!
Ludo’.
Reply sent
to Ludovic Courtès <ludo@gnu.org>:
You have taken responsibility.
(Sun, 19 Apr 2020 14:29:01 GMT) (full text, mbox, link).
Notification sent
to Diego Nicola Barbato <dnbarbato@posteo.de>:
bug acknowledged by developer.
(Sun, 19 Apr 2020 14:29:01 GMT) (full text, mbox, link).
Hi,
Ludovic Courtès <ludo@gnu.org> writes:
> Hi,
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> Diego Nicola Barbato <dnbarbato@posteo.de> skribis:
>>
>>>>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.
>>
>> [...]
>>
>>>>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.
>>
>> I went ahead and pushed these two patches.
>
> These patches are in Shepherd 0.8.0, which was pushed in Guix master
> commit e3358a831e7d5d9e8dc614340e49ea5aeb11a7ff, so we’re done!
Great! Now we can simplify the 'start' method of
'syslogd-service-type'.
I did eventually write a test script, which I've attached in case we
want to add it to the Shepherd. I'm sorry it took so long that I missed
the new Shepherd release.
Regards,
Diego
Subject: Re: bug#40405: System log files are world readable
Date: Tue, 28 Apr 2020 22:57:38 +0200
Hello,
Diego Nicola Barbato <dnbarbato@posteo.de> skribis:
> Great! Now we can simplify the 'start' method of
> 'syslogd-service-type'.
Oh right, do you want to take care of it?
> I did eventually write a test script, which I've attached in case we
> want to add it to the Shepherd. I'm sorry it took so long that I missed
> the new Shepherd release.
No problem. I figured it was okay to add it without a test, but having
a test is always better so I’ve happily pushed your patch. Thank you!
Ludo’.
Information forwarded
to bug-guix@gnu.org: bug#40405; Package guix.
(Wed, 29 Apr 2020 10:03:01 GMT) (full text, mbox, link).
Subject: Re: bug#40405: System log files are world readable
Date: Wed, 29 Apr 2020 12:02:05 +0200
Hi,
Ludovic Courtès <ludo@gnu.org> writes:
> Hello,
>
> Diego Nicola Barbato <dnbarbato@posteo.de> skribis:
>
>> Great! Now we can simplify the 'start' method of
>> 'syslogd-service-type'.
>
> Oh right, do you want to take care of it?
I already did: https://debbugs.gnu.org/40937
[...]
Regards,
Diego
bug archived.
Request was from Debbugs Internal Request <help-debbugs@gnu.org>
to internal_control@debbugs.gnu.org.
(Wed, 27 May 2020 11:24:05 GMT) (full text, mbox, link).
Debbugs is free software and licensed under the terms of the
GNU Public License version 2. The current version can be
obtained from https://bugs.debian.org/debbugs-source/.