From debbugs-submit-bounces@debbugs.gnu.org Wed Jan 09 11:17:07 2019 Received: (at 33966) by debbugs.gnu.org; 9 Jan 2019 16:17:07 +0000 Received: from localhost ([127.0.0.1]:52450 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ghGXT-0001JX-A2 for submit@debbugs.gnu.org; Wed, 09 Jan 2019 11:17:07 -0500 Received: from hera.aquilenet.fr ([185.233.100.1]:48112) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ghGXR-0001JN-9t for 33966@debbugs.gnu.org; Wed, 09 Jan 2019 11:17:05 -0500 Received: from localhost (localhost [127.0.0.1]) by hera.aquilenet.fr (Postfix) with ESMTP id 8F47E11F5; Wed, 9 Jan 2019 17:17:03 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at aquilenet.fr Received: from hera.aquilenet.fr ([127.0.0.1]) by localhost (hera.aquilenet.fr [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id d6lyJGZY_7eg; Wed, 9 Jan 2019 17:17:02 +0100 (CET) Received: from ribbon (unknown [IPv6:2a01:e0a:1d:7270:af76:b9b:ca24:c465]) by hera.aquilenet.fr (Postfix) with ESMTPSA id A6D7CC9A; Wed, 9 Jan 2019 17:17:02 +0100 (CET) From: =?utf-8?Q?Ludovic_Court=C3=A8s?= To: Florian Dold Subject: Re: [bug#33966] fcgiwrap: additional options for logging and unix domain sockets References: <624ba072-d5fe-b159-46af-61e79caf22f1@gmail.com> Date: Wed, 09 Jan 2019 17:17:01 +0100 In-Reply-To: <624ba072-d5fe-b159-46af-61e79caf22f1@gmail.com> (Florian Dold's message of "Thu, 3 Jan 2019 21:02:38 +0100") Message-ID: <87a7k94xhe.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 1.0 (+) X-Debbugs-Envelope-To: 33966 Cc: 33966@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.0 (/) Hi Florian, Florian Dold 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 > 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=E2=80=99d be great if you could list the modified variables for web.scm; otherwise I can do it for you. > (define-record-type* fcgiwrap-configuration > make-fcgiwrap-configuration > fcgiwrap-configuration? > - (package fcgiwrap-configuration-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 ; > + (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 =E2=80=98socket-permissions=E2=80=99 and also leave out interpre= tation of #t as #o666? Also the accessor should then be =E2=80=98fcgiwrap-socket-permissions=E2=80= =99. > + (ensure-socket-dir? fcgiwrap-ensure-socket-dir? > + (default #f))) s/dir/directory/ please. :-) Also please remove tabs from the file. Could you make sure =E2=80=9Cmake check-system TESTS=3Dcgit=E2=80=9D still = passes after the change? The rest LGTM. Could you send an updated patch? Thank you! Ludo=E2=80=99.