bug#51487: The openssh service does not allow multiple authorized key files per user

bug#51487: The openssh service does not allow multiple authorized key files per user

[Vivien Kraus via Bug reports for GNU Guix]

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


Dear guix,

The openssh service is configured with a list of authorized keys, as a
list of items, where each item is a list of 2 values, the user name (as
a string) and the public key file (a file-like object). The service can
be extended with new keys.

To have multiple keys per user, we can put them on the same file-like
object, each on its own line. However, if we put two different records,
only the last one is remembered.

This is a problem if we want to extend the service for users that
already have a key. As I am trying to create a service that would
convert GPG keys to SSH keys, I am in this exact situation: the users
may have already defined SSH keys, and I want to add some more without
losing the others.

Best regards,

Vivien

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

bug#51487: Acknowledgement (The openssh service does not allow multiple authorized key files per user)

[Vivien Kraus via Bug reports for GNU Guix]

[-- Attachment #1.1: Type: text/plain, Size: 552 bytes --]


I have a patch, what do you think?

I tested it by building an operating system of the form:

(operating-system
  ...
  (services
   (append
       (list
        (service openssh-service-type
                 (openssh-configuration
                  (authorized-keys
                   `(("root" ,(plain-file "first-key" "ssh-rsa ..."))
                     ("root" ,(plain-file "second-key" "ssh-rsa ..."))))))))))

I caught the derivation to build the authorized-keys directory, and root
had 2 keys. Without the patch, root had only 1 key.

Vivien


[-- Attachment #1.2: ~/Projets/guix/0001-gnu-openssh-service-Collect-all-keys-for-all-users.patch --]
[-- Type: message/external-body, Size: 109 bytes --]

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

bug#51487: Acknowledgement (The openssh service does not allow multiple authorized key files per user)

[Vivien Kraus via Bug reports for GNU Guix]

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


Vivien Kraus <vivien@planete-kraus.eu> writes:
> I have a patch, what do you think?
>
> I tested it by building an operating system of the form:
>
> (operating-system
>   ...
>   (services
>    (append
>        (list
>         (service openssh-service-type
>                  (openssh-configuration
>                   (authorized-keys
>                    `(("root" ,(plain-file "first-key" "ssh-rsa ..."))
>                      ("root" ,(plain-file "second-key" "ssh-rsa ..."))))))))))
>
> I caught the derivation to build the authorized-keys directory, and root
> had 2 keys. Without the patch, root had only 1 key.

The patch wasn’t formatted correctly, sorry.


[-- Attachment #2: ~/Projets/guix/0001-gnu-openssh-service-Collect-all-keys-for-all-users.patch --]
[-- Type: message/external-body, Size: 109 bytes --]

[-- Attachment #3: Type: text/plain, Size: 12 bytes --]


>
> Vivien

bug#51487: Acknowledgement (The openssh service does not allow multiple authorized key files per user)

[Vivien Kraus via Bug reports for GNU Guix]

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

The patch does not seem to get formatted correctly, sorry. Hopefully,
this should work.

Vivien

[-- Attachment #2: 0001-gnu-openssh-service-Collect-all-keys-for-all-users.patch --]
[-- Type: text/x-patch, Size: 2028 bytes --]

From 8dcf1a92cb6ebbc537029f88d5c7197cbf4959aa Mon Sep 17 00:00:00 2001
From: Vivien Kraus <vivien@planete-kraus.eu>
Date: Fri, 29 Oct 2021 18:25:24 +0200
Subject: [PATCH] gnu: openssh-service: Collect all keys for all users.

* gnu/services/ssh.scm: (authorized-key-directory)[build]: ensure that no key is forgotten.
---
 gnu/services/ssh.scm | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/gnu/services/ssh.scm b/gnu/services/ssh.scm
index a018052eeb..118dfdbef8 100644
--- a/gnu/services/ssh.scm
+++ b/gnu/services/ssh.scm
@@ -415,17 +415,23 @@ (define build
                        (guix build utils))
 
           (mkdir #$output)
-          (for-each (match-lambda
-                      ((user keys ...)
-                       (let ((file (string-append #$output "/" user)))
-                         (call-with-output-file file
-                           (lambda (port)
-                             (for-each (lambda (key)
-                                         (call-with-input-file key
-                                           (cut dump-port <> port)))
-                                       keys))))))
-                    '#$keys))))
-
+          (let ((by-user (make-hash-table)))
+            (for-each
+             (match-lambda
+               ((user keys ...)
+                (hash-set! by-user user (append (hash-ref by-user user '()) keys))))
+             '#$keys)
+            (hash-for-each
+             (match-lambda*
+               ((user keys)
+                (let ((file (string-append #$output "/" user)))
+                  (call-with-output-file file
+                    (lambda (port)
+                      (for-each (lambda (key)
+                                  (call-with-input-file key
+                                    (cut dump-port <> port)))
+                                keys))))))
+             by-user)))))
   (computed-file "openssh-authorized-keys" build))
 
 (define (openssh-config-file config)
-- 
2.33.1

bug#51487: Acknowledgement (The openssh service does not allow multiple authorized key files per user)

[Vivien Kraus via Bug reports for GNU Guix]

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

After some discussion on #guix, this seems to be the easier way to fix
the problem:

Vivien

[-- Attachment #2: 0001-gnu-openssh-service-Collect-all-keys-for-all-users.patch --]
[-- Type: text/x-patch, Size: 1293 bytes --]

From d029179554fc2f9656f708e5315bca52928e9254 Mon Sep 17 00:00:00 2001
From: Vivien Kraus <vivien@planete-kraus.eu>
Date: Fri, 29 Oct 2021 18:25:24 +0200
Subject: [PATCH] gnu: openssh-service: Collect all keys for all users.

* gnu/services/ssh.scm: (authorized-key-directory)[build]: ensure that no key is forgotten.
---
 gnu/services/ssh.scm | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/gnu/services/ssh.scm b/gnu/services/ssh.scm
index a018052eeb..1309e062ce 100644
--- a/gnu/services/ssh.scm
+++ b/gnu/services/ssh.scm
@@ -532,10 +532,16 @@ (define (openssh-pam-services config)
 
 (define (extend-openssh-authorized-keys config keys)
   "Extend CONFIG with the extra authorized keys listed in KEYS."
-  (openssh-configuration
-   (inherit config)
-   (authorized-keys
-    (append (openssh-authorized-keys config) keys))))
+  (let ((all-keys (make-hash-table)))
+    (for-each
+     (match-lambda
+       ((user keys ...)
+        (hash-set! all-keys user (append (hash-ref all-keys user '()) keys))))
+     (append (openssh-authorized-keys config) keys))
+    (openssh-configuration
+     (inherit config)
+     (authorized-keys
+      (hash-map->list cons all-keys)))))
 
 (define openssh-service-type
   (service-type (name 'openssh)
-- 
2.33.1

bug#51487: Acknowledgement (The openssh service does not allow multiple authorized key files per user)

[Vivien Kraus via Bug reports for GNU Guix]

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

Le vendredi 29 octobre 2021 à 23:22 +0200, Vivien Kraus a écrit :
> After some discussion on #guix, this seems to be the easier way to
> fix
> the problem:
Sorry, I forgot to update the commit message.

Vivien

[-- Attachment #2: 0001-gnu-openssh-service-Collect-all-keys-for-all-users.patch --]
[-- Type: text/x-patch, Size: 1291 bytes --]

From b2f47730a3d9aa97716741134917c340354d9c3a Mon Sep 17 00:00:00 2001
From: Vivien Kraus <vivien@planete-kraus.eu>
Date: Fri, 29 Oct 2021 18:25:24 +0200
Subject: [PATCH] gnu: openssh-service: Collect all keys for all users.

* gnu/services/ssh.scm (extend-openssh-authorized-keys): ensure that no key is forgotten.
---
 gnu/services/ssh.scm | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/gnu/services/ssh.scm b/gnu/services/ssh.scm
index a018052eeb..1309e062ce 100644
--- a/gnu/services/ssh.scm
+++ b/gnu/services/ssh.scm
@@ -532,10 +532,16 @@ (define (openssh-pam-services config)
 
 (define (extend-openssh-authorized-keys config keys)
   "Extend CONFIG with the extra authorized keys listed in KEYS."
-  (openssh-configuration
-   (inherit config)
-   (authorized-keys
-    (append (openssh-authorized-keys config) keys))))
+  (let ((all-keys (make-hash-table)))
+    (for-each
+     (match-lambda
+       ((user keys ...)
+        (hash-set! all-keys user (append (hash-ref all-keys user '()) keys))))
+     (append (openssh-authorized-keys config) keys))
+    (openssh-configuration
+     (inherit config)
+     (authorized-keys
+      (hash-map->list cons all-keys)))))
 
 (define openssh-service-type
   (service-type (name 'openssh)
-- 
2.33.1

bug#51487: The openssh service does not allow multiple authorized key files per user

[Ludovic Courtès]

Hi,

Vivien Kraus <vivien@planete-kraus.eu> skribis:

> From b2f47730a3d9aa97716741134917c340354d9c3a Mon Sep 17 00:00:00 2001
> From: Vivien Kraus <vivien@planete-kraus.eu>
> Date: Fri, 29 Oct 2021 18:25:24 +0200
> Subject: [PATCH] gnu: openssh-service: Collect all keys for all users.
>
> * gnu/services/ssh.scm (extend-openssh-authorized-keys): ensure that no key is forgotten.

Good catch!

> diff --git a/gnu/services/ssh.scm b/gnu/services/ssh.scm
> index a018052eeb..1309e062ce 100644
> --- a/gnu/services/ssh.scm
> +++ b/gnu/services/ssh.scm
> @@ -532,10 +532,16 @@ (define (openssh-pam-services config)
>  
>  (define (extend-openssh-authorized-keys config keys)
>    "Extend CONFIG with the extra authorized keys listed in KEYS."
> -  (openssh-configuration
> -   (inherit config)
> -   (authorized-keys
> -    (append (openssh-authorized-keys config) keys))))
> +  (let ((all-keys (make-hash-table)))
> +    (for-each
> +     (match-lambda
> +       ((user keys ...)
> +        (hash-set! all-keys user (append (hash-ref all-keys user '()) keys))))
> +     (append (openssh-authorized-keys config) keys))
> +    (openssh-configuration
> +     (inherit config)
> +     (authorized-keys
> +      (hash-map->list cons all-keys)))))

Could you write it in functional style using a vhash (info "(guile)
VHashes")?  You’ll probably need two list traversals: one to build the
user/key mapping, and one to compute the list of users.

Thanks in advance,
Ludo’.

bug#51487: The openssh service does not allow multiple authorized key files per user

[Vivien Kraus via Bug reports for GNU Guix]

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

Hello,

Ludovic Courtès <ludo@gnu.org> writes:
> Could you write it in functional style using a vhash (info "(guile)
> VHashes")?  You’ll probably need two list traversals: one to build the
> user/key mapping, and one to compute the list of users.

I thought that as the vhash data structure inherited the drawbacks of
vlist, it would not be worth using in place of a hash table, but you’re
saying that it’s still a better (more functional) data structure, noted.

Here is the new patch (and I also forgot that appending short lists to
long lists was not great, so I do all the appending at the end of the
function now).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-openssh-service-Collect-all-keys-for-all-users.patch --]
[-- Type: text/x-patch, Size: 2104 bytes --]

From a2c4d7cefbc71fd3d35b0b7cc2f61118bd3a29b2 Mon Sep 17 00:00:00 2001
From: Vivien Kraus <vivien@planete-kraus.eu>
Date: Fri, 29 Oct 2021 18:25:24 +0200
Subject: [PATCH] gnu: openssh-service: Collect all keys for all users.

* gnu/services/ssh.scm (extend-openssh-authorized-keys): ensure that no key is forgotten.
---
 gnu/services/ssh.scm | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/gnu/services/ssh.scm b/gnu/services/ssh.scm
index a018052eeb..6ddaf55eeb 100644
--- a/gnu/services/ssh.scm
+++ b/gnu/services/ssh.scm
@@ -39,6 +39,7 @@ (define-module (gnu services ssh)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 vlist)
   #:export (lsh-configuration
             lsh-configuration?
             lsh-service
@@ -532,10 +533,30 @@ (define (openssh-pam-services config)
 
 (define (extend-openssh-authorized-keys config keys)
   "Extend CONFIG with the extra authorized keys listed in KEYS."
-  (openssh-configuration
-   (inherit config)
-   (authorized-keys
-    (append (openssh-authorized-keys config) keys))))
+  (let generate-keys
+      ((user-keys
+        (append (openssh-authorized-keys config) keys))
+       ;; The by-user vhash indexes a list of list of keys for each user, the
+       ;; list of list is not concatenated eagerly to avoid quadratic
+       ;; complexity.
+       (by-user (alist->vhash '())))
+    (match user-keys
+      (()
+       (openssh-configuration
+        (inherit config)
+        (authorized-keys
+         (vhash-fold
+          (lambda (user keys other-users)
+            `((,user ,@(apply append (reverse keys))) ,@other-users))
+          '() by-user))))
+      (((user keys ...) other-user-keys ...)
+       (let ((existing
+              (match (vhash-assoc user by-user)
+                ((_ . keys) keys)
+                (#f '()))))
+         (generate-keys
+          other-user-keys
+          (vhash-cons user `(,keys ,@existing) by-user)))))))
 
 (define openssh-service-type
   (service-type (name 'openssh)
-- 
2.33.1


[-- Attachment #3: Type: text/plain, Size: 8 bytes --]


Vivien

bug#51487: The openssh service does not allow multiple authorized key files per user

[Ludovic Courtès]

Hi,

Vivien Kraus <vivien@planete-kraus.eu> skribis:

>  (define (extend-openssh-authorized-keys config keys)
>    "Extend CONFIG with the extra authorized keys listed in KEYS."
> -  (openssh-configuration
> -   (inherit config)
> -   (authorized-keys
> -    (append (openssh-authorized-keys config) keys))))
> +  (let generate-keys
> +      ((user-keys
> +        (append (openssh-authorized-keys config) keys))
> +       ;; The by-user vhash indexes a list of list of keys for each user, the
> +       ;; list of list is not concatenated eagerly to avoid quadratic
> +       ;; complexity.
> +       (by-user (alist->vhash '())))
> +    (match user-keys
> +      (()
> +       (openssh-configuration
> +        (inherit config)
> +        (authorized-keys
> +         (vhash-fold
> +          (lambda (user keys other-users)
> +            `((,user ,@(apply append (reverse keys))) ,@other-users))
> +          '() by-user))))
> +      (((user keys ...) other-user-keys ...)
> +       (let ((existing
> +              (match (vhash-assoc user by-user)
> +                ((_ . keys) keys)
> +                (#f '()))))
> +         (generate-keys
> +          other-user-keys
> +          (vhash-cons user `(,keys ,@existing) by-user)))))))

I find it a bit hard to read.  What I had in mind is along these lines:

  (match (openssh-authorized-keys config)
    (((users _ ...) ...)
     ;; Build a user/key-list mapping.
     (let ((user-keys (fold (lambda (spec table)
                              (match spec
                                ((user keys ...)
                                 (vhash-cons user keys table))))
                            vlist-null
                            (openssh-authorized-keys config))))
       ;; Coalesce the key lists associated with each user.
       (map (lambda (user)
              (concatenate (vhash-fold* cons '() user user-keys)))
            users))))

WDYT?

Thanks,
Ludo’.

bug#51487: The openssh service does not allow multiple authorized key files per user

[Vivien Kraus via Bug reports for GNU Guix]

[-- Attachment #1.1: Type: text/plain, Size: 939 bytes --]


Ludovic Courtès <ludo@gnu.org> writes:
> I find it a bit hard to read.  What I had in mind is along these lines:
>
>   (match (openssh-authorized-keys config)
>     (((users _ ...) ...)
>      ;; Build a user/key-list mapping.
>      (let ((user-keys (fold (lambda (spec table)
>                               (match spec
>                                 ((user keys ...)
>                                  (vhash-cons user keys table))))
>                             vlist-null
>                             (openssh-authorized-keys config))))
>        ;; Coalesce the key lists associated with each user.
>        (map (lambda (user)
>               (concatenate (vhash-fold* cons '() user user-keys)))
>             users))))

That’s way cleaner. I didn’t know of vhash-fold*, it seems to save the
day!

(just fixing the final map function not to forget the user name in the
alist, and removing "spec")


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Collect all users in ssh service --]
[-- Type: text/x-patch, Size: 1755 bytes --]

From 7bc8abcfd5024f5269c36dc8cb44803eb0ab29ba Mon Sep 17 00:00:00 2001
From: Vivien Kraus <vivien@planete-kraus.eu>
Date: Fri, 29 Oct 2021 18:25:24 +0200
Subject: [PATCH] gnu: openssh-service: Collect all keys for all users.

* gnu/services/ssh.scm (extend-openssh-authorized-keys): ensure that no key is forgotten.
---
 gnu/services/ssh.scm | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/gnu/services/ssh.scm b/gnu/services/ssh.scm
index a018052eeb..92b470aa96 100644
--- a/gnu/services/ssh.scm
+++ b/gnu/services/ssh.scm
@@ -39,6 +39,7 @@ (define-module (gnu services ssh)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 vlist)
   #:export (lsh-configuration
             lsh-configuration?
             lsh-service
@@ -535,7 +536,19 @@ (define (extend-openssh-authorized-keys config keys)
   (openssh-configuration
    (inherit config)
    (authorized-keys
-    (append (openssh-authorized-keys config) keys))))
+    (match (openssh-authorized-keys config)
+      (((users _ ...) ...)
+       ;; Build a user/key-list mapping.
+       (let ((user-keys (fold (match-lambda*
+                                (((user keys ...) table)
+                                 (vhash-cons user keys table)))
+                              vlist-null
+                              (openssh-authorized-keys config))))
+         ;; Coalesce the key lists associated with each user.
+         (map (lambda (user)
+                `(,user
+                  ,@(concatenate (vhash-fold* cons '() user user-keys))))
+              users)))))))
 
 (define openssh-service-type
   (service-type (name 'openssh)
-- 
2.33.1


[-- Attachment #1.3: Type: text/plain, Size: 8 bytes --]


Vivien

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

bug#51487: The openssh service does not allow multiple authorized key files per user

[Ludovic Courtès]

Hi,

Vivien Kraus <vivien@planete-kraus.eu> skribis:

> (just fixing the final map function not to forget the user name in the
> alist, and removing "spec")

Oops, indeed.

> From 7bc8abcfd5024f5269c36dc8cb44803eb0ab29ba Mon Sep 17 00:00:00 2001
> From: Vivien Kraus <vivien@planete-kraus.eu>
> Date: Fri, 29 Oct 2021 18:25:24 +0200
> Subject: [PATCH] gnu: openssh-service: Collect all keys for all users.
>
> * gnu/services/ssh.scm (extend-openssh-authorized-keys): ensure that no key is forgotten.

I realized we could just use ‘alist->vhash’ instead of (fold …) so I did
that.

Applied, thanks!

Ludo’.