Matthias Bolte wrote:
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.
Ok.
- "/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?
Talking with libssh2 guys, this feature is not exactly working well,
they said that it is possible to pass a random passphrase (or even NULL)
that it will authenticate using pub and pvt keys. So, I assumed this as
a hardcoded NULL just until they fix this function.
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.
Appearently, going further the man pages and tracing all the function
return points, I figured out that this function may also return
LIBSSH2_ERROR_SOCKET_NONE or LIBSSH2_ERROR_NONE for many reasons. As far
as I understand, LIBSSH2_ERROR_NONE is for a succesful pubkey
authentication, and LIBSSH2_ERROR_SOCKET_NONE is for a non succesful.
Adjusted all values for this if construction.
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.
In this case I explicit check for errors (LIBSSH2_ERROR_ALLOC,
LIBSSH2_ERROR_SOCKET_SEND and LIBSSH2_ERROR_SOCKET_TIMEOUT) before fallback.
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.
Ok. I am stating files now.
@@ -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.
Ok.
Thanks again :)
[]'s
--
Eduardo Otubo
Software Engineer
Linux Technology Center
IBM Systems & Technology Group
Mobile: +55 19 8135 0885
eotubo@xxxxxxxxxxxxxxxxxx
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index a92046a..94581b2 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -101,6 +101,12 @@ phypOpen(virConnectPtr conn,
return VIR_DRV_OPEN_ERROR;
}
+ if (conn->uri->user == NULL) {
+ PHYP_ERROR(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Missing username in phyp:// URI"));
+ return VIR_DRV_OPEN_ERROR;
+ }
+
if (VIR_ALLOC(phyp_driver) < 0) {
virReportOOMError(conn);
goto failure;
@@ -225,10 +231,26 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
const char *password = NULL;
int sock;
int rc;
-
struct addrinfo *ai = NULL, *cur;
struct addrinfo hints;
int ret;
+ char *pubkey = NULL;
+ char *pvtkey = NULL;
+ char *userhome = virGetUserDirectory(NULL, geteuid());
+ struct stat pvt_stat, pub_stat;
+
+ if (userhome == NULL)
+ goto err;
+
+ if (virAsprintf(&pubkey, "%s/.ssh/id_rsa.pub", userhome) < 0) {
+ virReportOOMError(conn);
+ goto err;
+ }
+
+ if (virAsprintf(&pvtkey, "%s/.ssh/id_rsa", userhome) < 0) {
+ virReportOOMError(conn);
+ goto err;
+ }
memset(&hints, 0, sizeof(hints));
hints.ai_flags = AI_ADDRCONFIG | AI_NUMERICSERV;
@@ -280,15 +302,19 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
}
/* Trying authentication by pubkey */
+ if (stat(pvtkey, &pvt_stat) || stat(pubkey, &pub_stat))
+ goto err;
+
while ((rc =
libssh2_userauth_publickey_fromfile(session, username,
- "/home/user/"
- ".ssh/id_rsa.pub",
- "/home/user/"
- ".ssh/id_rsa",
- password)) ==
+ pubkey,
+ pvtkey,
+ NULL)) ==
LIBSSH2_ERROR_EAGAIN) ;
- if (rc) {
+
+ if (rc == LIBSSH2_ERROR_NONE
+ || rc == LIBSSH2_ERROR_PUBLICKEY_UNRECOGNIZED
+ || rc == LIBSSH2_ERROR_PUBLICKEY_UNVERIFIED) {
int i;
int hasPassphrase = 0;
@@ -341,15 +367,29 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
goto disconnect;
} else
goto exit;
+
+ } else if (rc == LIBSSH2_ERROR_NONE) {
+ goto exit;
+
+ } else if (rc == LIBSSH2_ERROR_ALLOC || rc == LIBSSH2_ERROR_SOCKET_SEND
+ || rc == LIBSSH2_ERROR_SOCKET_TIMEOUT) {
+ goto err;
}
+
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);
+ VIR_FREE(pvtkey);
VIR_FREE(password);
return session;
}
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list