[bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.

[muradm <mail@muradm.net>]

[-- Attachment #1.1: Type: text/plain, Size: 52 bytes --]


Now accepts either string or user-group as group.


[-- Attachment #1.2: v4-0001-gnu-seatd-service-type-Should-use-seat-group.patch --]
[-- Type: text/x-patch, Size: 6728 bytes --]

From ad9323d5b3f774ee479cf16f41753dbbe2f90ff4 Mon Sep 17 00:00:00 2001
From: muradm <mail@muradm.net>
Date: Fri, 22 Jul 2022 07:09:54 +0300
Subject: [PATCH v4] gnu: seatd-service-type: Should use seat group.
To: 56690@debbugs.gnu.org

* gnu/services/desktop.scm (seatd-service-type): Uses "seat" group.
[extensions]: Added account-service-type with seatd-accounts.
(seatd-accounts): Conditionally produces list with "seat" group.
(<seatd-configuration>):
[user] Removed user field, since it is not going to be used.
[group] Change default value to "seat" user-group.
* doc/guix.texi: Mention that users may need to become members of
"seat" group and update default value for group field. Add
explanation on seatd.sock file. Remove dropped user field.
---
 doc/guix.texi            | 29 +++++++++++++++++++++++++----
 gnu/services/desktop.scm | 25 +++++++++++++++++++------
 gnu/tests/desktop.scm    |  9 +++++++++
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 023b48ae35..40c6e66439 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23189,6 +23189,30 @@ input), without requiring the applications needing access to be root.
   %base-services)
 
 @end lisp
+
+@code{seatd} operates over a UNIX domain socket, with @code{libseat}
+providing the client-side of the protocol. Then applications dealing
+with seat management (e.g. @code{sway}) connects to @code{seatd} via
+mentioned socket.
+
+When seat management is provided by @code{seatd}, @code{libseat} users
+that acquire resources provided by @code{seatd} should have permissions
+to access its UNIX domain socket. By default, @code{seatd-service-type}
+provides ``seat'' group. And @code{libseat} user should become its
+member.
+
+@lisp
+(user-account
+  (name "alice")
+  (group "users")
+  (supplementary-groups '("wheel"   ;allow use of sudo, etc.
+                          "seat"    ;interact with seatd
+                          "audio"   ;sound card
+                          "video"   ;video devices such as webcams
+                          "cdrom")) ;the good ol' CD-ROM
+  (comment "Bob's sister"))
+@end lisp
+
 @end defvr
 
 @deftp {Data Type} seatd-configuration
@@ -23198,10 +23222,7 @@ Configuration record for the seatd daemon service.
 @item @code{seatd} (default: @code{seatd})
 The seatd package to use.
 
-@item @code{user} (default: @samp{"root"})
-User to own the seatd socket.
-
-@item @code{group} (default: @samp{"users"})
+@item @code{group} (default: @samp{"seat"})
 Group to own the seatd socket.
 
 @item @code{socket} (default: @samp{"/run/seatd.sock"})
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index f891d1b5cc..2e7b751c1a 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -13,7 +13,7 @@
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2020 Reza Alizadeh Majd <r.majd@pantherx.org>
 ;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
-;;; Copyright © 2021 muradm <mail@muradm.net>
+;;; Copyright © 2021, 2022 muradm <mail@muradm.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -69,6 +69,7 @@ (define-module (gnu services desktop)
   #:use-module (guix records)
   #:use-module (guix packages)
   #:use-module (guix store)
+  #:use-module (guix ui)
   #:use-module (guix utils)
   #:use-module (guix gexp)
   #:use-module (srfi srfi-1)
@@ -1643,12 +1644,21 @@ (define polkit-wheel-service
 ;;; seatd-service-type -- minimal seat management daemon
 ;;;
 
+(define (seatd-make-group g)
+  (user-group (name g) (system? #t)))
+
+(define (seatd-group-sanitizer g)
+  (cond ((user-group? g) g)
+        ((string? g) (seatd-make-group g))
+        (#t (leave (G_ "seatd: '~a' is not a valid group-%") (object->string g)))))
+
 (define-record-type* <seatd-configuration> seatd-configuration
   make-seatd-configuration
   seatd-configuration?
   (seatd seatd-package (default seatd))
-  (user seatd-user (default "root"))
-  (group seatd-group (default "users"))
+  (group seatd-group
+         (default (seatd-make-group "seat"))
+         (sanitize seatd-group-sanitizer))
   (socket seatd-socket (default "/run/seatd.sock"))
   (logfile seatd-logfile (default "/var/log/seatd.log"))
   (loglevel seatd-loglevel (default "info")))
@@ -1662,8 +1672,7 @@ (define (seatd-shepherd-service config)
          (provision '(seatd elogind))
          (start #~(make-forkexec-constructor
                    (list #$(file-append (seatd-package config) "/bin/seatd")
-                         "-u" #$(seatd-user config)
-                         "-g" #$(seatd-group config))
+                         "-g" #$(user-group-name (seatd-group config)))
                    #:environment-variables
                    (list (string-append "SEATD_LOGLEVEL="
                                         #$(seatd-loglevel config))
@@ -1672,9 +1681,12 @@ (define (seatd-shepherd-service config)
                    #:log-file #$(seatd-logfile config)))
          (stop #~(make-kill-destructor)))))
 
+(define seatd-accounts
+  (match-lambda (($ <seatd-configuration> _ group) (list group))))
+
 (define seatd-environment
   (match-lambda
-    (($ <seatd-configuration> _ _ _ socket)
+    (($ <seatd-configuration> _ _ socket)
      `(("SEATD_SOCK" . ,socket)))))
 
 (define seatd-service-type
@@ -1685,6 +1697,7 @@ (define seatd-service-type
 applications needing access to be root.")
    (extensions
     (list
+     (service-extension account-service-type seatd-accounts)
      (service-extension session-environment-service-type seatd-environment)
      ;; TODO: once cgroups is separate dependency we should not mount it here
      ;; for now it is mounted here, because elogind mounts it
diff --git a/gnu/tests/desktop.scm b/gnu/tests/desktop.scm
index 25971f9225..6fe6ec21be 100644
--- a/gnu/tests/desktop.scm
+++ b/gnu/tests/desktop.scm
@@ -255,6 +255,15 @@ (define (sock-var-sock var)
                    (socks (map wait-for-unix-socket-m socks)))
                 (and (= 2 (length socks)) (every identity socks)))))
 
+          (test-equal "seatd.sock ownership"
+            '("root" "seat")
+            `(,(marionette-eval
+                '(passwd:name (getpwuid (stat:uid (stat "/run/seatd.sock"))))
+                marionette)
+              ,(marionette-eval
+                '(group:name (getgrgid (stat:gid (stat "/run/seatd.sock"))))
+                marionette)))
+
           (test-assert "greetd is ready"
             (begin
               (marionette-type "ps -C greetd -o pid,args --no-headers > ps-greetd\n"
-- 
2.37.1


[-- Attachment #1.3: Type: text/plain, Size: 3909 bytes --]



Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

> Am Dienstag, dem 09.08.2022 um 22:47 +0300 schrieb muradm:
>> There is no such specification as login manager or what ever. 
>> User
>> is any one/thing acquiring resources via seat management. It is
>> perfectly fine to run mingetty, login into bash and from 
>> command line
>> start sway that will use libseat to acquire video for instance. 
>> Who is
>> user here?
>>
>> There is also no display manager as it was before. Please see 
>> my
>> explanation to unmatched-paren:
>> https://debbugs.gnu.org/cgi/bugreport.cgi?msg=46;bug=56690
>> What is sway in this usecase, it is not a user (like you or 
>> me),
>> it is not a display manager (as gdm, sddm etc.). It is just
>> application requiring video card (not only) resource, which
>> it instead of having exclusive root access, uses libseat to
>> acquire it in "seat managy" way. And greetd does/should not
>> care about seatd/libseat until it is not required to acquire
>> resources in "seat managy" way. Instead it is a greeter which
>> is totatly customizable, could be even a bash script or small
>> suckless-like application or else.
>>
>> This is the point of seatd I suppose, to do one thing only
>> without enforcing on who should do what.
>>
>> Thus, none of your proposals are suitable, and I can't come up
>> with something better than "seat management user" or "libseat
>> user". However in my opinion, the one who commits into such
>> setup, should be aware of what is seatd libseat and how, why to
>> interact with it.
> I think you're mixing user and application here, which makes 
> explaining
> this to others difficult.  For instance, GDM is both an 
> application
> (display manager) and a user launching this application. 
> Likewise for
> most other display managers.  Thus, there is a 1:1 mapping 
> between
> users and applications.
>
> With seatd, from what I understand, there is no such mapping. 
> However,
> given your description, the following is unclear: Does alice 
> need to be
> in the seat group to run bash?  To run sway?  To run sway *only 
> if not
> having talked to greetd first*?
>
>> > > > > +  (group seatd-group (default "seat"))
>> > > > > +  (existing-group? seatd-existing-group? (default #f))
>> > > > AFAIK this is not necessary.  accounts-service-type can
>> > > > handle
>> > > > multiple eq? groups, so as long as you're careful with 
>> > > > what
>> > > > you put
>> > > > into group, you shouldn't get an error.
>> > > ok field removed
>> > Note ‘eq?’ groups here.  In other words, you should be able 
>> > to
>> > take a
>> > group (not just a group name) for the group field, sanitize 
>> > the
>> > field
>> > so that it will always be a group, and then use that group in
>> > seatd-
>> > accounts (see the second option mentioned in
>> > <79341a82bf9cd5fc6c2227255095f3fe2927dcbe.camel@ist.tugraz.at>).
>> > If
>> > for instance instead of seat, you wanted the video group, you
>> > would
>> > have to take the one from %base-groups, rather than creating 
>> > a
>> > new one.
>> Sorry, but I'm not so proficient in english as you. I can only
>> speculate on what is written here. And that reference does not
>> say anything to me, even duck duck go gives single result, it 
>> is
>> your message. Could you please be more specific here, and/or
>> provide more useful hyperlink style references. Thanks in 
>> advance.
> I'll explain it in terms of lisp:
>
> (define seat1 (user-group (name "seat") (system #t))
> (define seat2 (user-group (name "seat") (system #t))
> (operating-system (groups (list seat1 seat1))) ; works, eq?
> (operating-system (groups (list seat2 seat2))) ; works, eq?
> (operating-system (groups (list seat1 seat2))) ; doesn't work
>
> For field sanitizers, see define-record-type*.
>
> Cheers


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]