[libvirt] [PATCH 0/2] Fixes for libssh2

Hi all, Here are two fixes to get libssh2 authentication working. Cédric Bosdonnat (2): Fix test wanting a negative size_t Fix handling keyboard-interactive callbacks for libssh2 src/rpc/virnetsshsession.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 1.8.4.5

--- src/rpc/virnetsshsession.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 7f47b29..57119f9 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -303,6 +303,7 @@ virNetSSHCheckHostKey(virNetSSHSessionPtr sess) virConnectCredential askKey; struct libssh2_knownhost *knownHostEntry = NULL; size_t i; + bool hasEchoPrompt = false; char *hostnameStr = NULL; if (sess->hostKeyVerify == VIR_NET_SSH_HOSTKEY_VERIFY_IGNORE) @@ -345,12 +346,12 @@ virNetSSHCheckHostKey(virNetSSHSessionPtr sess) for (i = 0; i < sess->cred->ncredtype; i++) { if (sess->cred->credtype[i] == VIR_CRED_ECHOPROMPT) { - i = -1; + hasEchoPrompt = true; break; } } - if (i > 0) { + if (!hasEchoPrompt) { virReportError(VIR_ERR_SSH, "%s", _("no suitable method to retrieve " "authentication credentials")); -- 1.8.4.5

On 10/25/2014 07:16 PM, Cédric Bosdonnat wrote:
--- src/rpc/virnetsshsession.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
Looking through some older patches I had marked as get to some day :-) ACK John
diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 7f47b29..57119f9 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -303,6 +303,7 @@ virNetSSHCheckHostKey(virNetSSHSessionPtr sess) virConnectCredential askKey; struct libssh2_knownhost *knownHostEntry = NULL; size_t i; + bool hasEchoPrompt = false; char *hostnameStr = NULL;
if (sess->hostKeyVerify == VIR_NET_SSH_HOSTKEY_VERIFY_IGNORE) @@ -345,12 +346,12 @@ virNetSSHCheckHostKey(virNetSSHSessionPtr sess)
for (i = 0; i < sess->cred->ncredtype; i++) { if (sess->cred->credtype[i] == VIR_CRED_ECHOPROMPT) { - i = -1; + hasEchoPrompt = true; break; } }
- if (i > 0) { + if (!hasEchoPrompt) { virReportError(VIR_ERR_SSH, "%s", _("no suitable method to retrieve " "authentication credentials"));

SSHD calls the KI callback with no prompt after all prompts have been issued. Just ignore those callbacks to avoid libvirt-java (and possibly others) to crash while accessing invalid pointers. --- src/rpc/virnetsshsession.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 57119f9..e9516b8 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -217,6 +217,10 @@ virNetSSHKbIntCb(const char *name ATTRIBUTE_UNUSED, priv->authCbErr = VIR_NET_SSH_AUTHCB_OK; + /* After all prompts, sshd calls us with 0 prompts: just ignore it */ + if (num_prompts == 0) + return; + /* find credential type for asking passwords */ for (i = 0; i < priv->cred->ncredtype; i++) { if (priv->cred->credtype[i] == VIR_CRED_PASSPHRASE || -- 1.8.4.5

On 10/25/2014 07:16 PM, Cédric Bosdonnat wrote:
SSHD calls the KI callback with no prompt after all prompts have been issued. Just ignore those callbacks to avoid libvirt-java (and possibly others) to crash while accessing invalid pointers. --- src/rpc/virnetsshsession.c | 4 ++++ 1 file changed, 4 insertions(+)
This one I'm a bit less positive on - it seems you would be doing the right thing if the input num_prompts == 0; however, when I read the man page (libssh2_userauth_keyboard_interactive_ex(3)) of what calls this I see: response_callback - As authentication proceeds, the host issues several (1 or more) challenges and requires responses. This callback will be called at this moment. The callback is responsible to obtain responses for the challenges, fill the provided data structure and then return control. Responses will be sent to the host. String values will be free(3)ed by the library. The callback prototype must match this: void response(const char *name, int name_len, const char *instruction, int instruction_len, int num_prompts, const LIBSSH2_USERAUTH_KBDINT_PROMPT *prompts, LIBSSH2_USERAUTH_KBDINT_RESPONSE *responses, void **abstract); This says to me the response_callback shouldn't be called unless there's 1 or more challenges. So are we fixing the root cause of the problem or a side effect of someone else's bug?
From a libvirt POV sure this works, but is it the right fix? Perhaps someone with more libssh2 knowledge can pipe in.
John
diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 57119f9..e9516b8 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -217,6 +217,10 @@ virNetSSHKbIntCb(const char *name ATTRIBUTE_UNUSED,
priv->authCbErr = VIR_NET_SSH_AUTHCB_OK;
+ /* After all prompts, sshd calls us with 0 prompts: just ignore it */ + if (num_prompts == 0) + return; + /* find credential type for asking passwords */ for (i = 0; i < priv->cred->ncredtype; i++) { if (priv->cred->credtype[i] == VIR_CRED_PASSPHRASE ||
participants (2)
-
Cédric Bosdonnat
-
John Ferlan