[PATCH] gnu: services: fprintd: Add PAM configuration.

  • Open
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Maxime Devos
  • Maya
Owner
unassigned
Submitted by
Maya
Severity
normal
M
M
Maya wrote on 27 Jul 2022 17:57
(name . guix-patches@gnu.org)(address . guix-patches@gnu.org)
4AtymQ5ic7YPCQjgRG3Dj73aZuO_Rx7GX8YSKBPeoVoOG_Z8LjXXbqvvfaq-ap0fgLADcsE8zibqDwkO7kazYXa0eMA3EeEaiU_6wGQ0yI8=@protonmail.com
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(-)

Toggle diff (68 lines)
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
M
M
Maxime Devos wrote on 27 Jul 2022 18:04
255aca6e-d1f9-f61f-d57e-857ae635f977@telenet.be
On 27-07-2022 17:57, Maya via Guix-patches via wrote:
Toggle quote (2 lines)
> + (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: OpenPGP_0x49E3EE22191725EE.asc
Attachment: OpenPGP_signature
M
M
Maxime Devos wrote on 27 Jul 2022 18:06
7d1308a7-80f1-33ea-f59c-a985a86593d9@telenet.be
On 27-07-2022 17:57, Maya via Guix-patches via wrote:
Toggle quote (1 lines)
> + #: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: OpenPGP_0x49E3EE22191725EE.asc
Attachment: OpenPGP_signature
M
M
Maxime Devos wrote on 27 Jul 2022 18:12
067bff4c-3ada-0597-2632-9482066df2f2@telenet.be
On 27-07-2022 17:57, Maya via Guix-patches via wrote:
Toggle quote (3 lines)
> 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,
Toggle quote (25 lines)
> 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: OpenPGP_0x49E3EE22191725EE.asc
Attachment: OpenPGP_signature
M
M
Maya wrote on 27 Jul 2022 22:26
(name . Maxime Devos)(address . maximedevos@telenet.be)(name . 56797@debbugs.gnu.org)(address . 56797@debbugs.gnu.org)
LcTuwD5B16pAYt1fDmGz1h8p7LuuJXmaMo9xaUF24goWjOD_OG4KHOhDM59KlDwoDRXpCnsHaUW8j49fWudFgjA7_XN-Dm1w2OO5gsblHN0=@protonmail.com
Toggle quote (5 lines)
>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.

Toggle quote (5 lines)
> > + #: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.

Toggle quote (3 lines)
> 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.

Toggle quote (3 lines)
> 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.

Toggle quote (32 lines)
> > 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.

Toggle quote (3 lines)
> 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.
M
M
Maxime Devos wrote on 27 Jul 2022 23:56
(name . Maya)(address . maya.omase@protonmail.com)(name . 56797@debbugs.gnu.org)(address . 56797@debbugs.gnu.org)
946b358e-da78-9307-ff12-8a4a060867a8@telenet.be
On 27-07-2022 22:26, Maya wrote:
Toggle quote (4 lines)
>> 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).
Toggle quote (4 lines)
>> 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.
Toggle quote (6 lines)
>> 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.
Toggle quote (1 lines)
> 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: file
Attachment: OpenPGP_0x49E3EE22191725EE.asc
Attachment: OpenPGP_signature
L
L
Ludovic Courtès wrote on 9 Aug 2022 17:00
Re: bug#56797: [PATCH] gnu: services: fprintd: Add PAM configuration.
(name . Maya)(address . maya.omase@protonmail.com)
87wnbh5uni.fsf_-_@gnu.org
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’.
L
L
Ludovic Courtès wrote on 31 Aug 2022 22:42
control message for bug #56797
(address . control@debbugs.gnu.org)
87o7w09mbj.fsf@gnu.org
tags 56797 + moreinfo
quit
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 56797@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 56797
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch