[bug#33966] fcgiwrap: additional options for logging and unix domain sockets

[Ludovic Courtès]

Hi Florian,

Florian Dold <address@hidden> skribis:

> this patch adds additional options to the fcgiwrap service.  In
> particular it allows
>
> 1. writing the output of the fcgi process to a file (with the 'log-file'
> option)
>
> 2. arranging for a directory to be created so that the fcgiwrap process
> can create its listening socket without running into permission problems
> (with the 'ensure-socket-dir?' option)
>
> 3. adjusting the permissions on the listening unix domain socket,
> typically so that users in the fcgiwrap group have read and write access
> to that socket (with the 'adjusted-socket-permissions' option)
>
> Additionally, a potentially left-over fcgiwrap socket is cleaned up
> before starting the service, which would otherwise lead to the process
> refusing to run.
>
> The documentation is also changed to address a potential security issue,
> now recommending against running fcgiwrap as root.

Thanks for working on it!

> The configuration defaults are not ideal (a tcp socket with unrestricted
> access from any local user), but impossible to change without breaking
> existing system definitions.

Yeah.  Perhaps we could print a warning or something to encourage users
to switch?

Overall LGTM.  Some minor comments below:

> From 3ac9c6fa536faff23291b21d4e649b85386fedfc Mon Sep 17 00:00:00 2001
> From: Florian Dold <address@hidden>
> Date: Thu, 3 Jan 2019 14:22:49 +0100
> Subject: [PATCH] services: fcgiwrap: Implement additional options
>
> The fcgiwrap service now supports logging and can be run
> on a unix domain socket as unprivileged user.
>
> * doc/guix.texi (Web Services): Document new options and replace
> dangerous advice about running fcgiwrap as root.
> * gnu/services/web.scm: Add the options 'log-file',
> 'adjusted-socket-permissions' and 'ensure-socket-dir?'.

It’d be great if you could list the modified variables for web.scm;
otherwise I can do it for you.

>  (define-record-type* <fcgiwrap-configuration> fcgiwrap-configuration
>    make-fcgiwrap-configuration
>    fcgiwrap-configuration?
> -  (package       fcgiwrap-configuration-package ;<package>
> -                 (default fcgiwrap))
> -  (socket        fcgiwrap-configuration-socket
> -                 (default "tcp:127.0.0.1:9000"))
> -  (user          fcgiwrap-configuration-user
> -                 (default "fcgiwrap"))
> -  (group         fcgiwrap-configuration-group
> -                 (default "fcgiwrap")))
> +  (package fcgiwrap-configuration-package ;<package>
> +           (default fcgiwrap))
> +  (socket fcgiwrap-configuration-socket
> +          (default "tcp:127.0.0.1:9000"))
> +  (user fcgiwrap-configuration-user
> +        (default "fcgiwrap"))
> +  (group fcgiwrap-configuration-group
> +         (default "fcgiwrap"))
> +  (log-file fcgiwrap-log-file
> +            (default #f))
> +  ;; boolean or octal mode integer
> +  (adjusted-socket-permissions fcgiwrap-adjusted-socket-permissions?
> +                               (default #f))

Maybe just ‘socket-permissions’ and also leave out interpretation of #t
as #o666?

Also the accessor should then be ‘fcgiwrap-socket-permissions’.

> +  (ensure-socket-dir? fcgiwrap-ensure-socket-dir?
> +                      (default #f)))

s/dir/directory/ please.  :-)

Also please remove tabs from the file.

Could you make sure “make check-system TESTS=cgit” still passes after
the change?

The rest LGTM.  Could you send an updated patch?

Thank you!

Ludo’.