diff mbox series

[bug#67288] services: laminar: Add configuration option for supplementary groups

Message ID CAJ=RwfaSXfTeFn_H9Jme10hdYzeQnxTMakbJb2ksTZ+3km_aiA@mail.gmail.com
State New
Headers show
Series [bug#67288] services: laminar: Add configuration option for supplementary groups | expand

Commit Message

Thompson, David Nov. 19, 2023, 7:58 p.m. UTC
Hey everyone,

I started using Laminar CI for my personal server, but I had trouble
with the current system service. My server is configured to only allow
members of the "git" group access to the Git repositories, so the CI
job running as the "laminar" user couldn't do anything useful. This
patch adds a new configuration field for a list of supplementary
groups to be used for the "laminar" user and the service process.

- Dave

Comments

Ludovic Courtès Nov. 25, 2023, 3:25 p.m. UTC | #1
Hi,

"Thompson, David" <dthompson2@worcester.edu> skribis:

> I started using Laminar CI for my personal server, but I had trouble
> with the current system service. My server is configured to only allow
> members of the "git" group access to the Git repositories, so the CI
> job running as the "laminar" user couldn't do anything useful. This
> patch adds a new configuration field for a list of supplementary
> groups to be used for the "laminar" user and the service process.

Cc’ing Arun and Chris, who know better than me.  Is this a problem they
worked around so far?

> From ed62d885a5493f64779bc9c2a9b9978af8f61824 Mon Sep 17 00:00:00 2001
> Message-ID: <ed62d885a5493f64779bc9c2a9b9978af8f61824.1700423610.git.dthompson2@worcester.edu>
> From: David Thompson <dthompson2@worcester.edu>
> Date: Sun, 19 Nov 2023 14:46:52 -0500
> Subject: [PATCH] services: laminar: Add configuration option for supplementary
>  groups.
>
> * gnu/services/ci (<laminar-configuration>)[supplemental-groups]: New field.
> (laminar-shepherd-service): Exec laminard with supplementary groups.
> (laminar-account): Add supplementary groups to laminar user.
> * doc/guix.texi (Laminar): Document new configuration field.

[...]

> +@item @code{supplementary-groups} (default: @code{()})
> +Supplementary groups for the Laminar user account.

Perhaps mention the “git” group example you gave above?

Otherwise looks pretty harmless to me.

Ludo’.
Arun Isaac Nov. 26, 2023, midnight UTC | #2
Hi,

>> I started using Laminar CI for my personal server, but I had trouble
>> with the current system service. My server is configured to only allow
>> members of the "git" group access to the Git repositories, so the CI
>> job running as the "laminar" user couldn't do anything useful. This
>> patch adds a new configuration field for a list of supplementary
>> groups to be used for the "laminar" user and the service process.
>
> Cc’ing Arun and Chris, who know better than me.  Is this a problem they
> worked around so far?

This kind of problem requiring supplementary groups exists with many of
our services, not just the laminar service. I don't run into trouble
with the laminar service because git repos on my servers are usually
publicly readable by all users (including the laminar user).

To provide another example, I have similar trouble with our nginx
service not being able to access Unix sockets created by our fcgiwrap
service. Now, I work around this by having a special fcgiwrap service in
guix-forge[1]. This special guix-forge fcgiwrap service differs from the
guix fcgiwrap service in that it creates separate fcgiwrap instances for
each web application each with its own explicitly specified permissions.

[1]: https://git.systemreboot.net/guix-forge/tree/guix/forge/fcgiwrap.scm

I don't think the solution to this problem is to add a
`supplementary-groups' field to all our services. I'm not sure how, but
we need to compose things better. If we don't, we may find we need to
add some other field in the future and quickly be in a combinatorial
explosion.

Thinking out loud, the Guix service configuration system is really a
propagator network. So, maybe, the solution is to allow one service to
*extend* another service by specifying what supplementary groups it
should be a part of. This is more flexible than simply adding a
configuration field. Sorry to be quite vague here. Does this make any
sense? Happy to chat more.

Regards,
Arun
Thompson, David Nov. 26, 2023, 12:16 a.m. UTC | #3
Hi Arun,

On Sat, Nov 25, 2023 at 7:00 PM Arun Isaac <arunisaac@systemreboot.net> wrote:
>
>
> Hi,
>
> >> I started using Laminar CI for my personal server, but I had trouble
> >> with the current system service. My server is configured to only allow
> >> members of the "git" group access to the Git repositories, so the CI
> >> job running as the "laminar" user couldn't do anything useful. This
> >> patch adds a new configuration field for a list of supplementary
> >> groups to be used for the "laminar" user and the service process.
> >
> > Cc’ing Arun and Chris, who know better than me.  Is this a problem they
> > worked around so far?
>
> This kind of problem requiring supplementary groups exists with many of
> our services, not just the laminar service. I don't run into trouble
> with the laminar service because git repos on my servers are usually
> publicly readable by all users (including the laminar user).

I figured the existing users of this service had something like this
going on. I don't want to make the permissions looser on my own
server, though.

> To provide another example, I have similar trouble with our nginx
> service not being able to access Unix sockets created by our fcgiwrap
> service. Now, I work around this by having a special fcgiwrap service in
> guix-forge[1]. This special guix-forge fcgiwrap service differs from the
> guix fcgiwrap service in that it creates separate fcgiwrap instances for
> each web application each with its own explicitly specified permissions.

I also hack the nginx service to made the nginx user part of the git group:

https://git.dthompson.us/guix-config/tree/takemi-os.scm#n20

This hack causes all sorts of annoying side effects, which is why I
didn't want to go through it all again for the laminar service.

> [1]: https://git.systemreboot.net/guix-forge/tree/guix/forge/fcgiwrap.scm
>
> I don't think the solution to this problem is to add a
> `supplementary-groups' field to all our services. I'm not sure how, but
> we need to compose things better. If we don't, we may find we need to
> add some other field in the future and quickly be in a combinatorial
> explosion.

I agree that this indicates a missing means of composition, but in the
short term I think it's fine to do simple things that help people out
even though it's not the "right thing".

> Thinking out loud, the Guix service configuration system is really a
> propagator network. So, maybe, the solution is to allow one service to
> *extend* another service by specifying what supplementary groups it
> should be a part of. This is more flexible than simply adding a
> configuration field. Sorry to be quite vague here. Does this make any
> sense? Happy to chat more.

Providing a way to modify user accounts in services would be great!
It's not a problem I can work on solving, though, at least not now.

- Dave
Arun Isaac Nov. 26, 2023, 3:47 p.m. UTC | #4
Hi David,

> I figured the existing users of this service had something like this
> going on. I don't want to make the permissions looser on my own
> server, though.

In my case, the git repos are for publishing online. So, global read
permissions are acceptable. I understand your situation may be
different.

> but in the short term I think it's fine to do simple things that help
> people out even though it's not the "right thing".

I agree.

Regards,
Arun
Arun Isaac Dec. 6, 2023, 1:19 p.m. UTC | #5
Hi David,

>> but in the short term I think it's fine to do simple things that help
>> people out even though it's not the "right thing".
>
> I agree.

Please go ahead and push this patch if everything else checks out. I
don't mean to block this patch.

Thanks,
Arun
Thompson, David Dec. 28, 2023, 5:58 p.m. UTC | #6
Hi Arun,

On Wed, Dec 6, 2023 at 8:19 AM Arun Isaac <arunisaac@systemreboot.net> wrote:
>
>
> Hi David,
>
> >> but in the short term I think it's fine to do simple things that help
> >> people out even though it's not the "right thing".
> >
> > I agree.
>
> Please go ahead and push this patch if everything else checks out. I
> don't mean to block this patch.

Okay, pushed! Thanks!

- Dave
diff mbox series

Patch

From ed62d885a5493f64779bc9c2a9b9978af8f61824 Mon Sep 17 00:00:00 2001
Message-ID: <ed62d885a5493f64779bc9c2a9b9978af8f61824.1700423610.git.dthompson2@worcester.edu>
From: David Thompson <dthompson2@worcester.edu>
Date: Sun, 19 Nov 2023 14:46:52 -0500
Subject: [PATCH] services: laminar: Add configuration option for supplementary
 groups.

* gnu/services/ci (<laminar-configuration>)[supplemental-groups]: New field.
(laminar-shepherd-service): Exec laminard with supplementary groups.
(laminar-account): Add supplementary groups to laminar user.
* doc/guix.texi (Laminar): Document new configuration field.

Change-Id: Iebfdbb58ea8c6dfa22bb8f64f6463e3ad133d2f9
---
 doc/guix.texi       |  3 +++
 gnu/services/ci.scm | 42 ++++++++++++++++++++++++------------------
 2 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 94903fb5e2..854486c3ea 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33955,6 +33955,9 @@  Continuous Integration
 @item @code{home-directory} (default: @code{"/var/lib/laminar"})
 The directory for job configurations and run directories.
 
+@item @code{supplementary-groups} (default: @code{()})
+Supplementary groups for the Laminar user account.
+
 @item @code{bind-http} (default: @code{"*:8080"})
 The interface/port or unix socket on which laminard should listen for
 incoming connections to the web frontend.
diff --git a/gnu/services/ci.scm b/gnu/services/ci.scm
index 172f85fe8e..01cc7c7d86 100644
--- a/gnu/services/ci.scm
+++ b/gnu/services/ci.scm
@@ -31,6 +31,7 @@  (define-module (gnu services ci)
   #:export (laminar-configuration
             laminar-configuration?
             laminar-configuration-home-directory
+            laminar-configuration-supplementary-groups
             laminar-configuration-bind-http
             laminar-configuration-bind-rpc
             laminar-configuration-title
@@ -50,26 +51,28 @@  (define-module (gnu services ci)
 (define-record-type* <laminar-configuration>
   laminar-configuration make-laminar-configuration
   laminar-configuration?
-  (laminar          laminars-configuration-laminar
-                    (default laminar))
-  (home-directory   laminar-configuration-home-directory
-                    (default "/var/lib/laminar"))
-  (bind-http        laminar-configuration-bind-http
-                    (default "*:8080"))
-  (bind-rpc         laminar-configuration-bind-rpc
-                    (default "unix-abstract:laminar"))
-  (title            laminar-configuration-title
-                    (default "Laminar"))
-  (keep-rundirs     laminar-keep-rundirs
-                    (default 0))
-  (archive-url      laminar-archive-url
-                    (default #f))
-  (base-url         laminar-base-url
-                    (default #f)))
+  (laminar              laminars-configuration-laminar
+                        (default laminar))
+  (home-directory       laminar-configuration-home-directory
+                        (default "/var/lib/laminar"))
+  (supplementary-groups laminar-configuration-supplementary-groups
+                        (default '()))
+  (bind-http            laminar-configuration-bind-http
+                        (default "*:8080"))
+  (bind-rpc             laminar-configuration-bind-rpc
+                        (default "unix-abstract:laminar"))
+  (title                laminar-configuration-title
+                        (default "Laminar"))
+  (keep-rundirs         laminar-keep-rundirs
+                        (default 0))
+  (archive-url          laminar-archive-url
+                        (default #f))
+  (base-url             laminar-base-url
+                        (default #f)))
 
 (define laminar-shepherd-service
   (match-lambda
-    (($ <laminar-configuration> laminar home-directory
+    (($ <laminar-configuration> laminar home-directory supplementary-groups
                                 bind-http bind-rpc
                                 title keep-rundirs archive-url
                                 base-url)
@@ -102,7 +105,8 @@  (define laminar-shepherd-service
                                               #$base-url))
                               '()))
                       #:user "laminar"
-                      #:group "laminar"))
+                      #:group "laminar"
+                      #:supplementary-groups '#$supplementary-groups))
             (stop #~(make-kill-destructor)))))))
 
 (define (laminar-account config)
@@ -113,6 +117,8 @@  (define (laminar-account config)
         (user-account
          (name "laminar")
          (group "laminar")
+         (supplementary-groups
+          (laminar-configuration-supplementary-groups config))
          (system? #t)
          (comment "Laminar privilege separation user")
          (home-directory (laminar-configuration-home-directory config))

base-commit: 2ab5e449246f98b049888dde3c310f5b4a0a64a2
prerequisite-patch-id: 20e0bd5d1f3c88351c4991ef9c652dbded53bf9a
-- 
2.41.0