Re: TOCTTOU race

[Maxime Devos]

On Thu, 2021-02-18 at 18:54 +0100, Ludovic Courtès wrote:
> [...]
> I think this should go either in (gnu build activation) or in a new (gnu
> build utils) module.
> 
> (guix build …) is for non-Guix-System things.

I've moved mkdir-p/perms into (gnu build activation).

> > +;; Based upon mkdir-p from (guix build utils)
> > +(define (verify-not-symbolic dir)
> > +  [...])

I've replaced the (when (eq? 'symlink) ...) with
(unless (eq? 'directory) ...).

> It’s tempting to do something like:
> 
>   (error "file name component is a directory" dir)

I've added a "not" between "is" and "a" ->
  (error "file name component is not a directory" dir)

> 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.

> > [...]
> 
> Per GNU and Guix convention, “path” is for “search paths”; here it
> should be “file” or something.

Changed in new patch (attached).

Apparently, I forgot a few #:use-module.  This should be corrected now.

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.)

Greetings,
Maxime

0001-services-prevent-following-symlinks-during-activatio.patch
Description: Text Data

signature.asc
Description: This is a digitally signed message part