On 06/27/13 14:31, David Maciejak wrote:
Hi,
I was discussing with Daniel about the best way to pass the ssh password
when using such kind of uri:
'xen+libssh2://root@192.168.0.10?sshauth=password
<http://root@192.168.0.10?sshauth=password>'
As it seems passing the password in the uri is not a good option, maybe we
can grab it from auth conf ? it seems it's not the case as now (tell me
if i am wrong).
I was planing on doing this stuff, but never managed to finish this.
So enclosed a patch to add this feature.
As you can see in virnetclient.c there is no virAuthGetPassword call, so
the authfile is never used.
The patch enclosed is modifying the function prototype to add
virConnectPtr parameter (so remote_driver.c virNetClientNewLibSSH2 call
has to be updated too and the corresponding .h too).
Once we have access to virConnectPtr, as you will see in the patch we
can check if authMethods is set to 'password' and grab the password from
auth file by calling virAuthGetPassword.
please use git format-patch and send-email in the future, it makes
reviewing easier.
See the attached patch for the review.
Peter
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 7a0c1f6..012d6d5 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -658,7 +658,9 @@ doRemoteOpen(virConnectPtr conn,
sshauth,
netcat,
sockname,
- auth);
+ auth,
+ conn);
This line will require tweaking ... see below ...
+
if (!priv->client)
goto failed;
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index fed2c87..75c22d7 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -37,6 +37,7 @@
#include "virutil.h"
#include "virerror.h"
#include "virstring.h"
+#include "virauth.h"
#define VIR_FROM_THIS VIR_FROM_RPC
@@ -389,7 +390,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
const char *authMethods,
const char *netcatPath,
const char *socketPath,
- virConnectAuthPtr authPtr)
+ virConnectAuthPtr authPtr,
+ virConnectPtr conn
... the virConnectPtr should be the first argument in this case. Also the
closing brace of the parameter list should be on the same line as the last
argument.
+ )
{
virNetSocketPtr sock = NULL;
virNetClientPtr ret = NULL;
@@ -402,6 +405,7 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
char *confdir = virGetUserConfigDirectory();
char *knownhosts = NULL;
char *privkey = NULL;
+ char *password = NULL;
/* Use default paths for known hosts an public keys if not provided */
if (confdir) {
@@ -471,11 +475,21 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
if (!(command = virBufferContentAndReset(&buf)))
goto no_memory;
- if (virNetSocketNewConnectLibSSH2(host, port, username, NULL, privkey,
+ password = virAuthGetPassword(conn, authPtr, "libssh2", username, host);
+
+ if ((strcmp(authMethods,"password") == 0) && password) {
authMethods is a comma separated list of possible methods to use. With a strcmp
this will not work if the list will contain other methods too. Use strstr() instead of
strcmp. Also this condition should make the call to virAuthGetPassword conditional
otherwise we would always ask the user for the password if it isn't stored in the auth
file even if it will never be used.
Additionally, even with the password method enabled, the password authentication
may be skipped as other methods can be used before. This would query the user
for the password allways even if it wouldn't be used ... for example as agent
based authentication would succeed before password based auth would be tried.
+
+ if (virNetSocketNewConnectLibSSH2(host, port, username, password, privkey,
knownhosts, knownHostsVerify, authMethods,
command, authPtr, &sock) != 0)
- goto cleanup;
+ goto cleanup;
+ } else {
+ if (virNetSocketNewConnectLibSSH2(host, port, username, NULL, privkey,
+ knownhosts, knownHostsVerify, authMethods,
+ command, authPtr, &sock) != 0)
+ goto cleanup;
+ }
if (!(ret = virNetClientNew(sock, NULL)))
goto cleanup;
sock = NULL;
diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h
index 4204a93..ccaf8ab 100644
--- a/src/rpc/virnetclient.h
+++ b/src/rpc/virnetclient.h
@@ -61,7 +61,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
const char *authMethods,
const char *netcatPath,
const char *socketPath,
- virConnectAuthPtr authPtr);
+ virConnectAuthPtr authPtr,
+ virConnectPtr conn
again, the brace should be on the same line as the argument.
+ );
virNetClientPtr virNetClientNewExternal(const char **cmdargv);
In common, the idea is okay, but the implementation has a few problems
that can't be solved trivially. I'll try to revive the patches I wrote a few
months ago to solve the same issue and post them.
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list