The openssh service does not allow multiple authorized key files per user

DoneSubmitted by Vivien Kraus.
Details
2 participants
  • Ludovic Courtès
  • Vivien Kraus
Owner
unassigned
Severity
normal
V
V
Vivien Kraus wrote on 29 Oct 2021 18:15
(address . bug-guix@gnu.org)
87fssjvmbp.fsf@planete-kraus.eu
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
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCAAdFiEEq4yIHjMvkliPpwQnO7C8EjLYuCwFAmF8HjoACgkQO7C8EjLY
uCxCIQv+NcyLkPpKJGrBT2ibgR1DkOO0sOad46VtBCgu8rCwmU+A5na6X4/k6cW/
IQniJhaaVv9BorI269rYch7vCQC5V4Vy0gCxrMhA4bWzHgTjB7J7Rz4oVB3XpvmS
Fe5an8kgr/hxMruanWGBRVCcbxPhjeervhcnYiicz2g6OWnq0CfhLv0i0rn6gpeY
/PRIlj4ehaL3UUIApAr+M9rzRNa4cg7jk2NXPbpfcx9YdYLOCFXT7qR0XXBFkSFj
LTaU0+Ex0Zxa5OFCW8eoqK5j8YfaL+UNGK7JayVuwJNL9FhEJclvO1inyCCb1p3R
ckgKnbhyQMg/MI2tL8FnXNh8jw99p2pvzMJ0VGnYhjOoiLruy8Tk6tdj5OkcY3Yb
BcKtBW3fDBV2Y9fXJZUK+DFwRgHo618zwe5QqNRpDvnfYNqXvAoteR7I0AoAgWMt
8a0pNMHFaT/cj3+qlOg9luveRyh+Amw9qMdO1Et1JJ+99SkEG5/OjDPPsD9qbYhs
irRrjAEb
=O9Al
-----END PGP SIGNATURE-----

V
V
Vivien Kraus wrote on 29 Oct 2021 18:39
Re: bug#51487: Acknowledgement (The openssh service does not allow multiple authorized key files per user)
(address . 51487@debbugs.gnu.org)
87bl37vl0d.fsf@planete-kraus.eu
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
Content-Type: text/x-patch
Content-ID: <87a6irvl0d.fsf@planete-kraus.eu>
Content-Transfer-Encoding: binary




-----BEGIN PGP SIGNATURE-----

iQGzBAEBCAAdFiEEq4yIHjMvkliPpwQnO7C8EjLYuCwFAmF8JOIACgkQO7C8EjLY
uCyI3QwAmU2y8iJYOfVR0ZgHbmX/rdeIbpWHtM6TFY3oH508SdQyoCwQDJxymQVE
SqG6xi2DcEBOZEYULGDA0EAfjnz37BHMTNWlOyuZ4jLcxJONCqEJeHz5nX40vV9x
1uWgxQ+ReTSloCjxxLbGH4aeCmFHPB4vvTqPuR3cZS6APTID+iKkju84FgIRr6Vl
3IKs8hxHr7ykBG88Q9OMnplDJcBjZwOg2rAF60+3XkUJxJsR616wTuW57rXAnQr8
MXxdjCGjQjWjv3scsmYx0sp0+ZT25JGCR5ymgkHNS6bw1v5YUHoKkOmXMB5A6b+N
Snjp9Qy4A8TIeZrk1XGq+f53/el12A2Vtq2Q8dJWZQO3IeZprs6GoSCDfmW2LwM1
6/yzbHYhQXAeWryZbvfk540cYcMHPbQQALe6hFrOJDQCIz56nq03UBrnWHCEVvZy
DmlnnNwLc/GuYAgASwiOfBjLxLkuf6XoZsCwcO4uHuEyUylSrPdi+ivepwP9bF3t
GcnzbUMO
=Q06M
-----END PGP SIGNATURE-----

V
V
Vivien Kraus wrote on 29 Oct 2021 18:45
(address . 51487@debbugs.gnu.org)
875ytfvkw4.fsf@planete-kraus.eu
Vivien Kraus <vivien@planete-kraus.eu> writes:
Toggle quote (18 lines)
> 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.
Content-Type: text/x-patch
Content-ID: <874k8zvkw4.fsf@planete-kraus.eu>
Content-Transfer-Encoding: binary




Toggle quote (2 lines)
>
> Vivien
V
V
Vivien Kraus wrote on 29 Oct 2021 18:51
(address . 51487@debbugs.gnu.org)
5e2cb25499ce79f6afc6b8fc775b6ff8e5817670.camel@planete-kraus.eu
The patch does not seem to get formatted correctly, sorry. Hopefully,
this should work.

Vivien
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(-)

Toggle diff (41 lines)
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
V
V
Vivien Kraus wrote on 29 Oct 2021 23:22
(address . 51487@debbugs.gnu.org)
a3baa9271aba2624bcdcc5f831d071a7dd792128.camel@planete-kraus.eu
After some discussion on #guix, this seems to be the easier way to fix
the problem:

Vivien
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(-)

Toggle diff (27 lines)
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
V
V
Vivien Kraus wrote on 29 Oct 2021 23:26
(address . 51487@debbugs.gnu.org)
e9d6b181ddd0dd156a10bbba9500da11a8f4cfde.camel@planete-kraus.eu
Le vendredi 29 octobre 2021 à 23:22 +0200, Vivien Kraus a écrit :
Toggle quote (3 lines)
> 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
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(-)

Toggle diff (27 lines)
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
L
L
Ludovic Courtès wrote on 7 Nov 2021 16:04
Re: bug#51487: The openssh service does not allow multiple authorized key files per user
(name . Vivien Kraus)(address . vivien@planete-kraus.eu)(address . 51487@debbugs.gnu.org)
87fss8knw3.fsf_-_@gnu.org
Hi,

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

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

Toggle quote (23 lines)
> 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’.
V
V
Vivien Kraus wrote on 7 Nov 2021 18:29
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 51487@debbugs.gnu.org)
87lf1zc1lg.fsf@planete-kraus.eu
Hello,

Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (4 lines)
> 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).
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(-)

Toggle diff (49 lines)
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
Vivien
L
L
Ludovic Courtès wrote on 15 Nov 2021 15:42
(name . Vivien Kraus)(address . vivien@planete-kraus.eu)(address . 51487@debbugs.gnu.org)
874k8d5vl7.fsf@gnu.org
Hi,

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

Toggle quote (31 lines)
> (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’.
V
V
Vivien Kraus wrote on 15 Nov 2021 16:31
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 51487@debbugs.gnu.org)
87fsrx4eku.fsf@planete-kraus.eu
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (16 lines)
> 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")
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(-)

Toggle diff (35 lines)
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
Vivien
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCAAdFiEEq4yIHjMvkliPpwQnO7C8EjLYuCwFAmGSfjIACgkQO7C8EjLY
uCxvrAv/bYqiYgs6ji87X0y9J2WR0yuK93iiWj55yPLkWOY5omw6B8GkpGKjbfij
a50hPRizgM7G99cfRKIveoD0dobHQdmGH/1M/8yQshfgcqOOm77Or5pCMWhS9I3v
UfLwzyMvZBA5eN+n9YNrAcxI+exysl/pwDsbRpaHlwIlFjzIIxMF5T/0abcd8J3L
2FjMxNvNMV91CWrelCtQwMXy+kimPe7tRg/PT4hXU154RMMX2kF1J4grSSZxZRaz
MmhajxwF1iMLb94EPT7PIk4Gr6DlEULYAwMKJKiSwY6hbFs1VwsUjMRGvnmcazaw
SJPQl0Y52m6KGg+kz+r9lyEFS5dTzD29xVN6+RVowP61/59Z55BP7yByOY1fe1qH
vzgvqIamIvodf6Yr4k7eI01SCi5RghX3xNJywf+XLMTpWa04LfLKwL+CbQfbzwN5
bI2iZw2kPinU66PaoAcixvQQidf0CQw7Bb58HKN/PU/xVxb2Es1AUaEwexM1rx8V
LCQSkV0x
=CkkV
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 16 Nov 2021 10:03
(name . Vivien Kraus)(address . vivien@planete-kraus.eu)(address . 51487-done@debbugs.gnu.org)
874k8c4gmg.fsf@gnu.org
Hi,

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

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

Oops, indeed.

Toggle quote (7 lines)
> 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’.
Closed
?
Your comment

This issue is archived.

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