Message ID | 20220722042745.26745-1-mail@muradm.net |
---|---|
State | Accepted |
Headers | show |
Series | [bug#56690] gnu: seatd-service-type: Should use seat group. | expand |
Context | Check | Description |
---|---|---|
cbaines/comparison | success | View comparision |
cbaines/git-branch | success | View Git branch |
cbaines/applying patch | success | View Laminar job |
cbaines/issue | success | View issue |
Because patches with replies are more likely to be visible: LGTM :) -- (
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
Hi, muradm <mail@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’.
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. -- (
Hi, Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > muradm <mail@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’.
Fixed in v2. Liliana Marie Prikler <liliana.prikler@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
Hi, "(" <paren@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’.
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. -- (
Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > "(" <paren@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’.
"(" <paren@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. > > -- (
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@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. ;;; @@ -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