[libvirt] [PATCH] phyp: ssh authentication with pub keys fixed
Matthias Bolte
matthias.bolte at googlemail.com
Sat Nov 7 22:25:57 UTC 2009
2009/11/7 Eduardo Otubo <otubo at linux.vnet.ibm.com>:
> Matthias Bolte wrote:
>>
>> 2009/11/6 Eduardo Otubo <otubo at linux.vnet.ibm.com>:
>>>
>>> + char *pubkey = NULL;
>>> + char *pvtkey = NULL;
>>> +
>>> + if (virAsprintf(&pubkey, "%s/.ssh/id_rsa.pub", getenv("HOME")) < 0)
>>> {
>>> + virReportOOMError(conn);
>>> + goto err;
>>> + }
>>> +
>>> + if (virAsprintf(&pvtkey, "%s/.ssh/id_rsa", getenv("HOME")) < 0) {
>>> + virReportOOMError(conn);
>>> + goto err;
>>> + }
>>
>> You should use virGetUserDirectory() instead of getenv("HOME"):
>>
>> char *userdir = virGetUserDirectory(NULL, geteuid());
>>
>> if (userdir == NULL)
>> goto err;
>>
>> Matthias
>
> Here it is.
> Thanks again.
>
> []'s
You included the old and the new version of this patch in the attached file.
> diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
> index a92046a..f96d2d6 100644
> --- a/src/phyp/phyp_driver.c
> +++ b/src/phyp/phyp_driver.c
[...]
> @@ -282,10 +297,8 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
> /* Trying authentication by pubkey */
> while ((rc =
> libssh2_userauth_publickey_fromfile(session, username,
You assign conn->uri->user to username and use it without checking for
NULL. You should either check conn->uri->user for NULL in phypOpen(),
as you do it for conn->uri->server and conn->uri->path, and return
VIR_DRV_OPEN_ERROR if its NULL or request a username via the auth
callback if conn->uri->user is NULL.
> - "/home/user/"
> - ".ssh/id_rsa.pub",
> - "/home/user/"
> - ".ssh/id_rsa",
> + pubkey,
> + pvtkey,
> password)) ==
The password (actually the passphrase) is NULL at this point. Is this
really working?
> LIBSSH2_ERROR_EAGAIN) ;
> if (rc) {
So you fallback to username/password authentication if keyfile
authentication failed (rc != 0). According to the
libssh2_userauth_publickey_fromfile manpage it may return this error
codes:
LIBSSH2_ERROR_ALLOC - An internal memory allocation call failed.
LIBSSH2_ERROR_SOCKET_SEND - Unable to send data on socket.
LIBSSH2_ERROR_SOCKET_TIMEOUT
LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED - The username/public key
combination was invalid.
LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED - The username/public key
combination was invalid, or the signature for the supplied public key
was invalid.
IMHO its not useful to fallback to username/password authentication
for the first three possible errors, only if a keyfile related error
occurs like the last two.
I wonder which error code will be returned if one or both keyfiles
don't exist. Maybe you should check if both keyfiles exist before
calling libssh2_userauth_publickey_fromfile() and fallback to
username/password authentication if one or both are missing.
> @@ -341,15 +354,22 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
> goto disconnect;
> } else
> goto exit;
> + } else {
> + goto exit;
> }
> disconnect:
> libssh2_session_disconnect(session, "Disconnecting...");
> libssh2_session_free(session);
> err:
> + VIR_FREE(userhome);
> + VIR_FREE(pubkey);
> + VIR_FREE(pvtkey);
> VIR_FREE(password);
> return NULL;
>
> exit:
> + VIR_FREE(userhome);
VIR_FREE(pubkey) is missing here, it's there in the first version of this patch.
> + VIR_FREE(pvtkey);
> VIR_FREE(password);
> return session;
> }
Matthias
More information about the libvir-list
mailing list