GNU bug report logs - #33966
fcgiwrap: additional options for logging and unix domain sockets

Previous Next

Full log

Message #8 received at 33966 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Florian Dold <florian.dold <at> gmail.com>
Cc: 33966 <at> debbugs.gnu.org
Subject: Re: [bug#33966] fcgiwrap: additional options for logging and unix
 domain sockets
Date: Wed, 09 Jan 2019 17:17:01 +0100

Hi Florian,

Florian Dold <florian.dold <at> gmail.com> 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 <flo <at> dold.me>
> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.