Re: [PATCH] block/ssh: Bump minimum libssh version to 0.8.7

[Richard W.M. Jones]

On Wed, May 19, 2021 at 05:58:59PM +0200, Thomas Huth wrote:
> It has been over two years since RHEL-8 was released, and thus per the
> platform build policy, we no longer need to support RHEL-7 as a build
> target. So from the RHEL-7 perspective, we do not have to support
> libssh v0.7 anymore now.

Not an objection, just an FYI: RHEL 7 has libssh-0.7.1-7.el7.x86_64

nbdkit-ssh-plugin settled on only supporting libssh >= 0.8.0, mainly
because we require knownhosts support which seems a fairly fundamental
requirement for security.

> Let's look at the versions from other distributions and operating
> systems - according to repology.org, current shipping versions are:
> 
>              RHEL-8: 0.9.4
>       Debian Buster: 0.8.7
>  openSUSE Leap 15.2: 0.8.7
>    Ubuntu LTS 18.04: 0.8.0 *
>    Ubuntu LTS 20.04: 0.9.3
>             FreeBSD: 0.9.5
>           Fedora 33: 0.9.5
>           Fedora 34: 0.9.5
>             OpenBSD: 0.9.5
>      macOS HomeBrew: 0.9.5
>          HaikuPorts: 0.9.5
> 
> * The version of libssh in Ubuntu 18.04 claims to be 0.8.0 from the
> name of the package, but in reality it is a 0.7 patched up as a
> Frankenstein monster with patches from the 0.8 development branch.
> This gave us some headaches in the past already and so it never worked
> with QEMU. All attempts to get it supported have failed in the past,
> patches for QEMU have never been merged and a request to Ubuntu to
> fix it in their 18.04 distro has been ignored:
> 
>  https://bugs.launchpad.net/ubuntu/+source/libssh/+bug/1847514
> 
> Thus we really should ignore the libssh in Ubuntu 18.04 in QEMU, too.
> 
> Fix it by bumping the minimum libssh version to something that is
> greater than 0.8.0 now. Debian Buster and openSUSE Leap have the
> oldest version and so 0.8.7 is the new minimum.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  block/ssh.c | 59 -----------------------------------------------------
>  configure   | 19 +----------------
>  2 files changed, 1 insertion(+), 77 deletions(-)
> 
> diff --git a/block/ssh.c b/block/ssh.c
> index ebe3d8b631..b51a031620 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -277,7 +277,6 @@ 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;
> @@ -343,46 +342,6 @@ static int check_host_key_knownhosts(BDRVSSHState *s, 
> Error **errp)
>          error_setg(errp, "error while checking for known server (%d)", 
> state);
>          goto out;
>      }
> -#else /* !HAVE_LIBSSH_0_8 */
> -    int state;
> -
> -    state = ssh_is_server_known(s->session);
> -    trace_ssh_server_status(state);
> -
> -    switch (state) {
> -    case SSH_SERVER_KNOWN_OK:
> -        /* OK */
> -        trace_ssh_check_host_key_knownhosts();
> -        break;
> -    case SSH_SERVER_KNOWN_CHANGED:
> -        ret = -EINVAL;
> -        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;
> -        error_setg(errp,
> -                   "host key for this server not found, another type 
> exists");
> -        goto out;
> -    case SSH_SERVER_FILE_NOT_FOUND:
> -        ret = -ENOENT;
> -        error_setg(errp, "known_hosts file not found");
> -        goto out;
> -    case SSH_SERVER_NOT_KNOWN:
> -        ret = -EINVAL;
> -        error_setg(errp, "no host key was found in known_hosts");
> -        goto out;
> -    case SSH_SERVER_ERROR:
> -        ret = -EINVAL;
> -        error_setg(errp, "server error");
> -        goto out;
> -    default:
> -        ret = -EINVAL;
> -        error_setg(errp, "error while checking for known server (%d)", 
> state);
> -        goto out;
> -    }
> -#endif /* !HAVE_LIBSSH_0_8 */
>  
>      /* known_hosts checking successful. */
>      ret = 0;
> @@ -438,11 +397,7 @@ check_host_key_hash(BDRVSSHState *s, const char *hash,
>      unsigned char *server_hash;
>      size_t server_hash_len;
>  
> -#ifdef HAVE_LIBSSH_0_8
>      r = ssh_get_server_publickey(s->session, &pubkey);
> -#else
> -    r = ssh_get_publickey(s->session, &pubkey);
> -#endif
>      if (r != SSH_OK) {
>          session_error_setg(errp, s, "failed to read remote host key");
>          return -EINVAL;
> @@ -1233,8 +1188,6 @@ static void unsafe_flush_warning(BDRVSSHState *s, const 
> char *what)
>      }
>  }
>  
> -#ifdef HAVE_LIBSSH_0_8
> -
>  static coroutine_fn int ssh_flush(BDRVSSHState *s, BlockDriverState *bs)
>  {
>      int r;
> @@ -1271,18 +1224,6 @@ static coroutine_fn int ssh_co_flush(BlockDriverState 
> *bs)
>      return ret;
>  }
>  
> -#else /* !HAVE_LIBSSH_0_8 */
> -
> -static coroutine_fn int ssh_co_flush(BlockDriverState *bs)
> -{
> -    BDRVSSHState *s = bs->opaque;
> -
> -    unsafe_flush_warning(s, "libssh >= 0.8.0");
> -    return 0;
> -}
> -
> -#endif /* !HAVE_LIBSSH_0_8 */
> -
>  static int64_t ssh_getlength(BlockDriverState *bs)
>  {
>      BDRVSSHState *s = bs->opaque;
> diff --git a/configure b/configure
> index 879a8e8f17..bf1c740494 100755
> --- a/configure
> +++ b/configure
> @@ -3512,7 +3512,7 @@ fi
>  ##########################################
>  # libssh probe
>  if test "$libssh" != "no" ; then
> -  if $pkg_config --exists libssh; then
> +  if $pkg_config --exists "libssh >= 0.8.7"; then
>      libssh_cflags=$($pkg_config libssh --cflags)
>      libssh_libs=$($pkg_config libssh --libs)
>      libssh=yes
> @@ -3524,23 +3524,6 @@ if test "$libssh" != "no" ; then
>    fi
>  fi
>  
> -##########################################
> -# Check for libssh 0.8
> -# This is done like this instead of using the LIBSSH_VERSION_* and
> -# SSH_VERSION_* macros because some distributions in the past shipped
> -# snapshots of the future 0.8 from Git, and those snapshots did not
> -# have updated version numbers (still referring to 0.7.0).
> -
> -if test "$libssh" = "yes"; then
> -  cat > $TMPC <<EOF
> -#include <libssh/libssh.h>
> -int main(void) { return ssh_get_server_publickey(NULL, NULL); }
> -EOF
> -  if compile_prog "$libssh_cflags" "$libssh_libs"; then
> -    libssh_cflags="-DHAVE_LIBSSH_0_8 $libssh_cflags"
> -  fi
> -fi
> -
>  ##########################################
>  # linux-aio probe
>  
> -- 
> 2.27.0

The patch looks completely obvious and mechanical.

Also I applied it on top of qemu and tested it by doing some
“qemu-system-x86-64 -hda ssh://remote/fedora-33.img” commands and it
appears to work fine.  Therefore:

Acked-by: Richard W.M. Jones <rjones@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top