From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47243) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wkad1-0006o1-Kx for qemu-devel@nongnu.org; Wed, 14 May 2014 10:58:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wkacw-0007WQ-2z for qemu-devel@nongnu.org; Wed, 14 May 2014 10:57:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47425) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wkacv-0007W8-R3 for qemu-devel@nongnu.org; Wed, 14 May 2014 10:57:50 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4EEvjgk013380 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 14 May 2014 10:57:48 -0400 Date: Wed, 14 May 2014 15:57:44 +0100 From: "Richard W.M. Jones" Message-ID: <20140514145744.GU1302@redhat.com> References: <1399996972-23429-1-git-send-email-armbru@redhat.com> <1399996972-23429-7-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1399996972-23429-7-git-send-email-armbru@redhat.com> Subject: Re: [Qemu-devel] [PATCH 06/18] block/ssh: Propagate errors through check_host_key() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com On Tue, May 13, 2014 at 06:02:40PM +0200, Markus Armbruster wrote: > Cc: "Richard W.M. Jones" > Signed-off-by: Markus Armbruster Reviewed-by: Richard W.M. Jones > block/ssh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 49 insertions(+), 19 deletions(-) > > diff --git a/block/ssh.c b/block/ssh.c > index e38d232..1df1946 100644 > --- a/block/ssh.c > +++ b/block/ssh.c > @@ -106,6 +106,31 @@ static void ssh_state_free(BDRVSSHState *s) > } > } > > +static void GCC_FMT_ATTR(3, 4) > +session_error_setg(Error **errp, BDRVSSHState *s, const char *fs, ...) > +{ > + va_list args; > + char *msg; > + > + va_start(args, fs); > + msg = g_strdup_vprintf(fs, args); > + va_end(args); > + > + if (s->session) { > + char *ssh_err; > + int ssh_err_code; > + > + /* This is not an errno. See . */ > + ssh_err_code = libssh2_session_last_error(s->session, > + &ssh_err, NULL, 0); > + error_setg(errp, "%s: %s (libssh2 error code: %d)", > + msg, ssh_err, ssh_err_code); > + } else { > + error_setg(errp, "%s", msg); > + } > + g_free(msg); > +} > + > /* Wrappers around error_report which make sure to dump as much > * information from libssh2 as possible. > */ > @@ -242,7 +267,7 @@ static void ssh_parse_filename(const char *filename, QDict *options, > } > > static int check_host_key_knownhosts(BDRVSSHState *s, > - const char *host, int port) > + const char *host, int port, Error **errp) > { > const char *home; > char *knh_file = NULL; > @@ -256,14 +281,15 @@ static int check_host_key_knownhosts(BDRVSSHState *s, > hostkey = libssh2_session_hostkey(s->session, &len, &type); > if (!hostkey) { > ret = -EINVAL; > - session_error_report(s, "failed to read remote host key"); > + session_error_setg(errp, s, "failed to read remote host key"); > goto out; > } > > knh = libssh2_knownhost_init(s->session); > if (!knh) { > ret = -EINVAL; > - session_error_report(s, "failed to initialize known hosts support"); > + session_error_setg(errp, s, > + "failed to initialize known hosts support"); > goto out; > } > > @@ -288,21 +314,23 @@ static int check_host_key_knownhosts(BDRVSSHState *s, > break; > case LIBSSH2_KNOWNHOST_CHECK_MISMATCH: > ret = -EINVAL; > - session_error_report(s, "host key does not match the one in known_hosts (found key %s)", > - found->key); > + session_error_setg(errp, s, > + "host key does not match the one in known_hosts" > + " (found key %s)", found->key); > goto out; > case LIBSSH2_KNOWNHOST_CHECK_NOTFOUND: > ret = -EINVAL; > - session_error_report(s, "no host key was found in known_hosts"); > + session_error_setg(errp, s, "no host key was found in known_hosts"); > goto out; > case LIBSSH2_KNOWNHOST_CHECK_FAILURE: > ret = -EINVAL; > - session_error_report(s, "failure matching the host key with known_hosts"); > + session_error_setg(errp, s, > + "failure matching the host key with known_hosts"); > goto out; > default: > ret = -EINVAL; > - session_error_report(s, "unknown error matching the host key with known_hosts (%d)", > - r); > + session_error_setg(errp, s, "unknown error matching the host key" > + " with known_hosts (%d)", r); > goto out; > } > > @@ -357,20 +385,20 @@ static int compare_fingerprint(const unsigned char *fingerprint, size_t len, > > static int > check_host_key_hash(BDRVSSHState *s, const char *hash, > - int hash_type, size_t fingerprint_len) > + int hash_type, size_t fingerprint_len, Error **errp) > { > const char *fingerprint; > > fingerprint = libssh2_hostkey_hash(s->session, hash_type); > if (!fingerprint) { > - session_error_report(s, "failed to read remote host key"); > + session_error_setg(errp, s, "failed to read remote host key"); > return -EINVAL; > } > > if(compare_fingerprint((unsigned char *) fingerprint, fingerprint_len, > hash) != 0) { > - error_report("remote host key does not match host_key_check '%s'", > - hash); > + error_setg(errp, "remote host key does not match host_key_check '%s'", > + hash); > return -EPERM; > } > > @@ -378,7 +406,7 @@ check_host_key_hash(BDRVSSHState *s, const char *hash, > } > > static int check_host_key(BDRVSSHState *s, const char *host, int port, > - const char *host_key_check) > + const char *host_key_check, Error **errp) > { > /* host_key_check=no */ > if (strcmp(host_key_check, "no") == 0) { > @@ -388,21 +416,21 @@ static int check_host_key(BDRVSSHState *s, const char *host, int port, > /* host_key_check=md5:xx:yy:zz:... */ > if (strncmp(host_key_check, "md5:", 4) == 0) { > return check_host_key_hash(s, &host_key_check[4], > - LIBSSH2_HOSTKEY_HASH_MD5, 16); > + LIBSSH2_HOSTKEY_HASH_MD5, 16, errp); > } > > /* host_key_check=sha1:xx:yy:zz:... */ > if (strncmp(host_key_check, "sha1:", 5) == 0) { > return check_host_key_hash(s, &host_key_check[5], > - LIBSSH2_HOSTKEY_HASH_SHA1, 20); > + LIBSSH2_HOSTKEY_HASH_SHA1, 20, errp); > } > > /* host_key_check=yes */ > if (strcmp(host_key_check, "yes") == 0) { > - return check_host_key_knownhosts(s, host, port); > + return check_host_key_knownhosts(s, host, port, errp); > } > > - error_report("unknown host_key_check setting (%s)", host_key_check); > + error_setg(errp, "unknown host_key_check setting (%s)", host_key_check); > return -EINVAL; > } > > @@ -541,8 +569,10 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options, > } > > /* Check the remote host's key against known_hosts. */ > - ret = check_host_key(s, host, port, host_key_check); > + ret = check_host_key(s, host, port, host_key_check, &err); > if (ret < 0) { > + qerror_report_err(err); > + error_free(err); > goto err; > } > > -- > 1.8.1.4 -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/