Re: [Qemu-devel] [PULL 0/8] Block patches

[Pino Toscano <ptoscano@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2519 bytes --]

On Monday, 24 June 2019 14:20:11 CEST Max Reitz wrote:
> On 23.06.19 19:18, Peter Maydell wrote:
> > On Fri, 21 Jun 2019 at 14:23, Max Reitz <mreitz@redhat.com> wrote:
> >>
> >> The following changes since commit 33d609990621dea6c7d056c86f707b8811320ac1:
> >>
> >>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-06-18 17:00:52 +0100)
> >>
> >> are available in the Git repository at:
> >>
> >>   https://github.com/XanClic/qemu.git tags/pull-block-2019-06-21
> >>
> >> for you to fetch changes up to e2a76186f7948b8b75d1b2b52638de7c2f7f7472:
> >>
> >>   iotests: Fix 205 for concurrent runs (2019-06-21 14:40:28 +0200)
> >>
> >> ----------------------------------------------------------------
> >> Block patches:
> >> - The SSH block driver now uses libssh instead of libssh2
> >> - The VMDK block driver gets read-only support for the seSparse
> >>   subformat
> >> - Various fixes
> >>
> > 
> > Hi; this failed to build on my s390 box:
> > 
> > /home/linux1/qemu/block/ssh.c: In function ‘check_host_key_knownhosts’:
> > /home/linux1/qemu/block/ssh.c:367:27: error: implicit declaration of
> > function ‘ssh_get_fingerprint_hash’
> > [-Werror=implicit-function-declaration]
> >              fingerprint = ssh_get_fingerprint_hash(SSH_PUBLICKEY_HASH_SHA1,
> >                            ^
> > /home/linux1/qemu/block/ssh.c:367:13: error: nested extern declaration
> > of ‘ssh_get_fingerprint_hash’ [-Werror=nested-externs]
> >              fingerprint = ssh_get_fingerprint_hash(SSH_PUBLICKEY_HASH_SHA1,
> >              ^
> > /home/linux1/qemu/block/ssh.c:367:25: error: assignment makes pointer
> > from integer without a cast [-Werror=int-conversion]
> >              fingerprint = ssh_get_fingerprint_hash(SSH_PUBLICKEY_HASH_SHA1,
> >                          ^
> > 
> > It looks like that function was introduced in libssh 0.8.3, and this box
> > has 0.6.3. (configure has correctly not defined HAVE_LIBSSH_0_8
> > but this usage is inside a bit of code that's compiled even when
> > that is not defined.)

Oops, sorry, I did not test the latest versions with that old libssh.

> Pino, would you be OK with dropping that piece of code for pre-0.8 and
> just replacing it with the else-error_setg()?

Some the variables in check_host_key_knownhosts must be moved within
the HAVE_LIBSSH_0_8 block now; attached fixup patch, please squash with
my patch (I can submit a v12, if needed/wanted).

-- 
Pino Toscano

[-- Attachment #1.2: fixup.diff --]
[-- Type: text/x-patch, Size: 2209 bytes --]

diff --git a/block/ssh.c b/block/ssh.c
index 048d0cc924..501933b855 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -277,14 +277,14 @@ static void ssh_parse_filename(const char *filename, QDict *options,
 static int check_host_key_knownhosts(BDRVSSHState *s, Error **errp)
 {
     int ret;
+#ifdef HAVE_LIBSSH_0_8
+    enum ssh_known_hosts_e state;
     int r;
     ssh_key pubkey;
     enum ssh_keytypes_e pubkey_type;
     unsigned char *server_hash = NULL;
     size_t server_hash_len;
     char *fingerprint = NULL;
-#ifdef HAVE_LIBSSH_0_8
-    enum ssh_known_hosts_e state;
 
     state = ssh_session_is_known_server(s->session);
     trace_ssh_server_status(state);
@@ -356,30 +356,9 @@ static int check_host_key_knownhosts(BDRVSSHState *s, Error **errp)
         break;
     case SSH_SERVER_KNOWN_CHANGED:
         ret = -EINVAL;
-        r = ssh_get_publickey(s->session, &pubkey);
-        if (r == 0) {
-            r = ssh_get_publickey_hash(pubkey, SSH_PUBLICKEY_HASH_SHA1,
-                                       &server_hash, &server_hash_len);
-            pubkey_type = ssh_key_type(pubkey);
-            ssh_key_free(pubkey);
-        }
-        if (r == 0) {
-            fingerprint = ssh_get_fingerprint_hash(SSH_PUBLICKEY_HASH_SHA1,
-                                                   server_hash,
-                                                   server_hash_len);
-            ssh_clean_pubkey_hash(&server_hash);
-        }
-        if (fingerprint) {
-            error_setg(errp,
-                       "host key (%s key with fingerprint %s) does not match "
-                       "the one in known_hosts; this may be a possible attack",
-                       ssh_key_type_to_char(pubkey_type), fingerprint);
-            ssh_string_free_char(fingerprint);
-        } else  {
-            error_setg(errp,
-                       "host key does not match the one in known_hosts; this "
-                       "may be a possible attack");
-        }
+        error_setg(errp,
+                   "host key does not match the one in known_hosts; this "
+                   "may be a possible attack");
         goto out;
     case SSH_SERVER_FOUND_OTHER:
         ret = -EINVAL;

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]