[PATCH 0/4] "guix deploy" authenticates SSH servers [security]

DoneSubmitted by Ludovic Courtès.
Details
2 participants
  • Ludovic Courtès
  • Jakob L. Kreuze
Owner
unassigned
Severity
normal
L
L
Ludovic Courtès wrote on 3 Dec 2019 22:09
(address . guix-patches@gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20191203210958.20936-1-ludo@gnu.org
Hi!
This series allow users to specify the remote host key in<machine-ssh-configuration> used for “guix deploy”, so youcan have that under version control and entirely managed byGuix, 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 notauthenticate 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
L
L
Ludovic Courtès wrote on 3 Dec 2019 22:15
[PATCH 1/4] ssh: Add 'authenticate-server*' and use it for offloading.
(address . 38478@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20191203211557.21145-1-ludo@gnu.org
* 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*): Newprocedures.--- guix/scripts/offload.scm | 30 ++---------------------------- guix/ssh.scm | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 28 deletions(-)
Toggle diff (105 lines)diff --git a/guix/scripts/offload.scm b/guix/scripts/offload.scmindex 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.scmindex 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
L
L
Ludovic Courtès wrote on 3 Dec 2019 22:15
[PATCH 3/4] ssh: 'open-ssh-session' can be passed the expected host key.
(address . 38478@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20191203211557.21145-3-ludo@gnu.org
* 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(-)
Toggle diff (72 lines)diff --git a/guix/ssh.scm b/guix/ssh.scmindex 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
L
L
Ludovic Courtès wrote on 3 Dec 2019 22:15
[PATCH 2/4] ssh: Always authenticate the server [security fix].
(address . 38478@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20191203211557.21145-2-ludo@gnu.org
Until now, users of 'open-ssh-session', including "guix deploy" and"GUIX_DAEMON_SOCKET=ssh://…" (but not "guix offload"), would notauthenticate 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(+)
Toggle diff (24 lines)diff --git a/guix/ssh.scm b/guix/ssh.scmindex 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
L
L
Ludovic Courtès wrote on 3 Dec 2019 22:15
[PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key.
(address . 38478@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
20191203211557.21145-4-ludo@gnu.org
* 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(-)
Toggle diff (62 lines)diff --git a/doc/guix.texi b/doc/guix.texiindex 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.scmindex 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
J
J
Jakob L. Kreuze wrote on 4 Dec 2019 14:19
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 38478@debbugs.gnu.org)
87d0d4qlc0.fsf@sdf.lonestar.org
I've only been able to follow the updates to "guix deploy" somewhattangentially, but I was very excited to see this patch in my inbox.Thumbs up from me, thanks Ludo!
Regards,Jakob
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEEa1VJLOiXAjQ2BGSm9Qb9Fp2P2VoFAl3nsoEACgkQ9Qb9Fp2P2VrCyg/+PiUW7T+Ag6SUAXVjv7cxgSAgBhyeJ27Qx0Bk3hZN1OMGu8817i+c/m8a1xeLgziJ5IQE2t3s+uDhFxW1WNq3ve6EwF0yxTZYKP9+V651ng0p0U868VtwDoprz/Y0HnvCw+dGuK71J30BjtCz/vyi/1GaDnIityfN617IlbX9hRG3Ug6UGAuq7x/sS4ZPcwmLjxoj8yuB5PvczLtQJwc9jQIfu6s6fyNA1lta6rs78tOdKI7UzvO7VCqhUvT2QlzC7VeA1VeMc41nebLTFvmiIG0i4oPMzHagbfXE+g0DXcGdTz1CgNi2fkT7/wzFdeN2707d8ZH2MYjMbEsoBJU4B3rt+R1wplG5QT8eU0DVm3TpHm66ry6pXkFR3I/p8LOpm+kM0TeW1aYI1ZxfKT+5fvuGvRA7iGhdIQAGKPw/Rj4XmFpIKnFE0ai2wtZzoJVrYb2lrl2jBsA+T2FI7MPDxSOFlHbKM2WYb4CA//1wJsyRqnFqRlDWS2AQQoPrMOtIeRgEJUf40jzXF4FF9EGMVg1PuDdpc6Fmbkc5b5CJuMHckFtKihlGQsQbzK/bQI67WDHjU+IM/RD0NrDOxy/DcGRIF0Php4A2ZBOteE0lk3YQWPjhEREU6Tt6KEmZicYk41Oj6rz1HKa1m2mDj8uAmBTBxlG2L0ial7fRqnzQzfY==jTji-----END PGP SIGNATURE-----
L
L
Ludovic Courtès wrote on 4 Dec 2019 18:16
control message for bug #38478
(address . control@debbugs.gnu.org)
875ziwc8or.fsf@gnu.org
tags 38478 + securityquit
L
L
Ludovic Courtès wrote on 4 Dec 2019 18:33
Re: [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key.
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)(address . 38478@debbugs.gnu.org)
87tv6gatc9.fsf@gnu.org
Hi!
zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis:
Toggle quote (4 lines)> 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 configrather than out-of-band in ~/.ssh/known_hosts.
Ludo’.
L
L
Ludovic Courtès wrote on 4 Dec 2019 18:33
control message for bug #38478
(address . control@debbugs.gnu.org)
87sgm0atbv.fsf@gnu.org
tags 38478 fixedclose 38478 quit
J
J
Jakob L. Kreuze wrote on 6 Dec 2019 01:50
Re: [bug#38478] [PATCH 4/4] machine: ssh: <machine-ssh-configuration> can include the host key.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 38478@debbugs.gnu.org)
87eexil1kq.fsf@sdf.lonestar.org
Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (2 lines)> I went ahead and pushed it as it seemed like a good idea to not wait.
Agreed :)
Toggle quote (6 lines)> 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 fordeprecating features like that? A warning message when omitted? Ifthat's the case, I'm definitely on board.
Regards,Jakob
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEEa1VJLOiXAjQ2BGSm9Qb9Fp2P2VoFAl3ppccACgkQ9Qb9Fp2P2VoOtQ//bsGKOOOJbZJ9xfSHTiuz3BaBO4kk1VM9sMqbIJE1IfdvavM4fGAem82glVJGsIdPLFtGcDnMETuobRUpP7u4qhrn1sBAhvUqmEO5iLCBXOXUI5W4fSkIYUnfD98H9Pg0qE5yfru598ldCwhn3vJ3WAncwebmLbOrgSyNVKBlboLXt7JUG6xTgv5dzsMVog47uIK5RfWDhw5T3GblfKijmIapqg32/W7GoHDRJ94/+Z/KBRd9iqeJSydl9QSuntdp+5m5O7bjCzrNJBCtuMpJ6VLmG1sNLjdwDAbDEzvY5T7OJMvZdtrnbbPd7GlUhz7Wsc1d7LqQ6JomqGLmfQQ3JiU0As5k4XFNbN+ZkOo2xaF3N6wutWP6DgJBkt3Mupo8erdQbmgjeSGkVRff+7naIOIv+U5DJ6BsHdHe7F0ljzHKCjOsBvpyFBIDbyCijr/szfXujiAME5xZv9SK6iOJNc5fri97tz5NhlBx+jXd0h9uhb3kkZXk432IXsDWTHjNzq5hvK2TdXbibHJfJOICHgZrMUv1kA0X573WO4rWUfUFnI+jpkCd9ryj5b4+3gcbZAn0H6D6H2zS9ngW+Gv8v2AKCFySBI3XxQrm0DaIg7kjkYFMnKjVfcwWHhUV5wLYr8O6kB176dQrVQHAx28ST3e8/6hTHmi+8nWWz9qe4HE==2xax-----END PGP SIGNATURE-----
L
L
Ludovic Courtès wrote on 6 Dec 2019 13:16
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)(address . 38478@debbugs.gnu.org)
87a785abti.fsf@gnu.org
Hi!
zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis:
Toggle quote (2 lines)> Ludovic Courtès <ludo@gnu.org> writes:
[...]
Toggle quote (11 lines)>> 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 itnow that “guix deploy” is still very new.
Ludo’.
L
L
Ludovic Courtès wrote on 7 Dec 2019 01:04
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)(address . 38478@debbugs.gnu.org)
8736dx80h1.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:
Toggle quote (22 lines)> 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’.
?
Your comment

This issue is archived.

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