[bug#65538] [PATCH v2] services: greetd: Add pam-gnupg support.

["Ludovic Courtès" <ludo@gnu.org>]

Hi,

Carlos Durán Domínguez <wurt@wurtshell.com> skribis:

> My main concern is that the new 'pam-gnupg-module?' procedure could be wrong,
> but I did not find any other solution.

OK.

> I also think that using 'insert-before' in gnu/services/pam-mount.scm could be
> a problem. If other PAM modules needs to be on certain positions, it could be
> an “if mess” or maybe not, I do not know much about PAM modules.

Yeah, I’m also wondering what will happen if several services have
ordering requirements.  This seems to break composability, but maybe
it’s a problem that PAM has.

> +@file{~/.pam-gnupg} (see
> +@url{https://github.com/cruegge/pam-gnupg#setup-guide, pam-gnupg setup
> +guide}), and can be queried with @code{gpg -K --with-keygrip}
> +(@pxref{Commands to select the type of operation,,,gnupg}).  Presetting
> +passphrases must be enabled by adding @code{allow-preset-passphrase} in
> +@file{~/.gnupg/gpg-agent.conf} (@pxref{Put a passphrase into the
> +cache,,,gnupg}).

Note that @pxref should first name the “node” (section) of the manual
you’re referring to, and there should be one last argument giving the
title of the manual:

  https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Four-and-Five-Arguments.html

The ones above should be adjusted to refer to existing nodes of the
GnuPG manual.

>         (if (member (pam-service-name pam)
>                     '("login" "greetd" "su" "slim" "gdm-password" "sddm"))
>             (pam-service
> +            ;; pam-mount module must be before pam-gnupg, because the later
> +            ;; needs to be at the end (See pam-gnupg README.md)
>              (inherit pam)

I went to look at the ‘README.md’ file, and all I could find is:

  At least, `pam_gnupg.so` should come after `pam_unix.so`,
  `pam_systemd_home.so`, `pam_systemd.so` and `pam_env.so` in case you
  use those modules.

Is it what you’re referring to?

The ‘README.md’ also gave me a sense that pam-gnupg is not fully baked:

  The code was written mainly by looking at and occasionally copying
  from Gnome Keyring's PAM module and pam_mount and is based on a
  somewhat mediocre understanding of the details of both PAM and C. You
  should be aware that there may be potentially dangerous bugs lurking.

It also makes this recommendation, which I find dubious security-wise:

  - Add

        allow-preset-passphrase

    to `~/.gnupg/gpg-agent.conf`. Optionally, customize the cache timeout via
    `max-cache-ttl`, e.g. set

        max-cache-ttl 86400

    to have it expire after a day.

I guess that’s expected given the purpose of the tool, but still.

Overall that made me wonder if this is something we should promote.
WDYT?

> +(define (pam-gnupg-module? name)
> +  "Return `#t' if NAME is the path to the pam-gnupg module, `#f' otherwise."
> +  (let ((module (pam-entry-module name)))
> +    (and (file-append? module)
> +         (equal? (first (file-append-suffix module))
> +                 "/lib/security/pam_gnupg.so"))))

This is not ideal, because someone might give a module that’s not a
<file-append>, but I can’t think of a better way.

> -  #:use-module (rnrs io ports)                    ;need 'port-position' etc.
> +  #:use-module (rnrs io ports)          ;need 'port-position' etc.
>    #:use-module ((rnrs bytevectors) #:select (bytevector-u8-set!))
>    #:use-module (guix memoization)
>    #:use-module ((guix build utils)
>                  #:select (dump-port mkdir-p delete-file-recursively
> -                          call-with-temporary-output-file %xz-parallel-args))
> +                                    call-with-temporary-output-file %xz-parallel-args))
>    #:use-module ((guix build syscalls) #:select (mkdtemp! fdatasync))
>    #:use-module ((guix combinators) #:select (fold2))
> -  #:use-module (guix diagnostics)           ;<location>, &error-location, etc.
> +  #:use-module (guix diagnostics)       ;<location>, &error-location, etc.
>    #:use-module (ice-9 format)
>    #:use-module ((ice-9 iconv) #:prefix iconv:)
>    #:use-module (ice-9 match)
> @@ -57,7 +58,7 @@ (define-module (guix utils)
>    #:use-module (ice-9 vlist)
>    #:autoload   (zlib) (make-zlib-input-port make-zlib-output-port)
>    #:use-module (system foreign)
> -  #:re-export (<location>                         ;for backwards compatibility
> +  #:re-export (<location>               ;for backwards compatibility

Unnecessary changes.  :-)

Thanks,
Ludo’.