[bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

[bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

[Maya via Guix-patches via]

Added a feature to fprintd-service-type to allow unlocking PAM modules (ie. gdm login, gnome polkit etc.) by fingerprint.

---

 gnu/services/authentication.scm | 49 +++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/gnu/services/authentication.scm b/gnu/services/authentication.scm
index f7becdfafb..5737c15f4c 100644
--- a/gnu/services/authentication.scm
+++ b/gnu/services/authentication.scm
@@ -44,9 +44,50 @@ (define-module (gnu services authentication)
             nslcd-configuration?
             nslcd-service-type))

-(define-configuration fprintd-configuration
+(define-configuration/no-serialization fprintd-configuration
   (fprintd      (file-like fprintd)
-                "The fprintd package"))
+                "The fprintd package")
+  (unlock-gdm?
+   (boolean #t)
+   "Generate PAM configuration that unlocks gdm with fprintd.")
+  (unlock-other
+   (list '("polkit-1" "sddm")) ;; polkit-1 is the name of a PAM module for GNOME polkit
+   "List of other PAM modules that can be unlocked with fprintd.
+
+This depends on your desktop configuration. If you for example want GNOME prompts to be unlocked by fingerprint, you add @code{polkit-1} to this list. (This is enabled by default.)
+"))
+
+(define (fprintd-pam-other-services config fprintd-module)
+  (lambda (pam)
+    (if (member (pam-service-name pam)
+                (fprintd-configuration-unlock-other config))
+        (let ((sufficient
+               (pam-entry
+                (control "sufficient")
+                (module fprintd-module))))
+          (pam-service
+           (inherit pam)
+           (auth (cons sufficient (pam-service-auth pam)))))
+        pam)))
+
+(define (fprintd-pam-gdm-services fprintd-module)
+  (list
+   (pam-service
+    (inherit (unix-pam-service "gdm-fingerprint"
+                               #:login-uid? #t))
+    (auth (list
+           (pam-entry
+            (control "required")
+            (module fprintd-module)))))))
+
+(define (fprintd-pam-services config)
+  (let ((fprintd-module
+         #~(string-append #$(fprintd-configuration-fprintd config) "/lib/security/pam_fprintd.so")))
+    (cons
+     (fprintd-pam-other-services config fprintd-module)
+     (if fprintd-configuration-unlock-gdm?
+         (fprintd-pam-gdm-services fprintd-module)
+         '()))))

 (define (fprintd-dbus-service config)
   (list (fprintd-configuration-fprintd config)))
@@ -57,7 +98,9 @@ (define fprintd-service-type
                  (list (service-extension dbus-root-service-type
                                           fprintd-dbus-service)
                        (service-extension polkit-service-type
-                                          fprintd-dbus-service)))
+                                          fprintd-dbus-service)
+                       (service-extension pam-root-service-type
+                                          fprintd-pam-services)))
                 (default-value (fprintd-configuration))
                 (description
                  "Run fprintd, a fingerprint management daemon.")))
--
2.37.0

I sincerely that the gdm pam module is correct. Guix uses non-standard way of defining pam services and it was hard for me to decipher needed contents for gdm-fingerprint. /However, I tested it on my laptop and it works! My only concern is security/

I chose the most usual modules to unlock by fingerprint, if you think that the list is missing something or has something that should not be there, let me know!

With wishes for zero-bug code,
Maya

[bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 361 bytes --]


On 27-07-2022 17:57, Maya via Guix-patches via wrote:
> +  (let ((fprintd-module
> +         #~(string-append #$(fprintd-configuration-fprintd config) "/lib/security/pam_fprintd.so")))

This can be simplified to

    (let ((fprintd-module (file-append (fprintd-configuration-fprintd 
config) "/lib/security/pam_fprintd.so")))

Greetings,
Maxime.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 247 bytes --]


On 27-07-2022 17:57, Maya via Guix-patches via wrote:
> +                               #:login-uid? #t))

What's this line for?  I'm not finding 'login-uid?' anywhere in the 
manual, a comment would be in order.

Greetings,
Maxie.



[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 2044 bytes --]


On 27-07-2022 17:57, Maya via Guix-patches via wrote:
> Added a feature to fprintd-service-type to allow unlocking PAM modules (ie. gdm login, gnome polkit etc.) by fingerprint.
>
> ---

Documentation is missing (in the manual), so as-is, this new feature is 
hard to find.

Also, the manual required giving every top-level procedure a docstring 
IIRC,

>   gnu/services/authentication.scm | 49 +++++++++++++++++++++++++++++++--
>   1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/gnu/services/authentication.scm b/gnu/services/authentication.scm
> index f7becdfafb..5737c15f4c 100644
> --- a/gnu/services/authentication.scm
> +++ b/gnu/services/authentication.scm
> @@ -44,9 +44,50 @@ (define-module (gnu services authentication)
>               nslcd-configuration?
>               nslcd-service-type))
>
> -(define-configuration fprintd-configuration
> +(define-configuration/no-serialization fprintd-configuration
>     (fprintd      (file-like fprintd)
> -                "The fprintd package"))
> +                "The fprintd package")
> +  (unlock-gdm?
> +   (boolean #t)
> +   "Generate PAM configuration that unlocks gdm with fprintd.")
> +  (unlock-other
> +   (list '("polkit-1" "sddm")) ;; polkit-1 is the name of a PAM module for GNOME polkit
> +   "List of other PAM modules that can be unlocked with fprintd.
> +
> +This depends on your desktop configuration. If you for example want GNOME prompts to be unlocked by fingerprint, you add @code{polkit-1} to this list. (This is enabled by default.)
> +"))

This documentation is unclear -- does this field need to be set to the 
_name_ of the module, or to the _file name_ of the _shared library_ (as 
a file-like, not a direct file name, because of staging), or ...?  Also, 
the 'list' check can be more precise, IIRC there was some method for not 
just using list? but doing things like list-of-strings?.

Anyway, I don't really know PAM, but I've written some comments on the 
patch, hopefullt they are useful.

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

[Maya via Guix-patches via]

>This can be simplified to
>
>    (let ((fprintd-module (file-append (fprintd-configuration-fprintd
>config) "/lib/security/pam_fprintd.so")))

Yes, thank you, I am not yet that great with my guix-fu.

> > +                               #:login-uid? #t))

> What's this line for?  I'm not finding 'login-uid?' anywhere in the
> manual, a comment would be in order.

I've got this from the unix-pam-service and from gdm-service-type. The code this refers to in gnu/system/pam.scm:

,@(if login-uid?
     (list (pam-entry       ;to fill in /proc/self/loginuid
                (control "required")
                (module "pam_loginuid.so")))
     '())

gdm-service-type uses it in all 3 of it's pam modules. So I figured it ought to be there. I can investigate further, but it seems like I should not touch it.

> Documentation is missing (in the manual), so as-is, this new feature is
> hard to find.

Oh? I didn't know that. Doesn't define-configuration generate documentation automatically? If it does not, I will hapilly add it, but I have never written any, so it will be a learning process.

> Also, the manual required giving every top-level procedure a docstring
> IIRC,

There is that requirement, yes. But there weren't any around this method so I thought the configuration sufficed, but if it is a requirement, I will do that.

> >   gnu/services/authentication.scm | 49 +++++++++++++++++++++++++++++++--
> >   1 file changed, 46 insertions(+), 3 deletions(-)
> >
> > diff --git a/gnu/services/authentication.scm b/gnu/services/authentication.scm
> > index f7becdfafb..5737c15f4c 100644
> > --- a/gnu/services/authentication.scm
> > +++ b/gnu/services/authentication.scm
> > @@ -44,9 +44,50 @@ (define-module (gnu services authentication)
> >               nslcd-configuration?
> >               nslcd-service-type))
> >
> > -(define-configuration fprintd-configuration
> > +(define-configuration/no-serialization fprintd-configuration
> >     (fprintd      (file-like fprintd)
> > -                "The fprintd package"))
> > +                "The fprintd package")
> > +  (unlock-gdm?
> > +   (boolean #t)
> > +   "Generate PAM configuration that unlocks gdm with fprintd.")
> > +  (unlock-other
> > +   (list '("polkit-1" "sddm")) ;; polkit-1 is the name of a PAM module for GNOME polkit
> > +   "List of other PAM modules that can be unlocked with fprintd.
> > +
> > +This depends on your desktop configuration. If you for example want GNOME prompts to be unlocked by fingerprint, you add @code{polkit-1} to this list. (This is enabled by default.)
> +"))

> This documentation is unclear -- does this field need to be set to the
> _name_ of the module, or to the _file name_ of the _shared library_ (as
> a file-like, not a direct file name, because of staging), or ...?  Also,
> the 'list' check can be more precise, IIRC there was some method for not
> just using list? but doing things like list-of-strings?.

The name of the pam module, not a shared library. So the file in /etc/pam.d. It is a direct name, since it is not inside the store, pam modules have static path.

As for the configuration options, it's my first time using them and I didn't really understand the define-syntax definition, so I really just skimmed through the guix repository for some uses.

> Anyway, I don't really know PAM, but I've written some comments on the
> patch, hopefully they are useful.

They are a lot! Thank you very much. I hope those comments will be less needed in the future, as I become better as a contributor.

With all the best for tomorrow and all the days to come,
Maya.

[bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

[Maxime Devos]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1961 bytes --]


On 27-07-2022 22:26, Maya wrote:
>> Documentation is missing (in the manual), so as-is, this new feature is
>> hard to find.
> Oh? I didn't know that. Doesn't define-configuration generate documentation automatically? If it does not, I will hapilly add it, but I have never written any, so it will be a learning process.
>
There is some procedure that takes a record type and generates some 
documentation, but it is not automatically copied into the manual, you 
will have to do that yourself (and maybe tweak the result a little: 
what's a good docstring in code doesn't always fit very well in a manual).

>> Also, the manual required giving every top-level procedure a docstring
>> IIRC,
> There is that requirement, yes. But there weren't any around this method so I thought the configuration sufficed, but if it is a requirement, I will do that.
>
I don't know if the requirement is overly strictly formulated or if the 
surrounding code is wrong.

>> This documentation is unclear -- does this field need to be set to the
>> _name_  of the module, or to the_file name_  of the_shared library_  (as
>> a file-like, not a direct file name, because of staging), or ...?  Also,
>> the 'list' check can be more precise, IIRC there was some method for not
>> just using list? but doing things like list-of-strings?.
> The name of the pam module, not a shared library. So the file in /etc/pam.d. It is a direct name, since it is not inside the store, pam modules have static path.
To be clear, it is clear if you look at the default value, but I think 
it's best to be explicit in the documentation.

> As for the configuration options, it's my first time using them and I didn't really understand the define-syntax definition, so I really just skimmed through the guix repository for some uses.
IIRC, there are some procedures you can use to define list-of-x? 
procedures but I don't recall the details.

Greetings,
Maxime.


[-- Attachment #1.1.1.2: Type: text/html, Size: 3484 bytes --]

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 929 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[bug#56797] [PATCH] gnu: services: fprintd: Add PAM configuration.

[Ludovic Courtès]

Hi Maya,

Could you send an updated patch taking Maxime’s suggestions into
account?  Let us know here or on IRC if you need guidance.

Thanks for your work!

Ludo’.