GNU bug report logs - #38478
[PATCH 0/4] "guix deploy" authenticates SSH servers [security]

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 38478 in the body.
You can then email your comments to 38478 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 0/4] "guix deploy" authenticates SSH servers [security]
Date: Tue,  3 Dec 2019 22:09:58 +0100

Hi!

This series allow users to specify the remote host key in
<machine-ssh-configuration> used for “guix deploy”, so you
can have that under version control and entirely managed by
Guix, like “guix offload” does.

The second patch fixes a security issue: ‘open-ssh-session’ from
(guix ssh), which is used by “guix deploy” and support for
“GUIX_DAEMON_SOCKET=ssh://…” in (guix store ssh), would not
authenticate the server it’s talking to.

Feedback welcome!

Ludo’.

Ludovic Courtès (4):
  ssh: Add 'authenticate-server*' and use it for offloading.
  ssh: Always authenticate the server [security fix].
  ssh: 'open-ssh-session' can be passed the expected host key.
  machine: ssh: <machine-ssh-configuration> can include the host key.

 doc/guix.texi            | 12 +++++++
 gnu/machine/ssh.scm      |  9 ++++--
 guix/scripts/offload.scm | 30 ++---------------
 guix/ssh.scm             | 69 ++++++++++++++++++++++++++++++++++++++--
 4 files changed, 87 insertions(+), 33 deletions(-)

-- 
2.24.0

Message #8 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 38478 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 1/4] ssh: Add 'authenticate-server*' and use it for offloading.
Date: Tue,  3 Dec 2019 22:15:54 +0100

* guix/scripts/offload.scm (host-key->type+key): Remove.
(open-ssh-session): Replace server authentication code with a call to
'authenticate-server*'.
* guix/ssh.scm (host-key->type+key, authenticate-server*): New
procedures.
---
 guix/scripts/offload.scm | 30 ++----------------------------
 guix/ssh.scm             | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/guix/scripts/offload.scm b/guix/scripts/offload.scm
index 18473684eb..e81b6c25f2 100644
--- a/guix/scripts/offload.scm
+++ b/guix/scripts/offload.scm
@@ -149,19 +149,6 @@ ignoring it~%")
          (leave (G_ "failed to load machine file '~a': ~s~%")
                 file args))))))
 
-(define (host-key->type+key host-key)
-  "Destructure HOST-KEY, an OpenSSH host key string, and return two values:
-its key type as a symbol, and the actual base64-encoded string."
-  (define (type->symbol type)
-    (and (string-prefix? "ssh-" type)
-         (string->symbol (string-drop type 4))))
-
-  (match (string-tokenize host-key)
-    ((type key x)
-     (values (type->symbol type) key))
-    ((type key)
-     (values (type->symbol type) key))))
-
 (define (private-key-from-file* file)
   "Like 'private-key-from-file', but raise an error that 'with-error-handling'
 can interpret meaningfully."
@@ -203,21 +190,8 @@ private key from '~a': ~a")
                                (build-machine-compression-level machine))))
     (match (connect! session)
       ('ok
-       ;; Authenticate the server.  XXX: Guile-SSH 0.10.1 doesn't know about
-       ;; ed25519 keys and 'get-key-type' returns #f in that case.
-       (let-values (((server)   (get-server-public-key session))
-                    ((type key) (host-key->type+key
-                                 (build-machine-host-key machine))))
-         (unless (and (or (not (get-key-type server))
-                          (eq? (get-key-type server) type))
-                      (string=? (public-key->string server) key))
-           ;; Key mismatch: something's wrong.  XXX: It could be that the server
-           ;; provided its Ed25519 key when we where expecting its RSA key.
-           (leave (G_ "server at '~a' returned host key '~a' of type '~a' \
-instead of '~a' of type '~a'~%")
-                  (build-machine-name machine)
-                  (public-key->string server) (get-key-type server)
-                  key type)))
+       ;; Make sure the server's key is what we expect.
+       (authenticate-server* session (build-machine-host-key machine))
 
        (let ((auth (userauth-public-key! session private)))
          (unless (eq? 'success auth)
diff --git a/guix/ssh.scm b/guix/ssh.scm
index 5fd3c280e8..f34e71392b 100644
--- a/guix/ssh.scm
+++ b/guix/ssh.scm
@@ -37,6 +37,8 @@
   #:use-module (ice-9 format)
   #:use-module (ice-9 binary-ports)
   #:export (open-ssh-session
+            authenticate-server*
+
             remote-inferior
             remote-daemon-channel
             connect-to-remote-daemon
@@ -60,6 +62,41 @@
 (define %compression
   "zlib <at> openssh.com,zlib")
 
+(define (host-key->type+key host-key)
+  "Destructure HOST-KEY, an OpenSSH host key string, and return two values:
+its key type as a symbol, and the actual base64-encoded string."
+  (define (type->symbol type)
+    (and (string-prefix? "ssh-" type)
+         (string->symbol (string-drop type 4))))
+
+  (match (string-tokenize host-key)
+    ((type key x)
+     (values (type->symbol type) key))
+    ((type key)
+     (values (type->symbol type) key))))
+
+(define (authenticate-server* session key)
+  "Make sure the server for SESSION has the given KEY, where KEY is a string
+such as \"ssh-ed25519 AAAAC3Nz… root <at> example.org\".  Raise an exception if the
+actual key does not match."
+  (let-values (((server)   (get-server-public-key session))
+               ((type key) (host-key->type+key key)))
+    (unless (and (or (not (get-key-type server))
+                     (eq? (get-key-type server) type))
+                 (string=? (public-key->string server) key))
+      ;; Key mismatch: something's wrong.  XXX: It could be that the server
+      ;; provided its Ed25519 key when we where expecting its RSA key.  XXX:
+      ;; Guile-SSH 0.10.1 doesn't know about ed25519 keys and 'get-key-type'
+      ;; returns #f in that case.
+      (raise (condition
+              (&message
+               (message (format #f (G_ "server at '~a' returned host key \
+'~a' of type '~a' instead of '~a' of type '~a'~%")
+                                (session-get session 'host)
+                                (public-key->string server)
+                                (get-key-type server)
+                                key type))))))))
+
 (define* (open-ssh-session host #:key user port identity
                            (compression %compression)
                            (timeout 3600))
-- 
2.24.0

Message #11 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 38478 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 3/4] ssh: 'open-ssh-session' can be passed the expected host
 key.
Date: Tue,  3 Dec 2019 22:15:56 +0100

* guix/ssh.scm (open-ssh-session): Add #:host-key parameter.
Pass #:knownhosts to 'make-session'.  When HOST-KEY is true, call
'authenticate-server*' instead of 'authenticate-server'.
---
 guix/ssh.scm | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/guix/ssh.scm b/guix/ssh.scm
index 519c723155..291ce20b61 100644
--- a/guix/ssh.scm
+++ b/guix/ssh.scm
@@ -98,14 +98,20 @@ actual key does not match."
                                 key type))))))))
 
 (define* (open-ssh-session host #:key user port identity
+                           host-key
                            (compression %compression)
                            (timeout 3600))
   "Open an SSH session for HOST and return it.  IDENTITY specifies the file
 name of a private key to use for authenticating with the host.  When USER,
 PORT, or IDENTITY are #f, use default values or whatever '~/.ssh/config'
-specifies; otherwise use them.  Install TIMEOUT as the maximum time in seconds
-after which a read or write operation on a channel of the returned session is
-considered as failing.
+specifies; otherwise use them.
+
+When HOST-KEY is true, it must be a string like \"ssh-ed25519 AAAAC3Nz…
+root <at> example.org\"; the server is authenticated and an error is raised if its
+host key is different from HOST-KEY.
+
+Install TIMEOUT as the maximum time in seconds after which a read or write
+operation on a channel of the returned session is considered as failing.
 
 Throw an error on failure."
   (let ((session (make-session #:user user
@@ -115,6 +121,11 @@ Throw an error on failure."
                                #:timeout 10       ;seconds
                                ;; #:log-verbosity 'protocol
 
+                               ;; Prevent libssh from reading
+                               ;; ~/.ssh/known_hosts when the caller provides
+                               ;; a HOST-KEY to match against.
+                               #:knownhosts (and host-key "/dev/null")
+
                                ;; We need lightweight compression when
                                ;; exchanging full archives.
                                #:compression compression
@@ -125,16 +136,20 @@ Throw an error on failure."
 
     (match (connect! session)
       ('ok
-       ;; Authenticate against ~/.ssh/known_hosts.
-       (match (authenticate-server session)
-         ('ok #f)
-         (reason
-          (raise (condition
-                  (&message
-                   (message (format #f (G_ "failed to authenticate \
+       (if host-key
+           ;; Make sure the server's key is what we expect.
+           (authenticate-server* session host-key)
+
+           ;; Authenticate against ~/.ssh/known_hosts.
+           (match (authenticate-server session)
+             ('ok #f)
+             (reason
+              (raise (condition
+                      (&message
+                       (message (format #f (G_ "failed to authenticate \
 server at '~a': ~a")
-                                    (session-get session 'host)
-                                    reason)))))))
+                                        (session-get session 'host)
+                                        reason))))))))
 
        ;; Use public key authentication, via the SSH agent if it's available.
        (match (userauth-public-key/auto! session)
-- 
2.24.0

Message #14 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 38478 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 2/4] ssh: Always authenticate the server [security fix].
Date: Tue,  3 Dec 2019 22:15:55 +0100

Until now, users of 'open-ssh-session', including "guix deploy" and
"GUIX_DAEMON_SOCKET=ssh://…" (but not "guix offload"), would not
authenticate the SSH server they're talking to.

* guix/ssh.scm (open-ssh-session): Call 'authenticate-server'.
---
 guix/ssh.scm | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/guix/ssh.scm b/guix/ssh.scm
index f34e71392b..519c723155 100644
--- a/guix/ssh.scm
+++ b/guix/ssh.scm
@@ -125,6 +125,17 @@ Throw an error on failure."
 
     (match (connect! session)
       ('ok
+       ;; Authenticate against ~/.ssh/known_hosts.
+       (match (authenticate-server session)
+         ('ok #f)
+         (reason
+          (raise (condition
+                  (&message
+                   (message (format #f (G_ "failed to authenticate \
+server at '~a': ~a")
+                                    (session-get session 'host)
+                                    reason)))))))
+
        ;; Use public key authentication, via the SSH agent if it's available.
        (match (userauth-public-key/auto! session)
          ('success
-- 
2.24.0

Message #17 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 38478 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the
 host key.
Date: Tue,  3 Dec 2019 22:15:57 +0100

* gnu/machine/ssh.scm (<machine-ssh-configuration>)[host-key]: New field.
(machine-ssh-session): Pass #:host-key to 'open-ssh-session'.
* doc/guix.texi (Invoking guix deploy): Document it.
---
 doc/guix.texi       | 12 ++++++++++++
 gnu/machine/ssh.scm |  9 +++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 2da1ecd64c..e6e015ad3e 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -26412,6 +26412,18 @@ keyring.
 @item @code{identity} (default: @code{#f})
 If specified, the path to the SSH private key to use to authenticate with the
 remote host.
+
+@item @code{host-key} (default: @code{#f})
+This should be the SSH host key of the machine, which looks like this:
+
+@example
+ssh-ed25519 AAAAC3Nz <at> dots{} root@@example.org
+@end example
+
+When @code{host-key} is @code{#f}, the server is authenticated against
+the @file{~/.ssh/known_hosts} file, just like the OpenSSH @command{ssh}
+client does.
+
 @end table
 @end deftp
 
diff --git a/gnu/machine/ssh.scm b/gnu/machine/ssh.scm
index 6e3ed0e092..23ae917b79 100644
--- a/gnu/machine/ssh.scm
+++ b/gnu/machine/ssh.scm
@@ -54,6 +54,7 @@
             machine-ssh-configuration-authorize?
             machine-ssh-configuration-port
             machine-ssh-configuration-user
+            machine-ssh-configuration-host-key
             machine-ssh-configuration-session))
 
 ;;; Commentary:
@@ -87,6 +88,8 @@
   (identity       machine-ssh-configuration-identity       ; path to a private key
                   (default #f))
   (session        machine-ssh-configuration-session        ; session
+                  (default #f))
+  (host-key       machine-ssh-configuration-host-key       ; #f | string
                   (default #f)))
 
 (define (machine-ssh-session machine)
@@ -98,11 +101,13 @@ one from the configuration's parameters if one was not provided."
         (let ((host-name (machine-ssh-configuration-host-name config))
               (user (machine-ssh-configuration-user config))
               (port (machine-ssh-configuration-port config))
-              (identity (machine-ssh-configuration-identity config)))
+              (identity (machine-ssh-configuration-identity config))
+              (host-key (machine-ssh-configuration-host-key config)))
           (open-ssh-session host-name
                             #:user user
                             #:port port
-                            #:identity identity)))))
+                            #:identity identity
+                            #:host-key host-key)))))
 
 
 ;;;
-- 
2.24.0

Message #20 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: zerodaysfordays <at> sdf.lonestar.org (Jakob L. Kreuze)
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 38478 <at> debbugs.gnu.org
Subject: Re: [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration>
 can include the host key.
Date: Wed, 04 Dec 2019 08:19:59 -0500

[Message part 1 (text/plain, inline)]

I've only been able to follow the updates to "guix deploy" somewhat
tangentially, but I was very excited to see this patch in my inbox.
Thumbs up from me, thanks Ludo!

Regards,
Jakob

[signature.asc (application/pgp-signature, inline)]

Message #25 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: zerodaysfordays <at> sdf.lonestar.org (Jakob L. Kreuze)
Cc: 38478 <at> debbugs.gnu.org
Subject: Re: [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration>
 can include the host key.
Date: Wed, 04 Dec 2019 18:33:42 +0100

Hi!

zerodaysfordays <at> sdf.lonestar.org (Jakob L. Kreuze) skribis:

> I've only been able to follow the updates to "guix deploy" somewhat
> tangentially, but I was very excited to see this patch in my inbox.
> Thumbs up from me, thanks Ludo!

Heheh, thank you!

I went ahead and pushed it as it seemed like a good idea to not wait.

BTW, I’m wondering if we should go further and deprecate missing/#f
‘host-key’ fields altogether.  WDYT?

To me it just seems wiser to have that info within the deploy config
rather than out-of-band in ~/.ssh/known_hosts.

Ludo’.

Message #32 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: zerodaysfordays <at> sdf.lonestar.org (Jakob L. Kreuze)
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 38478 <at> debbugs.gnu.org
Subject: Re: [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration>
 can include the host key.
Date: Thu, 05 Dec 2019 19:50:13 -0500

[Message part 1 (text/plain, inline)]

Ludovic Courtès <ludo <at> gnu.org> writes:

> I went ahead and pushed it as it seemed like a good idea to not wait.

Agreed :)

> BTW, I’m wondering if we should go further and deprecate missing/#f
> ‘host-key’ fields altogether.  WDYT?
>
> To me it just seems wiser to have that info within the deploy config
> rather than out-of-band in ~/.ssh/known_hosts.

I feel that's more in-line with the goals of Guix -- implicitly reading
~/.ssh/known_hosts doesn't seem declarative to me. What's our means for
deprecating features like that? A warning message when omitted? If
that's the case, I'm definitely on board.

Regards,
Jakob

[signature.asc (application/pgp-signature, inline)]

Message #35 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: zerodaysfordays <at> sdf.lonestar.org (Jakob L. Kreuze)
Cc: 38478 <at> debbugs.gnu.org
Subject: Re: [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration>
 can include the host key.
Date: Fri, 06 Dec 2019 13:16:41 +0100

Hi!

zerodaysfordays <at> sdf.lonestar.org (Jakob L. Kreuze) skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:

[...]

>> BTW, I’m wondering if we should go further and deprecate missing/#f
>> ‘host-key’ fields altogether.  WDYT?
>>
>> To me it just seems wiser to have that info within the deploy config
>> rather than out-of-band in ~/.ssh/known_hosts.
>
> I feel that's more in-line with the goals of Guix -- implicitly reading
> ~/.ssh/known_hosts doesn't seem declarative to me. What's our means for
> deprecating features like that? A warning message when omitted? If
> that's the case, I'm definitely on board.

Yup, we can emit a deprecation warning when the key is #f.

So let’s take that route if nobody objects.  It’s easier to deprecate it
now that “guix deploy” is still very new.

Ludo’.

Message #38 received at 38478 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: zerodaysfordays <at> sdf.lonestar.org (Jakob L. Kreuze)
Cc: 38478 <at> debbugs.gnu.org
Subject: Re: [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration>
 can include the host key.
Date: Sat, 07 Dec 2019 01:04:42 +0100

Ludovic Courtès <ludo <at> gnu.org> skribis:

> zerodaysfordays <at> sdf.lonestar.org (Jakob L. Kreuze) skribis:
>
>> Ludovic Courtès <ludo <at> gnu.org> writes:
>
> [...]
>
>>> BTW, I’m wondering if we should go further and deprecate missing/#f
>>> ‘host-key’ fields altogether.  WDYT?
>>>
>>> To me it just seems wiser to have that info within the deploy config
>>> rather than out-of-band in ~/.ssh/known_hosts.
>>
>> I feel that's more in-line with the goals of Guix -- implicitly reading
>> ~/.ssh/known_hosts doesn't seem declarative to me. What's our means for
>> deprecating features like that? A warning message when omitted? If
>> that's the case, I'm definitely on board.
>
> Yup, we can emit a deprecation warning when the key is #f.
>
> So let’s take that route if nobody objects.  It’s easier to deprecate it
> now that “guix deploy” is still very new.

Done in commit 2617d956d8ae122128a1ba2cc74983cbd683b042!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.