[PATCH 0/3] block: misc fixes & improvements for SSH block driver key fingerprints

[PATCH 0/3] block: misc fixes & improvements for SSH block driver key fingerprints

[Daniel P. Berrangé]

 * The docs were pointing people towards the obsolete and insecure
   MD5 fingerprint config instead of preferred sha256
 * The sha256 fingerprint handling wasn't wired up into the legacy
   CLI parsing code
 * Finger print check failures were hard to diagnose due to limited
   info reported on error.

Daniel P. Berrangé (3):
  block: better document SSH host key fingerprint checking
  block: support sha256 fingerprint with pre-blockdev options
  block: print the server key type and fingerprint on failure

 block/ssh.c                            | 42 +++++++++++++++++++++-----
 docs/system/qemu-block-drivers.rst.inc | 30 +++++++++++++++---
 2 files changed, 61 insertions(+), 11 deletions(-)

-- 
2.31.1

[PATCH 1/3] block: better document SSH host key fingerprint checking

[Daniel P. Berrangé]

The docs still illustrate host key fingerprint checking using the old
md5 hashes which are considered insecure and obsolete. Change it to
illustrate using a sha256 hash. Also show how to extract the hash
value from the known_hosts file.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/system/qemu-block-drivers.rst.inc | 30 ++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/docs/system/qemu-block-drivers.rst.inc b/docs/system/qemu-block-drivers.rst.inc
index 16225710eb..2aeeaf6361 100644
--- a/docs/system/qemu-block-drivers.rst.inc
+++ b/docs/system/qemu-block-drivers.rst.inc
@@ -778,10 +778,32 @@ The optional *HOST_KEY_CHECK* parameter controls how the remote
 host's key is checked.  The default is ``yes`` which means to use
 the local ``.ssh/known_hosts`` file.  Setting this to ``no``
 turns off known-hosts checking.  Or you can check that the host key
-matches a specific fingerprint:
-``host_key_check=md5:78:45:8e:14:57:4f:d5:45:83:0a:0e:f3:49:82:c9:c8``
-(``sha1:`` can also be used as a prefix, but note that OpenSSH
-tools only use MD5 to print fingerprints).
+matches a specific fingerprint. The fingerprint can be provided in
+``md5``, ``sha1``, or ``sha256`` format, however, it is strongly
+recommended to only use ``sha256``, since the other options are
+considered insecure by modern standards. The fingerprint value
+must be given as a hex encoded string::
+
+  host_key_check=sha256:04ce2ae89ff4295a6b9c4111640bdcb3297858ee55cb434d9dd88796e93aa795``
+
+The key string may optionally contain ":" separators between
+each pair of hex digits.
+
+The ``$HOME/.ssh/known_hosts`` file contains the base64 encoded
+host keys. These can be converted into the format needed for
+QEMU using a command such as::
+
+   $ for key in `grep 10.33.8.112 known_hosts | awk '{print $3}'`
+     do
+       echo $key | base64 -d | sha256sum
+     done
+     6c3aa525beda9dc83eadfbd7e5ba7d976ecb59575d1633c87cd06ed2ed6e366f  -
+     12214fd9ea5b408086f98ecccd9958609bd9ac7c0ea316734006bc7818b45dc8  -
+     d36420137bcbd101209ef70c3b15dc07362fbe0fa53c5b135eba6e6afa82f0ce  -
+
+Note that there can be multiple keys present per host, each with
+different key ciphers. Care is needed to pick the key fingerprint
+that matches the cipher QEMU will negotiate with the remote server.
 
 Currently authentication must be done using ssh-agent.  Other
 authentication methods may be supported in future.
-- 
2.31.1

[PATCH 2/3] block: support sha256 fingerprint with pre-blockdev options

[Daniel P. Berrangé]

When support for sha256 fingerprint checking was aded in

  commit bf783261f0aee6e81af3916bff7606d71ccdc153
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Tue Jun 22 12:51:56 2021 +0100

    block/ssh: add support for sha256 host key fingerprints

it was only made to work with -blockdev. Getting it working with
-drive requires some extra custom parsing.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/ssh.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/ssh.c b/block/ssh.c
index e0fbb4934b..fcc0ab765a 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -556,6 +556,11 @@ static bool ssh_process_legacy_options(QDict *output_opts,
             qdict_put_str(output_opts, "host-key-check.type", "sha1");
             qdict_put_str(output_opts, "host-key-check.hash",
                           &host_key_check[5]);
+        } else if (strncmp(host_key_check, "sha256:", 7) == 0) {
+            qdict_put_str(output_opts, "host-key-check.mode", "hash");
+            qdict_put_str(output_opts, "host-key-check.type", "sha256");
+            qdict_put_str(output_opts, "host-key-check.hash",
+                          &host_key_check[7]);
         } else if (strcmp(host_key_check, "yes") == 0) {
             qdict_put_str(output_opts, "host-key-check.mode", "known_hosts");
         } else {
-- 
2.31.1

[PATCH 3/3] block: print the server key type and fingerprint on failure

[Daniel P. Berrangé]

When validating the server key fingerprint fails, it is difficult for
the user to know what they got wrong. The fingerprint accepted by QEMU
is received in a different format than openssh displays. There can also
be keys for multiple different ciphers in known_hosts. It may not be
obvious which cipher QEMU will use and whether it will be the same
as openssh. Address this by printing the server key type and its
corresponding fingerprint in the format QEMU accepts.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/ssh.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index fcc0ab765a..967a2b971e 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -386,14 +386,28 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len,
     return *host_key_check - '\0';
 }
 
+static char *format_fingerprint(const unsigned char *fingerprint, size_t len)
+{
+    static const char *hex = "0123456789abcdef";
+    char *ret = g_new0(char, (len * 2) + 1);
+    for (size_t i = 0; i < len; i++) {
+        ret[i * 2] = hex[((fingerprint[i] >> 4) & 0xf)];
+        ret[(i * 2) + 1] = hex[(fingerprint[i] & 0xf)];
+    }
+    ret[len * 2] = '\0';
+    return ret;
+}
+
 static int
 check_host_key_hash(BDRVSSHState *s, const char *hash,
-                    enum ssh_publickey_hash_type type, Error **errp)
+                    enum ssh_publickey_hash_type type, const char *typestr,
+                    Error **errp)
 {
     int r;
     ssh_key pubkey;
     unsigned char *server_hash;
     size_t server_hash_len;
+    const char *keytype;
 
     r = ssh_get_server_publickey(s->session, &pubkey);
     if (r != SSH_OK) {
@@ -401,6 +415,8 @@ check_host_key_hash(BDRVSSHState *s, const char *hash,
         return -EINVAL;
     }
 
+    keytype = ssh_key_type_to_char(ssh_key_type(pubkey));
+
     r = ssh_get_publickey_hash(pubkey, type, &server_hash, &server_hash_len);
     ssh_key_free(pubkey);
     if (r != 0) {
@@ -410,12 +426,16 @@ check_host_key_hash(BDRVSSHState *s, const char *hash,
     }
 
     r = compare_fingerprint(server_hash, server_hash_len, hash);
-    ssh_clean_pubkey_hash(&server_hash);
     if (r != 0) {
-        error_setg(errp, "remote host key does not match host_key_check '%s'",
-                   hash);
+        g_autofree char *server_fp = format_fingerprint(server_hash,
+                                                        server_hash_len);
+        error_setg(errp, "remote host %s key fingerprint '%s:%s' "
+                   "does not match host_key_check '%s:%s'",
+                   keytype, typestr, server_fp, typestr, hash);
+        ssh_clean_pubkey_hash(&server_hash);
         return -EPERM;
     }
+    ssh_clean_pubkey_hash(&server_hash);
 
     return 0;
 }
@@ -436,13 +456,16 @@ static int check_host_key(BDRVSSHState *s, SshHostKeyCheck *hkc, Error **errp)
     case SSH_HOST_KEY_CHECK_MODE_HASH:
         if (hkc->u.hash.type == SSH_HOST_KEY_CHECK_HASH_TYPE_MD5) {
             return check_host_key_hash(s, hkc->u.hash.hash,
-                                       SSH_PUBLICKEY_HASH_MD5, errp);
+                                       SSH_PUBLICKEY_HASH_MD5, "md5",
+                                       errp);
         } else if (hkc->u.hash.type == SSH_HOST_KEY_CHECK_HASH_TYPE_SHA1) {
             return check_host_key_hash(s, hkc->u.hash.hash,
-                                       SSH_PUBLICKEY_HASH_SHA1, errp);
+                                       SSH_PUBLICKEY_HASH_SHA1, "sha1",
+                                       errp);
         } else if (hkc->u.hash.type == SSH_HOST_KEY_CHECK_HASH_TYPE_SHA256) {
             return check_host_key_hash(s, hkc->u.hash.hash,
-                                       SSH_PUBLICKEY_HASH_SHA256, errp);
+                                       SSH_PUBLICKEY_HASH_SHA256, "sha256",
+                                       errp);
         }
         g_assert_not_reached();
         break;
-- 
2.31.1

Re: [PATCH 1/3] block: better document SSH host key fingerprint checking

[Hanna Reitz]

On 18.11.21 15:35, Daniel P. Berrangé wrote:
> The docs still illustrate host key fingerprint checking using the old
> md5 hashes which are considered insecure and obsolete. Change it to
> illustrate using a sha256 hash. Also show how to extract the hash
> value from the known_hosts file.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   docs/system/qemu-block-drivers.rst.inc | 30 ++++++++++++++++++++++----
>   1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/docs/system/qemu-block-drivers.rst.inc b/docs/system/qemu-block-drivers.rst.inc
> index 16225710eb..2aeeaf6361 100644
> --- a/docs/system/qemu-block-drivers.rst.inc
> +++ b/docs/system/qemu-block-drivers.rst.inc
> @@ -778,10 +778,32 @@ The optional *HOST_KEY_CHECK* parameter controls how the remote
>   host's key is checked.  The default is ``yes`` which means to use
>   the local ``.ssh/known_hosts`` file.  Setting this to ``no``
>   turns off known-hosts checking.  Or you can check that the host key
> -matches a specific fingerprint:
> -``host_key_check=md5:78:45:8e:14:57:4f:d5:45:83:0a:0e:f3:49:82:c9:c8``
> -(``sha1:`` can also be used as a prefix, but note that OpenSSH
> -tools only use MD5 to print fingerprints).
> +matches a specific fingerprint. The fingerprint can be provided in
> +``md5``, ``sha1``, or ``sha256`` format, however, it is strongly
> +recommended to only use ``sha256``, since the other options are
> +considered insecure by modern standards. The fingerprint value
> +must be given as a hex encoded string::
> +
> +  host_key_check=sha256:04ce2ae89ff4295a6b9c4111640bdcb3297858ee55cb434d9dd88796e93aa795``

I think the backticks at the end of this line should be dropped.

With that done:

Reviewed-by: Hanna Reitz <hreitz@redhat.com>

> +
> +The key string may optionally contain ":" separators between
> +each pair of hex digits.
> +
> +The ``$HOME/.ssh/known_hosts`` file contains the base64 encoded
> +host keys. These can be converted into the format needed for
> +QEMU using a command such as::
> +
> +   $ for key in `grep 10.33.8.112 known_hosts | awk '{print $3}'`
> +     do
> +       echo $key | base64 -d | sha256sum
> +     done
> +     6c3aa525beda9dc83eadfbd7e5ba7d976ecb59575d1633c87cd06ed2ed6e366f  -
> +     12214fd9ea5b408086f98ecccd9958609bd9ac7c0ea316734006bc7818b45dc8  -
> +     d36420137bcbd101209ef70c3b15dc07362fbe0fa53c5b135eba6e6afa82f0ce  -
> +
> +Note that there can be multiple keys present per host, each with
> +different key ciphers. Care is needed to pick the key fingerprint
> +that matches the cipher QEMU will negotiate with the remote server.
>   
>   Currently authentication must be done using ssh-agent.  Other
>   authentication methods may be supported in future.

Re: [PATCH 2/3] block: support sha256 fingerprint with pre-blockdev options

[Hanna Reitz]

On 18.11.21 15:35, Daniel P. Berrangé wrote:
> When support for sha256 fingerprint checking was aded in
>
>    commit bf783261f0aee6e81af3916bff7606d71ccdc153
>    Author: Daniel P. Berrangé <berrange@redhat.com>
>    Date:   Tue Jun 22 12:51:56 2021 +0100
>
>      block/ssh: add support for sha256 host key fingerprints
>
> it was only made to work with -blockdev. Getting it working with
> -drive requires some extra custom parsing.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   block/ssh.c | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Hanna Reitz <hreitz@redhat.com>

Re: [PATCH 3/3] block: print the server key type and fingerprint on failure

[Hanna Reitz]

On 18.11.21 15:35, Daniel P. Berrangé wrote:
> When validating the server key fingerprint fails, it is difficult for
> the user to know what they got wrong. The fingerprint accepted by QEMU
> is received in a different format than openssh displays. There can also
> be keys for multiple different ciphers in known_hosts. It may not be
> obvious which cipher QEMU will use and whether it will be the same
> as openssh. Address this by printing the server key type and its
> corresponding fingerprint in the format QEMU accepts.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   block/ssh.c | 37 ++++++++++++++++++++++++++++++-------
>   1 file changed, 30 insertions(+), 7 deletions(-)

Nice!

Reviewed-by: Hanna Reitz <hreitz@redhat.com>

> diff --git a/block/ssh.c b/block/ssh.c
> index fcc0ab765a..967a2b971e 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -386,14 +386,28 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len,
>       return *host_key_check - '\0';
>   }
>   
> +static char *format_fingerprint(const unsigned char *fingerprint, size_t len)
> +{
> +    static const char *hex = "0123456789abcdef";
> +    char *ret = g_new0(char, (len * 2) + 1);
> +    for (size_t i = 0; i < len; i++) {
> +        ret[i * 2] = hex[((fingerprint[i] >> 4) & 0xf)];
> +        ret[(i * 2) + 1] = hex[(fingerprint[i] & 0xf)];

(I would have found an sn?printf() solution a bit simpler here
(snprintf(&ret[i * 2], 2, "%02x", fingerprint[i])),
but now you already wrote the code, so...)

> +    }
> +    ret[len * 2] = '\0';
> +    return ret;
> +}

Re: [PATCH 3/3] block: print the server key type and fingerprint on failure

[Philippe Mathieu-Daudé]

On 11/18/21 15:35, Daniel P. Berrangé wrote:
> When validating the server key fingerprint fails, it is difficult for
> the user to know what they got wrong. The fingerprint accepted by QEMU
> is received in a different format than openssh displays. There can also
> be keys for multiple different ciphers in known_hosts. It may not be
> obvious which cipher QEMU will use and whether it will be the same
> as openssh. Address this by printing the server key type and its

"OpenSSH"? (twice)

> corresponding fingerprint in the format QEMU accepts.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  block/ssh.c | 37 ++++++++++++++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>