From: "Ludovic Courtès" <ludo@gnu.org>
To: "Carlos Durán Domínguez" <wurt@wurtshell.com>
Cc: Tobias Geerinckx-Rice <me@tobias.gr>,
Simon Tournier <zimon.toutoune@gmail.com>,
paren@disroot.org, Christopher Baines <mail@cbaines.net>,
Ricardo Wurmus <rekado@elephly.net>,
Raghav Gururajan <rg@raghavgururajan.name>,
jgart <jgart@dismail.de>, Mathieu Othacehe <othacehe@gnu.org>,
65538@debbugs.gnu.org
Subject: [bug#65538] [PATCH v2] services: greetd: Add pam-gnupg support.
Date: Thu, 05 Oct 2023 14:57:09 +0200 [thread overview]
Message-ID: <87il7l6xcq.fsf@gnu.org> (raw)
In-Reply-To: <20230825144806.6315-1-wurt@wurtshell.com> ("Carlos Durán Domínguez"'s message of "Fri, 25 Aug 2023 16:48:03 +0200")
Hello,
Carlos Durán Domínguez <wurt@wurtshell.com> skribis:
> I retry to implement the pam-gnupg module for the greetd system service. It is A PAM module that hands over your login password to gpg-agent. I added de documentation and the insert-before procedure (maybe it needs a better name), to ensure that the pam-gnupg module will be loaded at the end.
>
> * doc/guix.texi: documentation about #:gnupg? option on (greetd-configuration).
> * gnu/services.scm (insert-before): new procedure.
> * gnu/services/base.scm (greetd-configuration): new option #:gnupg?.
> * gnu/services/pam-mount.scm: ensure that pam mount module goes before pam gnupg module.
> * gnu/system/pam.scm (pam-gnupg-module?): new procedure and ensure that pam gnupg module is at the end of (unix-pam-service).
Nice work!
A minor point: the commit log should normally lists all
changed/added/removed entities. You can use ‘git log’ to see examples,
but the committer will tweak it for you if needed (no big deal).
[...]
> +@item @code{gnupg?} (default: @code{#f})
> +If enabled, @code{pam-gnupg} will attempt to automatically unlock the
> +user's GPG keys with the login password via @code{gpg-agent}. The
> +keygrips of all keys to be unlocked should be written to
> +@file{~/.pam-gnupg}, and can be queried with @code{gpg -K
> +--with-keygrip}. Presetting passphrases must be enabled by adding
> +@code{allow-preset-passphrase} in @file{~/.gnupg/gpg-agent.conf}.
Perhaps you can add a cross-reference to the relevant part of the GnuPG
manual? (With @pxref or similar.)
> +(define (insert-before pred lst1 lst2)
> + "Return a list appending LST2 just before the first element on LST1 that
> + satisfy the predicate PRED."
> + (cond
> + ((null? lst1) lst2)
> + ((pred (car lst1)) (append lst2 lst1))
> + (else (cons (car lst1) (insert-before pred (cdr lst1) lst2)))))
I’d rather have it in (guix utils). Also, please use ‘match’ and avoid
car/cdr as per
<https://guix.gnu.org/manual/devel/en/html_node/Data-Types-and-Pattern-Matching.html>.
> (pam-service
> (inherit pam)
> - (auth (append (pam-service-auth pam)
> - (list optional-pam-mount)))
> - (session (append (pam-service-session pam)
> - (list optional-pam-mount))))
> + (auth (insert-before pam-gnupg-module?
> + (pam-service-auth pam)
> + (list optional-pam-mount)))
> + (session (insert-before pam-gnupg-module?
> + (pam-service-session pam)
> + (list optional-pam-mount))))
Could you add a comment explaining why this ordering is important?
> +(define (pam-gnupg-module? name)
> + "Return `#t' if NAME is the path to the pam-gnupg module, `#f' otherwise."
> + (equal? (pam-entry-module name)
> + (file-append pam-gnupg "/lib/security/pam_gnupg.so")))
<package> records in general cannot be compared with ‘equal?’, so the
above procedure won’t work in the general case. (It wouldn’t work with
custom variants of the ‘pam-gnupg’ package, too.)
Can you think of another way we could check whether a <pam-entry>
corresponds to ‘pam-gnupg’?
Thanks,
Ludo’.
next prev parent reply other threads:[~2023-10-05 13:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-25 14:48 [bug#65538] [PATCH v2] services: greetd: Add pam-gnupg support guix-patches--- via
2023-10-05 12:57 ` Ludovic Courtès [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-10-06 0:53 [bug#65538] [PATCH v3] " guix-patches--- via
2023-10-11 20:54 ` [bug#65538] [PATCH v2] " Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87il7l6xcq.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=65538@debbugs.gnu.org \
--cc=jgart@dismail.de \
--cc=mail@cbaines.net \
--cc=me@tobias.gr \
--cc=othacehe@gnu.org \
--cc=paren@disroot.org \
--cc=rekado@elephly.net \
--cc=rg@raghavgururajan.name \
--cc=wurt@wurtshell.com \
--cc=zimon.toutoune@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).