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

  • Done
  • quality assurance status badge
Details
2 participants
  • Ludovic Courtès
  • Vivien Kraus
Owner
unassigned
Submitted by
Vivien Kraus
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 an email to 51487@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 51487
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