Re: TOCTTOU race

[Ludovic Courtès]

Hi Maxime,

Maxime Devos <maximedevos@telenet.be> skribis:

> On Thu, 2021-02-18 at 18:54 +0100, Ludovic Courtès wrote:

[...]

>> Note that, if that happens at boot time, the system will fail to boot (I
>> think you’d get a REPL rather than a kernel panic, but it’d be good to
>> check in a VM.)
>
> If that happens, that's too bad.  Just ignoring the error seems bad from
> a security perspective.  I verified in a VM you'd get a REPL.
> From the REPL, a sysadmin could investigate and choose to delete the offending
> symlink & reboot (and presumably fix the security bug and upgrade the 
> service),
> or decide Guix System needs to be reinstalled.

OK, sounds reasonable.

> Please take note that I didn't correct all potentially insecure activation 
> gexps.
> These should ideally be done by someone who knows how to use the particular 
> service
> and have a system to test it on.  (My changes to nscld-service-type and 
> knot-activation
> are untested.)

I agree this is how it should happen ideally… let’s see if things happen
“ideally”.  :-)

> From 2c3968f658ada27d2062a960d229f3db9cfe208c Mon Sep 17 00:00:00 2001
> From: Maxime Devos <maximedevos@telenet.be>
> Date: Sun, 14 Feb 2021 12:57:32 +0100
> Subject: [PATCH] services: prevent following symlinks during activation
                             ^
Nitpick: we usually capitalize here and in the commit log.

Perhaps add a couple of lines explaining that this fixes a potential
security issue, with a link to this thread.

> Currently, there's a TOCTTOU race.  This can be addressed
> once guile has bindings for fstatat, openat and friends.

I’d move that comment next to the ‘mkdir-p/perms’ definition.

> * guix/build/service-utils.scm: new module
>   with new procedure 'mkdir-p/perms'.

I think you can remove these lines.

> * Makefile.am (MODULES): compile new module.
> * gnu/services/authentication.scm
>   (%nslcd-activation, nslcd-service-type): use new procedure.
> * gnu/services/cups.scm (%cups-activation): likewise.
> * gnu/services/dbus.scm (dbus-activation): likewise.
> * gnu/services/dns.scm (knot-activation): likewise.

LGTM for master, thanks!

Ludo’.