unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
@ 2022-07-22  4:27 muradm
  2022-07-24 16:28 ` ( via Guix-patches via
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: muradm @ 2022-07-22  4:27 UTC (permalink / raw)
  To: 56690

* 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@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
-- 
2.36.1





^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-07-22  4:27 [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group muradm
@ 2022-07-24 16:28 ` ( via Guix-patches via
  2022-08-05  8:10 ` Liliana Marie Prikler
  2022-08-06 20:46 ` Ludovic Courtès
  2 siblings, 0 replies; 19+ messages in thread
From: ( via Guix-patches via @ 2022-07-24 16:28 UTC (permalink / raw)
  To: muradm, 56690

Because patches with replies are more likely to be visible: LGTM :)

    -- (




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-07-22  4:27 [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group muradm
  2022-07-24 16:28 ` ( via Guix-patches via
@ 2022-08-05  8:10 ` Liliana Marie Prikler
  2022-08-07 20:45   ` muradm
  2022-08-06 20:46 ` Ludovic Courtès
  2 siblings, 1 reply; 19+ messages in thread
From: Liliana Marie Prikler @ 2022-08-05  8:10 UTC (permalink / raw)
  To: muradm, 56690

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




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-07-22  4:27 [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group muradm
  2022-07-24 16:28 ` ( via Guix-patches via
  2022-08-05  8:10 ` Liliana Marie Prikler
@ 2022-08-06 20:46 ` Ludovic Courtès
  2022-08-06 20:50   ` ( via Guix-patches via
  2022-08-07 17:28   ` muradm
  2 siblings, 2 replies; 19+ messages in thread
From: Ludovic Courtès @ 2022-08-06 20:46 UTC (permalink / raw)
  To: muradm; +Cc: 56690

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’.




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-06 20:46 ` Ludovic Courtès
@ 2022-08-06 20:50   ` ( via Guix-patches via
  2022-08-08  8:58     ` Ludovic Courtès
  2022-08-07 17:28   ` muradm
  1 sibling, 1 reply; 19+ messages in thread
From: ( via Guix-patches via @ 2022-08-06 20:50 UTC (permalink / raw)
  To: Ludovic Courtès, muradm; +Cc: 56690

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.

    -- (




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-06 20:46 ` Ludovic Courtès
  2022-08-06 20:50   ` ( via Guix-patches via
@ 2022-08-07 17:28   ` muradm
  2022-08-07 20:05     ` muradm
  1 sibling, 1 reply; 19+ messages in thread
From: muradm @ 2022-08-07 17:28 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 56690

[-- Attachment #1: Type: text/plain, Size: 2743 bytes --]


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’.


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-07 17:28   ` muradm
@ 2022-08-07 20:05     ` muradm
  2022-08-08  6:08       ` Liliana Marie Prikler
  0 siblings, 1 reply; 19+ messages in thread
From: muradm @ 2022-08-07 20:05 UTC (permalink / raw)
  To: Ludovic Courtès, 56690


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


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


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

From edf954714a71ea3c1b8a872df40ed3735dff10f8 Mon Sep 17 00:00:00 2001
From: muradm <mail@muradm.net>
Date: Fri, 22 Jul 2022 07:09:54 +0300
Subject: [PATCH v2] 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] 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@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.
 ;;;
@@ -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


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



muradm <mail@muradm.net> writes:

> [[PGP Signed Part:Undecided]]
>
> 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’.
>
> [[End of PGP Signed Part]]


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-05  8:10 ` Liliana Marie Prikler
@ 2022-08-07 20:45   ` muradm
  0 siblings, 0 replies; 19+ messages in thread
From: muradm @ 2022-08-07 20:45 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: 56690

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]


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


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-07 20:05     ` muradm
@ 2022-08-08  6:08       ` Liliana Marie Prikler
  2022-08-08 18:50         ` muradm
  0 siblings, 1 reply; 19+ messages in thread
From: Liliana Marie Prikler @ 2022-08-08  6:08 UTC (permalink / raw)
  To: muradm, Ludovic Courtès, 56690

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




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-06 20:50   ` ( via Guix-patches via
@ 2022-08-08  8:58     ` Ludovic Courtès
  2022-08-08  9:12       ` ( via Guix-patches via
  2022-08-08 18:55       ` muradm
  0 siblings, 2 replies; 19+ messages in thread
From: Ludovic Courtès @ 2022-08-08  8:58 UTC (permalink / raw)
  To: (; +Cc: muradm, 56690

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’.




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-08  8:58     ` Ludovic Courtès
@ 2022-08-08  9:12       ` ( via Guix-patches via
  2022-08-08 19:44         ` muradm
  2022-08-08 18:55       ` muradm
  1 sibling, 1 reply; 19+ messages in thread
From: ( via Guix-patches via @ 2022-08-08  9:12 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: muradm, 56690

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.

    -- (




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-08  6:08       ` Liliana Marie Prikler
@ 2022-08-08 18:50         ` muradm
  2022-08-09  6:57           ` Liliana Marie Prikler
  0 siblings, 1 reply; 19+ messages in thread
From: muradm @ 2022-08-08 18:50 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: Ludovic Courtès, 56690


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

From e733977a5cbd2bb34cf129908b16a4a0af830f02 Mon Sep 17 00:00:00 2001
From: muradm <mail@muradm.net>
Date: Fri, 22 Jul 2022 07:09:54 +0300
Subject: [PATCH v3] 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".
* 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@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.
 ;;;
@@ -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


[-- Attachment #1.2: Type: text/plain, Size: 1844 bytes --]


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


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-08  8:58     ` Ludovic Courtès
  2022-08-08  9:12       ` ( via Guix-patches via
@ 2022-08-08 18:55       ` muradm
  1 sibling, 0 replies; 19+ messages in thread
From: muradm @ 2022-08-08 18:55 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: (, 56690

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]


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’.


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-08  9:12       ` ( via Guix-patches via
@ 2022-08-08 19:44         ` muradm
  0 siblings, 0 replies; 19+ messages in thread
From: muradm @ 2022-08-08 19:44 UTC (permalink / raw)
  To: (; +Cc: ludo, 56690

[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]


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

>
>     -- (


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-08 18:50         ` muradm
@ 2022-08-09  6:57           ` Liliana Marie Prikler
  2022-08-09 19:47             ` muradm
  0 siblings, 1 reply; 19+ messages in thread
From: Liliana Marie Prikler @ 2022-08-09  6:57 UTC (permalink / raw)
  To: muradm; +Cc: Ludovic Courtès, 56690

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@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





^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-09  6:57           ` Liliana Marie Prikler
@ 2022-08-09 19:47             ` muradm
  2022-08-10  8:07               ` Liliana Marie Prikler
  0 siblings, 1 reply; 19+ messages in thread
From: muradm @ 2022-08-09 19:47 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: Ludovic Courtès, 56690

[-- Attachment #1: Type: text/plain, Size: 3400 bytes --]


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


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-09 19:47             ` muradm
@ 2022-08-10  8:07               ` Liliana Marie Prikler
  2022-08-13 17:39                 ` muradm
  2022-08-22 20:17                 ` muradm
  0 siblings, 2 replies; 19+ messages in thread
From: Liliana Marie Prikler @ 2022-08-10  8:07 UTC (permalink / raw)
  To: muradm; +Cc: Ludovic Courtès, 56690

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




^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-10  8:07               ` Liliana Marie Prikler
@ 2022-08-13 17:39                 ` muradm
  2022-08-22 20:17                 ` muradm
  1 sibling, 0 replies; 19+ messages in thread
From: muradm @ 2022-08-13 17:39 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: Ludovic Courtès, 56690

[-- Attachment #1: Type: text/plain, Size: 4986 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.
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@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


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group.
  2022-08-10  8:07               ` Liliana Marie Prikler
  2022-08-13 17:39                 ` muradm
@ 2022-08-22 20:17                 ` muradm
  1 sibling, 0 replies; 19+ messages in thread
From: muradm @ 2022-08-22 20:17 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: ludo, 56690


[-- 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 --]

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-08-22 20:21 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-22  4:27 [bug#56690] [PATCH] gnu: seatd-service-type: Should use seat group muradm
2022-07-24 16:28 ` ( via Guix-patches via
2022-08-05  8:10 ` Liliana Marie Prikler
2022-08-07 20:45   ` muradm
2022-08-06 20:46 ` Ludovic Courtès
2022-08-06 20:50   ` ( via Guix-patches via
2022-08-08  8:58     ` Ludovic Courtès
2022-08-08  9:12       ` ( via Guix-patches via
2022-08-08 19:44         ` muradm
2022-08-08 18:55       ` muradm
2022-08-07 17:28   ` muradm
2022-08-07 20:05     ` muradm
2022-08-08  6:08       ` Liliana Marie Prikler
2022-08-08 18:50         ` muradm
2022-08-09  6:57           ` Liliana Marie Prikler
2022-08-09 19:47             ` muradm
2022-08-10  8:07               ` Liliana Marie Prikler
2022-08-13 17:39                 ` muradm
2022-08-22 20:17                 ` muradm

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).