Re: [libvirt] [PATCH] phyp: ssh authentication with pub keys fixed
2009/11/7 Eduardo Otubo <otubo(a)linux.vnet.ibm.com>:
Matthias Bolte wrote:
>
> 2009/11/6 Eduardo Otubo <otubo(a)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