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

Previous Next

Package: guix-patches;

Reported by: muradm <mail <at> muradm.net>

Date: Fri, 22 Jul 2022 04:28:01 UTC

Severity: normal

Tags: patch

Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 56690 in the body.
You can then email your comments to 56690 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Fri, 22 Jul 2022 04:28:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to muradm <mail <at> muradm.net>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Fri, 22 Jul 2022 04:28:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: seatd-service-type: Should use seat group.
Date: Fri, 22 Jul 2022 07:27:45 +0300
* gnu/services/desktop.scm (seatd-service-type): Uses "seat" group.
[extensions]: Added account-service-type with %seatd-accounts.
(%seatd-accounts): List with "seat" group.
(<seatd-configuration>): [group] Change default value to "seat".
* doc/guix.texi: Mention that users may need to become members of
"seat" group and update default value for group field.
---
 doc/guix.texi            | 18 +++++++++++++++++-
 gnu/services/desktop.scm |  8 ++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 3c5864ec1a..750ed9b121 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23151,6 +23151,22 @@ input), without requiring the applications needing access to be root.
   %base-services)
 
 @end lisp
+
+Users which are going to interact with @code{seatd} daemon while logged in
+should be added to @code{seat} group. For instance:
+
+@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
@@ -23163,7 +23179,7 @@ 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 29a3722f1b..0d7cd71732 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -13,7 +13,7 @@
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2020 Reza Alizadeh Majd <r.majd <at> pantherx.org>
 ;;; Copyright © 2021 Brice Waegeneire <brice <at> waegenei.re>
-;;; Copyright © 2021 muradm <mail <at> muradm.net>
+;;; Copyright © 2021, 2022 muradm <mail <at> muradm.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1646,7 +1646,7 @@ (define-record-type* <seatd-configuration> seatd-configuration
   seatd-configuration?
   (seatd seatd-package (default seatd))
   (user seatd-user (default "root"))
-  (group seatd-group (default "users"))
+  (group seatd-group (default "seat"))
   (socket seatd-socket (default "/run/seatd.sock"))
   (logfile seatd-logfile (default "/var/log/seatd.log"))
   (loglevel seatd-loglevel (default "info")))
@@ -1670,6 +1670,9 @@ (define (seatd-shepherd-service config)
                    #:log-file #$(seatd-logfile config)))
          (stop #~(make-kill-destructor)))))
 
+(define %seatd-accounts
+  (list (user-group (name "seat") (system? #t))))
+
 (define seatd-environment
   (match-lambda
     (($ <seatd-configuration> _ _ _ socket)
@@ -1683,6 +1686,7 @@ (define seatd-service-type
 applications needing access to be root.")
    (extensions
     (list
+     (service-extension account-service-type (const %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
-- 
2.36.1





Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Sun, 24 Jul 2022 16:29:02 GMT) Full text and rfc822 format available.

Message #8 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: "(" <paren <at> disroot.org>
To: "muradm" <mail <at> muradm.net>, <56690 <at> debbugs.gnu.org>
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Sun, 24 Jul 2022 17:28:43 +0100
Because patches with replies are more likely to be visible: LGTM :)

    -- (




Added indication that bug 56690 blocks56971 Request was from Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at> to control <at> debbugs.gnu.org. (Thu, 04 Aug 2022 11:09:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Fri, 05 Aug 2022 08:11:01 GMT) Full text and rfc822 format available.

Message #13 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
To: muradm <mail <at> muradm.net>, 56690 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: seatd-service-type: Should use seat group.
Date: Fri, 05 Aug 2022 10:10:40 +0200
Am Freitag, dem 22.07.2022 um 07:27 +0300 schrieb muradm:
> * gnu/services/desktop.scm (seatd-service-type): Uses "seat" group.
> [extensions]: Added account-service-type with %seatd-accounts.
> (%seatd-accounts): List with "seat" group.
> (<seatd-configuration>): [group] Change default value to "seat".
> * doc/guix.texi: Mention that users may need to become members of
> "seat" group and update default value for group field.
Note, that your current patch adds a little asymmetry.  Even if you
configure seatd to use a group different from seat, a (now useless)
seat group will be created.

There are (at least) two possible fixes for this:
1. Disable configuration for the group altogether, marking the field as
deprecated.
2. Change the field into one that accepts a group.  Also sanitize the
field so that if a string such as "seat" is provided, it is turned into
a group.  Then make seatd-accounts return this group.

Cheers




Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Sat, 06 Aug 2022 20:47:01 GMT) Full text and rfc822 format available.

Message #16 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: muradm <mail <at> muradm.net>
Cc: 56690 <at> debbugs.gnu.org
Subject: Re: bug#56690: [PATCH] gnu: seatd-service-type: Should use seat group.
Date: Sat, 06 Aug 2022 22:46:49 +0200
Hi,

muradm <mail <at> muradm.net> skribis:

> * gnu/services/desktop.scm (seatd-service-type): Uses "seat" group.
> [extensions]: Added account-service-type with %seatd-accounts.
> (%seatd-accounts): List with "seat" group.
> (<seatd-configuration>): [group] Change default value to "seat".
> * doc/guix.texi: Mention that users may need to become members of
> "seat" group and update default value for group field.

I guess I’m missing some context: is this fixing a bug currently
present?  (Apologies if this has been discussed elsewhere!)

> +Users which are going to interact with @code{seatd} daemon while logged in

s/which/who/

> +should be added to @code{seat} group. For instance:
> +
> +@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"))

The problem I see with this extra doc is that even I wouldn’t know how
to tell whether I’m going to “interact with seatd”.  Fundamentally it’s
not something I really care about.  :-)

How could we improve on this?  Like, if this is important, should it be
the default?

Thanks,
Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Sat, 06 Aug 2022 20:51:01 GMT) Full text and rfc822 format available.

Message #19 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: "(" <paren <at> disroot.org>
To: Ludovic Courtès <ludo <at> gnu.org>, "muradm" <mail <at> muradm.net>
Cc: 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Sat, 06 Aug 2022 21:50:27 +0100
On Sat Aug 6, 2022 at 9:46 PM BST, Ludovic Courtès wrote:
> I guess I’m missing some context: is this fixing a bug currently
> present?  (Apologies if this has been discussed elsewhere!)

This is one of two patches that fix a problem where any greetd greeter
more complex than agreety hangs on boot, basically rendering greetd
useless. I think the underlying cause is their being unable to connect
to seatd.sock?

At least, that's the symptom I know about. I'm not sure whether there
are others.

    -- (




Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Sun, 07 Aug 2022 17:42:02 GMT) Full text and rfc822 format available.

Message #22 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 56690 <at> debbugs.gnu.org
Subject: Re: bug#56690: [PATCH] gnu: seatd-service-type: Should use seat group.
Date: Sun, 07 Aug 2022 20:28:58 +0300
[Message part 1 (text/plain, inline)]
Hi,

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi,
>
> muradm <mail <at> muradm.net> skribis:
>
>> * gnu/services/desktop.scm (seatd-service-type): Uses "seat" 
>> group.
>> [extensions]: Added account-service-type with %seatd-accounts.
>> (%seatd-accounts): List with "seat" group.
>> (<seatd-configuration>): [group] Change default value to 
>> "seat".
>> * doc/guix.texi: Mention that users may need to become members 
>> of
>> "seat" group and update default value for group field.
>
> I guess I’m missing some context: is this fixing a bug currently
> present?  (Apologies if this has been discussed elsewhere!)
>

Not really a bug, but misconfiguration i suppose. Started here 
with
commit about month or two ago:

https://lists.gnu.org/archive/html/guix-devel/2022-08/msg00021.html

Basically, with original configuration, greeter was in the wheel 
group
which allowed it to communicate with seatd over /run/seatd.sock.

>> +Users which are going to interact with @code{seatd} daemon 
>> while logged in
>
> s/which/who/
>

With above fix, wheel and other groups were removed. While it was 
not
affecting default greeter agretty, some people including me, use
graphical greeter gtkgreet or others based on sway. Then sway with
greeter started by greetd needs to communicate with seatd. Due to
the fact of missing permission, greeter just dies with blank 
screen.

So "users which are going to interact" basically users who want
to run sway, or anything else requiring libseat based seat 
management
present.

>> +should be added to @code{seat} group. For instance:
>> +
>> +@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"))
>
> The problem I see with this extra doc is that even I wouldn’t 
> know how
> to tell whether I’m going to “interact with seatd”. 
> Fundamentally it’s
> not something I really care about.  :-)
>
> How could we improve on this?  Like, if this is important, 
> should it be
> the default?
>

Two options, a) users who want greetd/seatd setup normally 
advanced
users wishing to get away from systemd/logind/dbus world, so they
probably was to be aware of what is going on; b) copy a piece of
documentation from seatd, explaining seatd.sock maybe. Other than
that I could ask the same question about video, audio etc. groups 
:)

> Thanks,
> Ludo’.

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Sun, 07 Aug 2022 20:12:02 GMT) Full text and rfc822 format available.

Message #25 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Ludovic Courtès <ludo <at> gnu.org>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Sun, 07 Aug 2022 23:05:29 +0300
[Message part 1 (text/plain, inline)]
here is updated patch:
- group is now correctly configurable
- dropped user field as it is mostlikely pointless
- group is created if necessary
- documentation updated adding mentioning of seatd.sock 
 permissions
- adding test case for seatd.sock ownership

thanks in advance,
muradm

[v2-0001-gnu-seatd-service-type-Should-use-seat-group.patch (text/x-patch, inline)]
From edf954714a71ea3c1b8a872df40ed3735dff10f8 Mon Sep 17 00:00:00 2001
From: muradm <mail <at> muradm.net>
Date: Fri, 22 Jul 2022 07:09:54 +0300
Subject: [PATCH v2] gnu: seatd-service-type: Should use seat group.
To: 56690 <at> 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] Drop user field, since it is not going to be used.
[group] Change default value to "seat".
[existing-group?] Add field which controls if group should be
created or not.
* 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            | 32 ++++++++++++++++++++++++++++----
 gnu/services/desktop.scm | 15 +++++++++++----
 gnu/tests/desktop.scm    |  9 +++++++++
 3 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 21cee4e369..cb896fedb4 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23139,6 +23139,29 @@ 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 mamanagement is provided by @code{seatd}, 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 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
@@ -23148,12 +23171,13 @@ 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{existing-group?} (default: @samp{#f})
+If group specified in @code{group} field is pre-existing,
+or should be created by @code{seatd-service-type}.
+
 @item @code{socket} (default: @samp{"/run/seatd.sock"})
 Where to create the seatd socket.
 
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 29a3722f1b..9a36927b9f 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -13,7 +13,7 @@
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2020 Reza Alizadeh Majd <r.majd <at> pantherx.org>
 ;;; Copyright © 2021 Brice Waegeneire <brice <at> waegenei.re>
-;;; Copyright © 2021 muradm <mail <at> muradm.net>
+;;; Copyright © 2021, 2022 muradm <mail <at> muradm.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1645,8 +1645,8 @@ (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 "seat"))
+  (existing-group? seatd-existing-group? (default #f))
   (socket seatd-socket (default "/run/seatd.sock"))
   (logfile seatd-logfile (default "/var/log/seatd.log"))
   (loglevel seatd-loglevel (default "info")))
@@ -1660,7 +1660,6 @@ (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))
                    #:environment-variables
                    (list (string-append "SEATD_LOGLEVEL="
@@ -1670,6 +1669,13 @@ (define (seatd-shepherd-service config)
                    #:log-file #$(seatd-logfile config)))
          (stop #~(make-kill-destructor)))))
 
+(define seatd-accounts
+  (match-lambda
+    (($ <seatd-configuration> _ group existing-group?)
+     `(,@(if existing-group?  '() (list (user-group
+                                         (name group)
+                                         (system? #t))))))))
+
 (define seatd-environment
   (match-lambda
     (($ <seatd-configuration> _ _ _ socket)
@@ -1683,6 +1689,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

[Message part 3 (text/plain, inline)]

muradm <mail <at> muradm.net> writes:

> [[PGP Signed Part:Undecided]]
>
> Hi,
>
> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Hi,
>>
>> muradm <mail <at> muradm.net> skribis:
>>
>>> * gnu/services/desktop.scm (seatd-service-type): Uses "seat" 
>>> group.
>>> [extensions]: Added account-service-type with %seatd-accounts.
>>> (%seatd-accounts): List with "seat" group.
>>> (<seatd-configuration>): [group] Change default value to 
>>> "seat".
>>> * doc/guix.texi: Mention that users may need to become members 
>>> of
>>> "seat" group and update default value for group field.
>>
>> I guess I’m missing some context: is this fixing a bug 
>> currently
>> present?  (Apologies if this has been discussed elsewhere!)
>>
>
> Not really a bug, but misconfiguration i suppose. Started here 
> with
> commit about month or two ago:
>
> https://lists.gnu.org/archive/html/guix-devel/2022-08/msg00021.html
>
> Basically, with original configuration, greeter was in the wheel 
> group
> which allowed it to communicate with seatd over /run/seatd.sock.
>
>>> +Users which are going to interact with @code{seatd} daemon 
>>> while
>>> logged in
>>
>> s/which/who/
>>
>
> With above fix, wheel and other groups were removed. While it 
> was not
> affecting default greeter agretty, some people including me, use
> graphical greeter gtkgreet or others based on sway. Then sway 
> with
> greeter started by greetd needs to communicate with seatd. Due 
> to
> the fact of missing permission, greeter just dies with blank 
> screen.
>
> So "users which are going to interact" basically users who want
> to run sway, or anything else requiring libseat based seat 
> management
> present.
>
>>> +should be added to @code{seat} group. For instance:
>>> +
>>> +@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"))
>>
>> The problem I see with this extra doc is that even I wouldn’t 
>> know
>> how
>> to tell whether I’m going to “interact with seatd”. 
>> Fundamentally
>> it’s
>> not something I really care about.  :-)
>>
>> How could we improve on this?  Like, if this is important, 
>> should it
>> be
>> the default?
>>
>
> Two options, a) users who want greetd/seatd setup normally 
> advanced
> users wishing to get away from systemd/logind/dbus world, so 
> they
> probably was to be aware of what is going on; b) copy a piece of
> documentation from seatd, explaining seatd.sock maybe. Other 
> than
> that I could ask the same question about video, audio etc. 
> groups :)
>
>> Thanks,
>> Ludo’.
>
> [[End of PGP Signed Part]]

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Sun, 07 Aug 2022 20:47:02 GMT) Full text and rfc822 format available.

Message #28 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
Cc: 56690 <at> debbugs.gnu.org
Subject: Re: [PATCH] gnu: seatd-service-type: Should use seat group.
Date: Sun, 07 Aug 2022 23:45:54 +0300
[Message part 1 (text/plain, inline)]
Fixed in v2.

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

> Am Freitag, dem 22.07.2022 um 07:27 +0300 schrieb muradm:
>> * gnu/services/desktop.scm (seatd-service-type): Uses "seat" 
>> group.
>> [extensions]: Added account-service-type with %seatd-accounts.
>> (%seatd-accounts): List with "seat" group.
>> (<seatd-configuration>): [group] Change default value to 
>> "seat".
>> * doc/guix.texi: Mention that users may need to become members 
>> of
>> "seat" group and update default value for group field.
> Note, that your current patch adds a little asymmetry.  Even if 
> you
> configure seatd to use a group different from seat, a (now 
> useless)
> seat group will be created.
>
> There are (at least) two possible fixes for this:
> 1. Disable configuration for the group altogether, marking the 
> field as
> deprecated.
> 2. Change the field into one that accepts a group.  Also 
> sanitize the
> field so that if a string such as "seat" is provided, it is 
> turned into
> a group.  Then make seatd-accounts return this group.
>
> Cheers

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 08 Aug 2022 06:09:02 GMT) Full text and rfc822 format available.

Message #31 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
To: muradm <mail <at> muradm.net>, Ludovic Courtès
 <ludo <at> gnu.org>,  56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Mon, 08 Aug 2022 08:08:53 +0200
Am Sonntag, dem 07.08.2022 um 23:05 +0300 schrieb muradm:

> * 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] Drop user field, since it is not going to be used.
Removed field.
> [group] Change default value to "seat".
> [existing-group?] Add field which controls if group should be
> created or not.
Would be Added field, but see below.
> * 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.

> +When seat mamanagement is provided by @code{seatd}, users that
> acquire
management.
> +resources provided by @code{seatd} should have permissions to access
> +its UNIX domain socket. By default, @code{seatd-service-type}
> provides
> +``seat'' group. And user should become its member.
Which user?  Closely related, who acquires resources provided by
@code{seatd}?  Just the greeter?  A regular user logging in?
What access level is needed/provided?  Read access?  Write access?


> +  (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.

Cheers




Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 08 Aug 2022 09:00:02 GMT) Full text and rfc822 format available.

Message #34 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: "(" <paren <at> disroot.org>
Cc: muradm <mail <at> muradm.net>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Mon, 08 Aug 2022 10:58:53 +0200
Hi,

"(" <paren <at> disroot.org> skribis:

> On Sat Aug 6, 2022 at 9:46 PM BST, Ludovic Courtès wrote:
>> I guess I’m missing some context: is this fixing a bug currently
>> present?  (Apologies if this has been discussed elsewhere!)
>
> This is one of two patches that fix a problem where any greetd greeter
> more complex than agreety hangs on boot, basically rendering greetd
> useless. I think the underlying cause is their being unable to connect
> to seatd.sock?
>
> At least, that's the symptom I know about. I'm not sure whether there
> are others.

Is there a bug report, and do we have system tests for this
functionality?

I admit I know little about greetd and cases where it might be used.
Having system tests for that would help make sure the relevant
functionality works.

Thanks,
Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 08 Aug 2022 09:13:02 GMT) Full text and rfc822 format available.

Message #37 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: "(" <paren <at> disroot.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: muradm <mail <at> muradm.net>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Mon, 08 Aug 2022 10:12:51 +0100
On Mon Aug 8, 2022 at 9:58 AM BST, Ludovic Courtès wrote:
> Is there a bug report, and do we have system tests for this
> functionality?

I don't believe there are system tests for greetd, no. There is
a bug report, though: <https://issues.guix.gnu.org/56971>.

> I admit I know little about greetd and cases where it might be used.

As I understand it, greetd is a daemon that handles the sensitive parts
of display managers, which it calls 'greeters'. It allows you to write a
login program without having to write those difficult and sensitive parts
by simply writing a GUI that sends JSON messages to the socket when it
gets input.

So the problem is some greeters try to talk to seatd, but since they
don't have the right permissions, they bail out.

    -- (




Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 08 Aug 2022 18:56:02 GMT) Full text and rfc822 format available.

Message #40 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Mon, 08 Aug 2022 21:50:43 +0300
[v3-0001-gnu-seatd-service-type-Should-use-seat-group.patch (text/x-patch, inline)]
From e733977a5cbd2bb34cf129908b16a4a0af830f02 Mon Sep 17 00:00:00 2001
From: muradm <mail <at> muradm.net>
Date: Fri, 22 Jul 2022 07:09:54 +0300
Subject: [PATCH v3] gnu: seatd-service-type: Should use seat group.
To: 56690 <at> 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".
* 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 | 14 +++++++++-----
 gnu/tests/desktop.scm    |  9 +++++++++
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 9a6a5c307d..92701fca10 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -23171,6 +23171,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
@@ -23180,10 +23204,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 29a3722f1b..fbd5a46a06 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -13,7 +13,7 @@
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2020 Reza Alizadeh Majd <r.majd <at> pantherx.org>
 ;;; Copyright © 2021 Brice Waegeneire <brice <at> waegenei.re>
-;;; Copyright © 2021 muradm <mail <at> muradm.net>
+;;; Copyright © 2021, 2022 muradm <mail <at> muradm.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1645,8 +1645,7 @@ (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 "seat"))
   (socket seatd-socket (default "/run/seatd.sock"))
   (logfile seatd-logfile (default "/var/log/seatd.log"))
   (loglevel seatd-loglevel (default "info")))
@@ -1660,7 +1659,6 @@ (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))
                    #:environment-variables
                    (list (string-append "SEATD_LOGLEVEL="
@@ -1670,9 +1668,14 @@ (define (seatd-shepherd-service config)
                    #:log-file #$(seatd-logfile config)))
          (stop #~(make-kill-destructor)))))
 
+(define seatd-accounts
+  (match-lambda
+    (($ <seatd-configuration> _ group)
+     (list (user-group (name group) (system? #t))))))
+
 (define seatd-environment
   (match-lambda
-    (($ <seatd-configuration> _ _ _ socket)
+    (($ <seatd-configuration> _ _ socket)
      `(("SEATD_SOCK" . ,socket)))))
 
 (define seatd-service-type
@@ -1683,6 +1686,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

[Message part 2 (text/plain, inline)]
Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at> writes:

> Am Sonntag, dem 07.08.2022 um 23:05 +0300 schrieb muradm:
>
>> * 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] Drop user field, since it is not going to be used.
> Removed field.
done

>> [group] Change default value to "seat".
>> [existing-group?] Add field which controls if group should be
>> created or not.
> Would be Added field, but see below.
obsolete

>> * 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.
>
>> +When seat mamanagement is provided by @code{seatd}, users that
>> acquire
> management.
done

>> +resources provided by @code{seatd} should have permissions to 
>> access
>> +its UNIX domain socket. By default, @code{seatd-service-type}
>> provides
>> +``seat'' group. And user should become its member.
> Which user?  Closely related, who acquires resources provided by
> @code{seatd}?  Just the greeter?  A regular user logging in?
> What access level is needed/provided?  Read access?  Write 
> access?
While I understand what you are saying, for me user is fine, and I
can't come up with better description, as my eyes too blurred on
this subject. Anyway for now I specified it as "libseat user".

>> +  (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

> Cheers

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 08 Aug 2022 19:03:01 GMT) Full text and rfc822 format available.

Message #43 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: "\(" <paren <at> disroot.org>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Mon, 08 Aug 2022 21:55:18 +0300
[Message part 1 (text/plain, inline)]
Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi,
>
> "(" <paren <at> disroot.org> skribis:
>
>> On Sat Aug 6, 2022 at 9:46 PM BST, Ludovic Courtès wrote:
>>> I guess I’m missing some context: is this fixing a bug 
>>> currently
>>> present?  (Apologies if this has been discussed elsewhere!)
>>
>> This is one of two patches that fix a problem where any greetd 
>> greeter
>> more complex than agreety hangs on boot, basically rendering 
>> greetd
>> useless. I think the underlying cause is their being unable to 
>> connect
>> to seatd.sock?
>>
>> At least, that's the symptom I know about. I'm not sure whether 
>> there
>> are others.
>
> Is there a bug report, and do we have system tests for this
> functionality?
Problem started with conversation on guix-devel, and related 
commit.
Last message of thread:
https://lists.gnu.org/archive/html/guix-devel/2022-08/msg00021.html

I was travelling and missed that change, when I
"guix pull && guix system reconfigure"ed at home and realized the
problem, I submitted fixes in the form of two patches 56690 and 
56699.

Then I was asked to open a bug report in guix-devel list, which 
is:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56971

>
> I admit I know little about greetd and cases where it might be 
> used.
> Having system tests for that would help make sure the relevant
> functionality works.
There is:

 make check-system TESTS="minimal-desktop"

Patches in 56690 and 56699 now include the tests for this case as 
well.

>
> Thanks,
> Ludo’.

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 08 Aug 2022 19:51:02 GMT) Full text and rfc822 format available.

Message #46 received at submit <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: "(" <paren <at> disroot.org>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 56690 <at> debbugs.gnu.org,
 guix-patches <at> gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Mon, 08 Aug 2022 22:44:37 +0300
[Message part 1 (text/plain, inline)]
"(" <paren <at> disroot.org> writes:

> On Mon Aug 8, 2022 at 9:58 AM BST, Ludovic Courtès wrote:
>> Is there a bug report, and do we have system tests for this
>> functionality?
>
> I don't believe there are system tests for greetd, no. There is
> a bug report, though: <https://issues.guix.gnu.org/56971>.
>
>> I admit I know little about greetd and cases where it might be 
>> used.
>
> As I understand it, greetd is a daemon that handles the 
> sensitive parts
> of display managers, which it calls 'greeters'. It allows you to 
> write a
> login program without having to write those difficult and 
> sensitive parts
> by simply writing a GUI that sends JSON messages to the socket 
> when it
> gets input.
For greetd/greeter this is fine explanation.

> So the problem is some greeters try to talk to seatd, but since 
> they
> don't have the right permissions, they bail out.
To be more correct here, greeter that requires both talking to 
greetd
and talking to seatd via libseat. Suppose gtkgreet which is 
running
with sway. So greetd will start greeter which is
"sway -c config-which-starts-gtkgreet.conf". Now you have two 
processes
in the scope of greeter, one is sway which has to talk to swatd 
via
libseat and the other is gtkgreet which is going to talk with 
greetd.

The one who bails out is sway here due to lack of permissions for
seatd.sock for talking to seatd via libseat.

>
>     -- (

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 08 Aug 2022 19:51:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Tue, 09 Aug 2022 06:58:02 GMT) Full text and rfc822 format available.

Message #52 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
To: muradm <mail <at> muradm.net>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Tue, 09 Aug 2022 08:57:29 +0200
Am Montag, dem 08.08.2022 um 21:50 +0300 schrieb muradm:
> > Which user?  Closely related, who acquires resources provided by
> > @code{seatd}?  Just the greeter?  A regular user logging in?
> > What access level is needed/provided?  Read access?  Write 
> > access?
> While I understand what you are saying, for me user is fine, and I
> can't come up with better description, as my eyes too blurred on
> this subject. Anyway for now I specified it as "libseat user".
I don't think this really aids us here – it instead lets us ask who is
a "libseat user".  Perhaps you want to specify "login managers" like
greetd or gdm/sddm/etc. explicitly here?  Also, (when) do regular users
have to be in the seat group?

> > > +  (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 <at> 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.

Cheers





Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Tue, 09 Aug 2022 20:09:01 GMT) Full text and rfc822 format available.

Message #55 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Tue, 09 Aug 2022 22:47:01 +0300
[Message part 1 (text/plain, inline)]
Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at> writes:

> Am Montag, dem 08.08.2022 um 21:50 +0300 schrieb muradm:
>> > Which user?  Closely related, who acquires resources provided 
>> > by
>> > @code{seatd}?  Just the greeter?  A regular user logging in?
>> > What access level is needed/provided?  Read access?  Write
>> > access?
>> While I understand what you are saying, for me user is fine, 
>> and I
>> can't come up with better description, as my eyes too blurred 
>> on
>> this subject. Anyway for now I specified it as "libseat user".
> I don't think this really aids us here – it instead lets us ask 
> who is
> a "libseat user".  Perhaps you want to specify "login managers" 
> like
> greetd or gdm/sddm/etc. explicitly here?  Also, (when) do 
> regular users
> have to be in the seat group?
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.

>> > > +  (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 <at> 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.

> Cheers

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Wed, 10 Aug 2022 08:08:02 GMT) Full text and rfc822 format available.

Message #58 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
To: muradm <mail <at> muradm.net>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Wed, 10 Aug 2022 10:07:07 +0200
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 <at> 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




Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Sat, 13 Aug 2022 17:55:02 GMT) Full text and rfc822 format available.

Message #61 received at 56690 <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 56690 <at> debbugs.gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Sat, 13 Aug 2022 20:39:25 +0300
[Message part 1 (text/plain, inline)]
Liliana Marie Prikler <liliana.prikler <at> 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.
I don't think that I miss, instead I intend to generalize as much
as possible. I suppose it is better to say, seat management can be
used by anyone or anything where greeter would be an example of
anything, and logged in user an example of anyone.

> 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?
Alice needs to be in seat group if any application and/or
script is going to be using libseat for acquiring resources in
"seat managy" way, in order to have access to seatd.sock.

> To run sway?
Since sway is aciqyuring resources using libseat in "seat managy"
way, then Alice will have to be in seat group to access 
seatd.sock.

> To run sway *only if not having talked to greetd first*?
greetd is unrelated here, as greetd by it self is not acquiring
resources in "seat managy" way. Currently no greeter for greetd
also talks via libseat to seatd _directly_. But special case of
gtkgreet which requires wayland compositor, which is sway, creates
indirect relation of "seat managy" resources acquisiion using
libseat. This indirect relation requiring user of greeter to be
a member of seat group.

>> > > > > +  (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 <at> 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*.
I know how eq? works. I don't understand what do you want me to
do with service configuration.

> Cheers

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 22 Aug 2022 20:21:02 GMT) Full text and rfc822 format available.

Message #64 received at submit <at> debbugs.gnu.org (full text, mbox):

From: muradm <mail <at> muradm.net>
To: Liliana Marie Prikler <liliana.prikler <at> ist.tugraz.at>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 56690 <at> debbugs.gnu.org,
 guix-patches <at> gnu.org
Subject: Re: [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat
 group.
Date: Mon, 22 Aug 2022 23:17:59 +0300
[Message part 1 (text/plain, inline)]
Now accepts either string or user-group as group.

[v4-0001-gnu-seatd-service-type-Should-use-seat-group.patch (text/x-patch, inline)]
From ad9323d5b3f774ee479cf16f41753dbbe2f90ff4 Mon Sep 17 00:00:00 2001
From: muradm <mail <at> muradm.net>
Date: Fri, 22 Jul 2022 07:09:54 +0300
Subject: [PATCH v4] gnu: seatd-service-type: Should use seat group.
To: 56690 <at> 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 <at> tobias.gr>
 ;;; Copyright © 2020 Reza Alizadeh Majd <r.majd <at> pantherx.org>
 ;;; Copyright © 2021 Brice Waegeneire <brice <at> waegenei.re>
-;;; Copyright © 2021 muradm <mail <at> muradm.net>
+;;; Copyright © 2021, 2022 muradm <mail <at> 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

[Message part 3 (text/plain, inline)]

Liliana Marie Prikler <liliana.prikler <at> 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 <at> 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

[signature.asc (application/pgp-signature, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#56690; Package guix-patches. (Mon, 22 Aug 2022 20:21:02 GMT) Full text and rfc822 format available.

Reply sent to Liliana Marie Prikler <liliana.prikler <at> gmail.com>:
You have taken responsibility. (Fri, 26 Aug 2022 17:07:01 GMT) Full text and rfc822 format available.

Notification sent to muradm <mail <at> muradm.net>:
bug acknowledged by developer. (Fri, 26 Aug 2022 17:07:01 GMT) Full text and rfc822 format available.

Message #72 received at 56690-done <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: muradm <mail <at> muradm.net>
Cc: 56690-done <at> debbugs.gnu.org, 56699-done <at> debbugs.gnu.org,
 56971-done <at> debbugs.gnu.org
Subject: Re: greeter user permissions are not enough to talk with seatd
Date: Fri, 26 Aug 2022 19:06:14 +0200
Am Donnerstag, dem 04.08.2022 um 12:45 +0300 schrieb muradm:
> * 56690 - gnu: seatd-service-type: Should use seat group.
> With this change, if seatd-service-type is present in the
> system configuration, "seat" group will be added, and seatd
> will run as root/seat. Group is configurable, but default is 
> "seat".
I made it so that by default the sanitizer is used to turn the string
"seat" into a group and used (ice-9 match), reducing some needless
redundancy.  I also reworded the manual to the best of my ability
following our conversations and adapted the commit message.

> * 56699 - gnu: greetd-service-type: Add greeter-extra-groups 
>   config field.
> With this change, if user wants to use seatd-service-type with
> greeter requiring seatd.sock, he can add "seat" group to
> greeter-extra-groups field.
I fixed some minor issue in the manual and reindented the marionette-
type in the tests, also reworded the commit message.

I didn't get the chance to run the system tests – some timeout causes
the marionette build to fail on my machine – but I verified
independently that at least the seatd socket has the right permissions.
I hope this will be enough for you to get gtkgreet running.

Cheers




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 24 Sep 2022 11:24:12 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 280 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.