bug#63652: [PATCH] services: screen-locker-service-type: Configurable PAM and setuid.

[Josselin Poiret via Guix-patches via <guix-patches@gnu.org>]

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

Hi muradm,

muradm <mail@muradm.net> writes:

> screen-locker-service-type by default does both define PAM entry
> and make program setuid binary. Normally both methods are
> mutually exclusive, if binary has setuid set it does not really
> needs PAM, otherway around also similar, if PAM is enabled
> binary should not relay on setuid.
>
> Recent swaylock package now compiled with PAM support. When PAM
> support is compiled in, swaylock rejects executing if binary is
> also setuid program.
>
> This change turns screen-locker-configuration from strict
> PAM AND setuid to more flexible PAM AND/OR setuid. Allowing
> swaylock to be configured properly while supporting other
> screen locker preferences.
>
> * gnu/services/xorg.scm (screen-locker-configuration): Switch from
> define-record-type to define-configuration.
> [using-pam?]: New field to control PAM entry existence.
> [using-setuid?]: New field to control setuid binary existence.
> (screen-locker-pam-services): Should not make unix-pam-service if
> using-pam? is set to #f.
> (screen-locker-setuid-programs): Should not make program setuid
> program if using-setuid? is set to #f.
> (screen-locker-generate-doc): Internal function to generate
> configuration documentation.
> (screen-locker-service): Adapt to new screen-locker-configuration.
> * gnu/services/desktop.scm (desktop-services-for-system): Adapt to
> new screen-locker-configuration.
> * doc/guix.texi: Reflect new changes to screen-locker-configuration.

Thanks!  Tested and pushed as f4f5ee6ad6e2432f52e37c549211df8f1cdbb571
with the following changes:

diff --git a/doc/guix-cookbook.texi b/doc/guix-cookbook.texi
index b1ffa72c0e..b9f5f6b6a9 100644
--- a/doc/guix-cookbook.texi
+++ b/doc/guix-cookbook.texi
@@ -2147,7 +2147,10 @@ Xorg
 can be achieved by adding the following service to your @file{config.scm}:
 
 @lisp
-(screen-locker-service slock)
+(service screen-locker-services-type
+         (screen-locker-configuration
+          (name "slock")
+          (program (file-append slock "/bin/slock"))))
 @end lisp
 
 If you manually lock your screen, e.g. by directly calling slock when you want to lock
diff --git a/doc/guix.texi b/doc/guix.texi
index 704bbd39d2..db37676e12 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -97,7 +97,7 @@
 Copyright @copyright{} 2021 pukkamustard@*
 Copyright @copyright{} 2021 Alice Brenon@*
 Copyright @copyright{} 2021, 2022 Josselin Poiret@*
-Copyright @copyright{} 2021 muradm@*
+Copyright @copyright{} 2021, 2023 muradm@*
 Copyright @copyright{} 2021, 2022 Andrew Tropin@*
 Copyright @copyright{} 2021 Sarah Morgensen@*
 Copyright @copyright{} 2022 Remco van 't Veer@*
@@ -22533,28 +22533,32 @@ X Window
 saver to the set of setuid programs and/or add a PAM entry for it.  The
 value for this service is a @code{<screen-locker-configuration>} object.
 
-While default behavior is to setup both setuid program and PAM entry,
-they are effectively mutually exclusive.  Screen locker programs may
-prevent executing when PAM is configured, and @code{setuid} is set on
-executable.  Then @code{using-setuid?} can be set to @code{#f}.
+While the default behavior is to setup both a setuid program and PAM
+entry, these two methods are redundant.  Screen locker programs may not
+execute when PAM is configured and @code{setuid} is set on their
+executable.  In this case, @code{using-setuid?} can be set to @code{#f}.
 
 For example, to make XlockMore usable:
 
 @lisp
 (service screen-locker-service-type
          (screen-locker-configuration
-           "xlock" (file-append xlockmore "/bin/xlock") #f))
+           (name "xlock")
+           (program (file-append xlockmore "/bin/xlock"))))
 @end lisp
 
 makes the good ol' XlockMore usable.
 
 For example, swaylock fails to execute when compiled with PAM support
-and setuid enabled, then one can disable setuid:
+and setuid enabled.  One can thus disable setuid:
 
 @lisp
 (service screen-locker-service-type
          (screen-locker-configuration
-           "swaylock" (file-append xlockmore "/bin/xlock") #f #t #f))
+           (name "swaylock")
+           (program (file-append xlockmore "/bin/xlock"))
+           (using-pam? #t)
+           (using-setuid? #f)))
 @end lisp
 
 @end defvar
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 639e99ff79..a63748b652 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -1840,13 +1840,11 @@ (define* (desktop-services-for-system #:optional
          (service screen-locker-service-type
                   (screen-locker-configuration
                    (name "slock")
-                   (program (file-append slock "/bin/slock"))
-                   (allow-empty-password? #f)))
+                   (program (file-append slock "/bin/slock"))))
          (service screen-locker-service-type
                   (screen-locker-configuration
                    (name "xlock")
-                   (program (file-append xlock "/bin/xlock"))
-                   (allow-empty-password? #f)))
+                   (program (file-append xlockmore "/bin/xlock"))))
 
          ;; Add udev rules for MTP devices so that non-root users can access
          ;; them.
diff --git a/gnu/services/xorg.scm b/gnu/services/xorg.scm
index b6c1636660..f8cf9f25b6 100644
--- a/gnu/services/xorg.scm
+++ b/gnu/services/xorg.scm
@@ -723,14 +723,6 @@ (define-configuration/no-serialization screen-locker-configuration
    (boolean #t)
    "Whether to setup program as setuid binary."))
 
-(define-deprecated/public-alias
-  screen-locker
-  screen-locker-configuration)
-
-(define-deprecated/public-alias
-  screen-locker?
-  screen-locker-configuration?)
-
 (define (screen-locker-pam-services config)
   (match-record config <screen-locker-configuration>
     (name allow-empty-password? using-pam?)

-- 
Josselin Poiret

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