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
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 38438@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 38438
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch