[libvirt] [PATCH 2/2] Fix handling keyboard-interactive callbacks for libssh2

[John Ferlan]

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 ||
>