[libvirt] [PATCH 0/2] rpc: Fix very old regression in libssh2 code
Peter Krempa (2): rpc: libssh2: Add more debugging info rpc: libssh2: Fix regression in ssh host key verification src/rpc/virnetsshsession.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) -- 2.4.5
--- src/rpc/virnetsshsession.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 7f47b29..becdf6e 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -524,6 +524,8 @@ virNetSSHAuthenticateAgent(virNetSSHSessionPtr sess, int ret; char *errmsg; + VIR_DEBUG("sess=%p", sess); + if (libssh2_agent_connect(sess->agent) < 0) { virReportError(VIR_ERR_SSH, "%s", _("Failed to connect to ssh agent")); @@ -595,6 +597,8 @@ virNetSSHAuthenticatePrivkey(virNetSSHSessionPtr sess, int ret; char *tmp; + VIR_DEBUG("sess=%p", sess); + /* try open the key with no password */ if ((ret = libssh2_userauth_publickey_fromfile(sess->session, priv->username, @@ -697,6 +701,8 @@ virNetSSHAuthenticatePassword(virNetSSHSessionPtr sess, int ret = -1; int rc; + VIR_DEBUG("sess=%p", sess); + if (priv->password) { /* tunelled password authentication */ if ((ret = libssh2_userauth_password(sess->session, @@ -770,6 +776,8 @@ virNetSSHAuthenticateKeyboardInteractive(virNetSSHSessionPtr sess, char *errmsg; int ret; + VIR_DEBUG("sess=%p", sess); + if (!sess->cred || !sess->cred->cb) { virReportError(VIR_ERR_SSH, "%s", _("Can't perform keyboard-interactive authentication: " @@ -837,6 +845,8 @@ virNetSSHAuthenticate(virNetSSHSessionPtr sess) size_t i; int ret; + VIR_DEBUG("sess=%p", sess); + if (!sess->nauths) { virReportError(VIR_ERR_SSH, "%s", _("No authentication methods and credentials " -- 2.4.5
Commit 792f81a40e caused a regression in the libssh2 host key verification code by changing the variable type of 'i' to unsigned. Since one of the loops used -1 as a special value if the asking callback was found the conversion made a subsequent test always fail. The bug was stealth enough to pass review, compilers and coverity. Refactor the condition to avoid problems. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1047861 --- src/rpc/virnetsshsession.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index becdf6e..406a831 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -344,16 +344,14 @@ virNetSSHCheckHostKey(virNetSSHSessionPtr sess) memset(&askKey, 0, sizeof(virConnectCredential)); for (i = 0; i < sess->cred->ncredtype; i++) { - if (sess->cred->credtype[i] == VIR_CRED_ECHOPROMPT) { - i = -1; + if (sess->cred->credtype[i] == VIR_CRED_ECHOPROMPT) break; - } } - if (i > 0) { + if (i == sess->cred->ncredtype) { virReportError(VIR_ERR_SSH, "%s", - _("no suitable method to retrieve " - "authentication credentials")); + _("no suitable callback for host key " + "verification")); return -1; } -- 2.4.5
On 10/02/2015 09:54 AM, Peter Krempa wrote:
Peter Krempa (2): rpc: libssh2: Add more debugging info rpc: libssh2: Fix regression in ssh host key verification
src/rpc/virnetsshsession.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
ACK series - that's a really interesting catch, but not the only place needing a cleanup... There's a few places where "size_t i" gets i = -1 and a few with "j = -1". John
On Fri, Oct 02, 2015 at 12:18:34 -0400, John Ferlan wrote:
On 10/02/2015 09:54 AM, Peter Krempa wrote:
Peter Krempa (2): rpc: libssh2: Add more debugging info rpc: libssh2: Fix regression in ssh host key verification
src/rpc/virnetsshsession.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
ACK series - that's a really interesting catch, but not the only place needing a cleanup... There's a few places where "size_t i" gets i = -1 and a few with "j = -1".
I've pushed this one since I was about to fix the libssh2 driver rather than introspecting all the code. Peter
participants (2)
-
John Ferlan -
Peter Krempa