Fcgiwrap service has no supplementary groups

  • Open
  • quality assurance status badge
Details
One participant
  • pelzflorian (Florian Pelz)
Owner
unassigned
Submitted by
pelzflorian (Florian Pelz)
Severity
normal
P
P
pelzflorian (Florian Pelz) wrote on 30 Nov 2019 19:49
(address . bug-guix@gnu.org)
20191130184924.io5qmo6ujyy2xeyy@pelzflorian.localdomain
Fcgiwrap should be started with the supplementary groups of its user.
Shepherd’s make-forkexec-constructor does not currently appear to
support this.

Upstream fcgiwrap ships with a systemd service with the User= setting.

Systemd confers this user’s supplementary groups by default:
Toggle quote (5 lines)
> If the User= setting is used the supplementary group list is
> initialized from the specified user's default group list, as defined
> in the system's user and group database. Additional groups may be
> configured through the SupplementaryGroups= setting (see below).

Not starting with supplementary groups sometimes causes problems.

Namely the Guix manual claims for Gitolite’s umask:
Toggle quote (4 lines)
> A value like ‘#o0027’ will give read access to the group used
> by Gitolite (by default: ‘git’). This is necessary when using
> Gitolite with software like cgit or gitweb.

But this does not work because giving a supplementary group git to the
fcgiwrap user does not confer the supplementary group git to fcgiwrap.
This is visible when looking at the fcgiwrap process in
`ps -eo pid,supgrp,args`. It is also visible by configuring nginx to

fastcgi_param SCRIPT_FILENAME /test/test.sh;

and making test.sh a script that prints "Content-Type: text/plain\n\n"
followed by the output of the id command.

Regards,
Florian
P
P
pelzflorian (Florian Pelz) wrote on 4 Dec 2019 11:22
(address . 38438@debbugs.gnu.org)
20191204102212.ldt6w4whzfz6ceq5@pelzflorian.localdomain
I had hoped the attached quick hack would fix my issue when testing
with the attached vm-image config from
That is, I wanted it to suffice to set Gitolite’s umask to #o0027 as
described in the manual instead of #o0022, after I do `usermod -aG git
fcgiwrap`. But instead I get “Operation not permitted” error from
setgroups. I will try again later with the position of setuid and
setgroups call swapped.

The hack makes make-forkexec-constructor use the supplementary groups
from the user. Systemd uses them by default. However they should be
made more configurable.

Regards,
Florian
From ddf372637089957e8c62d53c7eca07cfa9155a04 Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflorian@pelzflorian.de>
Date: Wed, 4 Dec 2019 09:33:08 +0100
Subject: [PATCH] gnu: shepherd: Patch Shepherd to set supplementary groups to
those of #:user.


* gnu/packages/patches/shepherd-set-supplementary-groups.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/admin.scm (shepherd): Use it.
---
gnu/local.mk | 1 +
gnu/packages/admin.scm | 4 +-
.../shepherd-set-supplementary-groups.patch | 43 +++++++++++++++++++
3 files changed, 47 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/shepherd-set-supplementary-groups.patch

Toggle diff (78 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 9ddd1349da..b807e3879c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1348,6 +1348,7 @@ dist_patch_DATA = \
%D%/packages/patches/seahorse-gkr-use-0-on-empty-flags.patch \
%D%/packages/patches/seq24-rename-mutex.patch \
%D%/packages/patches/sharutils-CVE-2018-1000097.patch \
+ %D%/packages/patches/shepherd-set-supplementary-groups.patch \
%D%/packages/patches/shishi-fix-libgcrypt-detection.patch \
%D%/packages/patches/slim-session.patch \
%D%/packages/patches/slim-config.patch \
diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index 6e5648d159..3f94b45623 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -201,7 +201,9 @@ and provides a \"top-like\" mode (monitoring).")
version ".tar.gz"))
(sha256
(base32
- "1xn6mb5bh8bpfgdrh09ja31jk0ln7bmxbbf0vjcqxkkixs2wl6sk"))))
+ "1xn6mb5bh8bpfgdrh09ja31jk0ln7bmxbbf0vjcqxkkixs2wl6sk"))
+ (patches
+ (search-patches "shepherd-set-supplementary-groups.patch"))))
(build-system gnu-build-system)
(arguments
'(#:configure-flags '("--localstatedir=/var")))
diff --git a/gnu/packages/patches/shepherd-set-supplementary-groups.patch b/gnu/packages/patches/shepherd-set-supplementary-groups.patch
new file mode 100644
index 0000000000..8cac24417d
--- /dev/null
+++ b/gnu/packages/patches/shepherd-set-supplementary-groups.patch
@@ -0,0 +1,43 @@
+diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
+index bd7e379..2344915 100644
+--- a/modules/shepherd/service.scm
++++ b/modules/shepherd/service.scm
+@@ -758,6 +758,28 @@ daemon writing FILE is running in a separate PID namespace."
+ (try-again)
+ (apply throw args)))))))
+
++(define (supplementary-gids user)
++ "Return a vector with the gid for each supplementary group USER belongs to.
++USER is the user name as a string."
++ ;; TODO: To find them, we loop through the group database, but maybe using
++ ;; glibc’s getgrouplist would be better. But it is not exported from Guile
++ ;; and it seems it is not part of POSIX (?).
++ (list->vector
++ (delete-duplicates
++ (dynamic-wind
++ (lambda () (setgrent))
++ (lambda ()
++ (let loop ((supgids '()))
++ (let ((group (getgrent)))
++ (define (user-among-group? group)
++ (member user (group:mem group)))
++ (match group
++ (#f supgids)
++ ((? user-among-group?)
++ (loop (cons (group:gid group) supgids)))
++ (else (loop supgids))))))
++ (lambda () (endgrent))))))
++
+ (define* (exec-command command
+ #:key
+ (user #f)
+@@ -826,7 +848,8 @@ false."
+ (when user
+ (catch #t
+ (lambda ()
+- (setuid (passwd:uid (getpw user))))
++ (setuid (passwd:uid (getpw user)))
++ (setgroups (supplementary-gids user)))
+ (lambda (key . args)
+ (format (current-error-port)
+ "failed to change to user ~s:~%" user)
--
2.24.0
Attachment: test-vm-config.scm
P
P
pelzflorian (Florian Pelz) wrote on 4 Dec 2019 12:32
(address . 38438@debbugs.gnu.org)
20191204113239.immmcpixu2achory@pelzflorian.localdomain
On Wed, Dec 04, 2019 at 11:22:13AM +0100, pelzflorian (Florian Pelz) wrote:
Toggle quote (2 lines)
> I had hoped the attached quick hack would fix my issue when testing

The now attached patch works now (after doing `usermod -aG git
fcgiwrap`, `herd stop fcgiwrap` and `herd start fcgiwrap`).

Regards,
Florian
From 901f3e0ff52e817344a839a5f7c55c96dd530704 Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflorian@pelzflorian.de>
Date: Wed, 4 Dec 2019 09:33:08 +0100
Subject: [PATCH] gnu: shepherd: Patch Shepherd to set supplementary groups to
those of #:user.


* gnu/packages/patches/shepherd-set-supplementary-groups.patch: New file.
* gnu/local.mk (dist_patch_DATA): Add it.
* gnu/packages/admin.scm (shepherd): Use it.
---
gnu/local.mk | 1 +
gnu/packages/admin.scm | 4 +-
.../shepherd-set-supplementary-groups.patch | 41 +++++++++++++++++++
3 files changed, 45 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/shepherd-set-supplementary-groups.patch

Toggle diff (76 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 9ddd1349da..b807e3879c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1348,6 +1348,7 @@ dist_patch_DATA = \
%D%/packages/patches/seahorse-gkr-use-0-on-empty-flags.patch \
%D%/packages/patches/seq24-rename-mutex.patch \
%D%/packages/patches/sharutils-CVE-2018-1000097.patch \
+ %D%/packages/patches/shepherd-set-supplementary-groups.patch \
%D%/packages/patches/shishi-fix-libgcrypt-detection.patch \
%D%/packages/patches/slim-session.patch \
%D%/packages/patches/slim-config.patch \
diff --git a/gnu/packages/admin.scm b/gnu/packages/admin.scm
index 6e5648d159..3f94b45623 100644
--- a/gnu/packages/admin.scm
+++ b/gnu/packages/admin.scm
@@ -201,7 +201,9 @@ and provides a \"top-like\" mode (monitoring).")
version ".tar.gz"))
(sha256
(base32
- "1xn6mb5bh8bpfgdrh09ja31jk0ln7bmxbbf0vjcqxkkixs2wl6sk"))))
+ "1xn6mb5bh8bpfgdrh09ja31jk0ln7bmxbbf0vjcqxkkixs2wl6sk"))
+ (patches
+ (search-patches "shepherd-set-supplementary-groups.patch"))))
(build-system gnu-build-system)
(arguments
'(#:configure-flags '("--localstatedir=/var")))
diff --git a/gnu/packages/patches/shepherd-set-supplementary-groups.patch b/gnu/packages/patches/shepherd-set-supplementary-groups.patch
new file mode 100644
index 0000000000..f72f7329f6
--- /dev/null
+++ b/gnu/packages/patches/shepherd-set-supplementary-groups.patch
@@ -0,0 +1,41 @@
+diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
+index bd7e379..74fed23 100644
+--- a/modules/shepherd/service.scm
++++ b/modules/shepherd/service.scm
+@@ -758,6 +758,28 @@ daemon writing FILE is running in a separate PID namespace."
+ (try-again)
+ (apply throw args)))))))
+
++(define (supplementary-gids user)
++ "Return a vector with the gid for each supplementary group USER belongs to.
++USER is the user name as a string."
++ ;; TODO: To find them, we loop through the group database, but maybe using
++ ;; glibc’s getgrouplist would be better. But it is not exported from Guile
++ ;; and it seems it is not part of POSIX (?).
++ (list->vector
++ (delete-duplicates
++ (dynamic-wind
++ (lambda () (setgrent))
++ (lambda ()
++ (let loop ((supgids '()))
++ (let ((group (getgrent)))
++ (define (user-among-group? group)
++ (member user (group:mem group)))
++ (match group
++ (#f supgids)
++ ((? user-among-group?)
++ (loop (cons (group:gid group) supgids)))
++ (else (loop supgids))))))
++ (lambda () (endgrent))))))
++
+ (define* (exec-command command
+ #:key
+ (user #f)
+@@ -826,6 +848,7 @@ false."
+ (when user
+ (catch #t
+ (lambda ()
++ (setgroups (supplementary-gids user))
+ (setuid (passwd:uid (getpw user))))
+ (lambda (key . args)
+ (format (current-error-port)
--
2.24.0
?