* [bug#38478] [PATCH 0/4] "guix deploy" authenticates SSH servers [security] @ 2019-12-03 21:09 Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 1/4] ssh: Add 'authenticate-server*' and use it for offloading Ludovic Courtès 0 siblings, 1 reply; 10+ messages in thread From: Ludovic Courtès @ 2019-12-03 21:09 UTC (permalink / raw) To: 38478; +Cc: Ludovic Courtès 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 1/4] ssh: Add 'authenticate-server*' and use it for offloading. 2019-12-03 21:09 [bug#38478] [PATCH 0/4] "guix deploy" authenticates SSH servers [security] Ludovic Courtès @ 2019-12-03 21:15 ` Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 2/4] ssh: Always authenticate the server [security fix] Ludovic Courtès ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Ludovic Courtès @ 2019-12-03 21:15 UTC (permalink / raw) To: 38478; +Cc: Ludovic Courtès * 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@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@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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 2/4] ssh: Always authenticate the server [security fix]. 2019-12-03 21:15 ` [bug#38478] [PATCH 1/4] ssh: Add 'authenticate-server*' and use it for offloading Ludovic Courtès @ 2019-12-03 21:15 ` Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 3/4] ssh: 'open-ssh-session' can be passed the expected host key Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the " Ludovic Courtès 2 siblings, 0 replies; 10+ messages in thread From: Ludovic Courtès @ 2019-12-03 21:15 UTC (permalink / raw) To: 38478; +Cc: Ludovic Courtès 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 3/4] ssh: 'open-ssh-session' can be passed the expected host key. 2019-12-03 21:15 ` [bug#38478] [PATCH 1/4] ssh: Add 'authenticate-server*' and use it for offloading Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 2/4] ssh: Always authenticate the server [security fix] Ludovic Courtès @ 2019-12-03 21:15 ` Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the " Ludovic Courtès 2 siblings, 0 replies; 10+ messages in thread From: Ludovic Courtès @ 2019-12-03 21:15 UTC (permalink / raw) To: 38478; +Cc: Ludovic Courtès * 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@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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key. 2019-12-03 21:15 ` [bug#38478] [PATCH 1/4] ssh: Add 'authenticate-server*' and use it for offloading Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 2/4] ssh: Always authenticate the server [security fix] Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 3/4] ssh: 'open-ssh-session' can be passed the expected host key Ludovic Courtès @ 2019-12-03 21:15 ` Ludovic Courtès 2019-12-04 13:19 ` Jakob L. Kreuze 2 siblings, 1 reply; 10+ messages in thread From: Ludovic Courtès @ 2019-12-03 21:15 UTC (permalink / raw) To: 38478; +Cc: Ludovic Courtès * 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@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))))) \f ;;; -- 2.24.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key. 2019-12-03 21:15 ` [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the " Ludovic Courtès @ 2019-12-04 13:19 ` Jakob L. Kreuze 2019-12-04 17:33 ` Ludovic Courtès 0 siblings, 1 reply; 10+ messages in thread From: Jakob L. Kreuze @ 2019-12-04 13:19 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 38478 [-- Attachment #1: Type: text/plain, Size: 184 bytes --] 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key. 2019-12-04 13:19 ` Jakob L. Kreuze @ 2019-12-04 17:33 ` Ludovic Courtès 2019-12-06 0:50 ` Jakob L. Kreuze 0 siblings, 1 reply; 10+ messages in thread From: Ludovic Courtès @ 2019-12-04 17:33 UTC (permalink / raw) To: Jakob L. Kreuze; +Cc: 38478 Hi! zerodaysfordays@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’. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key. 2019-12-04 17:33 ` Ludovic Courtès @ 2019-12-06 0:50 ` Jakob L. Kreuze 2019-12-06 12:16 ` Ludovic Courtès 0 siblings, 1 reply; 10+ messages in thread From: Jakob L. Kreuze @ 2019-12-06 0:50 UTC (permalink / raw) To: Ludovic Courtès; +Cc: 38478 [-- Attachment #1: Type: text/plain, Size: 651 bytes --] Ludovic Courtès <ludo@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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key. 2019-12-06 0:50 ` Jakob L. Kreuze @ 2019-12-06 12:16 ` Ludovic Courtès 2019-12-07 0:04 ` Ludovic Courtès 0 siblings, 1 reply; 10+ messages in thread From: Ludovic Courtès @ 2019-12-06 12:16 UTC (permalink / raw) To: Jakob L. Kreuze; +Cc: 38478 Hi! zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis: > Ludovic Courtès <ludo@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’. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key. 2019-12-06 12:16 ` Ludovic Courtès @ 2019-12-07 0:04 ` Ludovic Courtès 0 siblings, 0 replies; 10+ messages in thread From: Ludovic Courtès @ 2019-12-07 0:04 UTC (permalink / raw) To: Jakob L. Kreuze; +Cc: 38478 Ludovic Courtès <ludo@gnu.org> skribis: > zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis: > >> Ludovic Courtès <ludo@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’. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-12-07 0:06 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-12-03 21:09 [bug#38478] [PATCH 0/4] "guix deploy" authenticates SSH servers [security] Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 1/4] ssh: Add 'authenticate-server*' and use it for offloading Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 2/4] ssh: Always authenticate the server [security fix] Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 3/4] ssh: 'open-ssh-session' can be passed the expected host key Ludovic Courtès 2019-12-03 21:15 ` [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the " Ludovic Courtès 2019-12-04 13:19 ` Jakob L. Kreuze 2019-12-04 17:33 ` Ludovic Courtès 2019-12-06 0:50 ` Jakob L. Kreuze 2019-12-06 12:16 ` Ludovic Courtès 2019-12-07 0:04 ` Ludovic Courtès
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/guix.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.