[libvirt] [PATCH 0/3] libssh2: improve authentication options
This series adds the ability to use tunelled password authentication and enables to store credentials for such authentication in libvirts credential storage configuration. Peter Krempa (3): virAuth: Don't require virConnectPtr to retrieve authentication creds libssh2: Improve password based authentication remote: Improve libssh2 password authentication src/remote/remote_driver.c | 3 +- src/rpc/virnetclient.c | 11 ++--- src/rpc/virnetclient.h | 4 +- src/rpc/virnetsocket.c | 8 ++-- src/rpc/virnetsocket.h | 3 +- src/rpc/virnetsshsession.c | 110 ++++++++++++++++++++++++++++++++------------- src/rpc/virnetsshsession.h | 5 ++- src/util/virauth.c | 107 ++++++++++++++++++++++++++++++------------- src/util/virauth.h | 17 ++++++- 9 files changed, 191 insertions(+), 77 deletions(-) -- 1.8.2.1
Previously a connection object was required to retrieve the auth credentials. This patch adds the option to call the retrieval functions only using the connection URI or path to the configuration file. This will allow to use this toolkit to request passwords for ssh authentication in the libssh2 connection driver. Changes: *virAuthGetConfigFilePathURI(): use URI to retrieve the config file path *virAuthGetCredential(): Remove the need to propagate conn object virAuthGetPasswordPath(): *virAuthGetUsernamePath(): New functions, that use config file path instead of conn object --- src/util/virauth.c | 107 +++++++++++++++++++++++++++++++++++++---------------- src/util/virauth.h | 17 ++++++++- 2 files changed, 91 insertions(+), 33 deletions(-) diff --git a/src/util/virauth.c b/src/util/virauth.c index cd22e89..a19f55d 100644 --- a/src/util/virauth.c +++ b/src/util/virauth.c @@ -36,9 +36,9 @@ #define VIR_FROM_THIS VIR_FROM_AUTH - -int virAuthGetConfigFilePath(virConnectPtr conn, - char **path) +int +virAuthGetConfigFilePathURI(virURIPtr uri, + char **path) { int ret = -1; size_t i; @@ -56,13 +56,12 @@ int virAuthGetConfigFilePath(virConnectPtr conn, return 0; } - if (conn && conn->uri) { - for (i = 0; i < conn->uri->paramsCount; i++) { - if (STREQ_NULLABLE(conn->uri->params[i].name, "authfile") && - conn->uri->params[i].value) { - VIR_DEBUG("Using path from URI '%s'", - conn->uri->params[i].value); - if (VIR_STRDUP(*path, conn->uri->params[i].value) < 0) + if (uri) { + for (i = 0; i < uri->paramsCount; i++) { + if (STREQ_NULLABLE(uri->params[i].name, "authfile") && + uri->params[i].value) { + VIR_DEBUG("Using path from URI '%s'", uri->params[i].value); + if (VIR_STRDUP(*path, uri->params[i].value) < 0) goto cleanup; return 0; } @@ -105,33 +104,36 @@ no_memory: } +int +virAuthGetConfigFilePath(virConnectPtr conn, + char **path) +{ + return virAuthGetConfigFilePathURI(conn ? conn->uri : NULL, path); +} + + static int -virAuthGetCredential(virConnectPtr conn, - const char *servicename, +virAuthGetCredential(const char *servicename, + const char *hostname, const char *credname, + const char *path, char **value) { int ret = -1; - char *path = NULL; virAuthConfigPtr config = NULL; const char *tmp; *value = NULL; - if (virAuthGetConfigFilePath(conn, &path) < 0) - goto cleanup; - - if (path == NULL) { - ret = 0; - goto cleanup; - } + if (path == NULL) + return 0; if (!(config = virAuthConfigNew(path))) goto cleanup; if (virAuthConfigLookup(config, servicename, - VIR_URI_SERVER(conn->uri), + hostname, credname, &tmp) < 0) goto cleanup; @@ -143,24 +145,23 @@ virAuthGetCredential(virConnectPtr conn, cleanup: virAuthConfigFree(config); - VIR_FREE(path); return ret; } char * -virAuthGetUsername(virConnectPtr conn, - virConnectAuthPtr auth, - const char *servicename, - const char *defaultUsername, - const char *hostname) +virAuthGetUsernamePath(const char *path, + virConnectAuthPtr auth, + const char *servicename, + const char *defaultUsername, + const char *hostname) { unsigned int ncred; virConnectCredential cred; char *prompt; char *ret = NULL; - if (virAuthGetCredential(conn, servicename, "username", &ret) < 0) + if (virAuthGetCredential(servicename, hostname, "username", path, &ret) < 0) return NULL; if (ret != NULL) return ret; @@ -203,20 +204,41 @@ virAuthGetUsername(virConnectPtr conn, } - char * -virAuthGetPassword(virConnectPtr conn, +virAuthGetUsername(virConnectPtr conn, virConnectAuthPtr auth, const char *servicename, - const char *username, + const char *defaultUsername, const char *hostname) { + char *ret; + char *path; + + if (virAuthGetConfigFilePath(conn, &path) < 0) + return NULL; + + ret = virAuthGetUsernamePath(path, auth, servicename, + defaultUsername, hostname); + + VIR_FREE(path); + + return ret; +} + + +char * +virAuthGetPasswordPath(const char *path, + virConnectAuthPtr auth, + const char *servicename, + const char *username, + const char *hostname) +{ unsigned int ncred; virConnectCredential cred; char *prompt; char *ret = NULL; - if (virAuthGetCredential(conn, servicename, "password", &ret) < 0) + if (virAuthGetCredential(servicename, hostname, "password", path, &ret) < 0) return NULL; if (ret != NULL) return ret; @@ -252,3 +274,24 @@ virAuthGetPassword(virConnectPtr conn, return cred.result; } + + +char * +virAuthGetPassword(virConnectPtr conn, + virConnectAuthPtr auth, + const char *servicename, + const char *username, + const char *hostname) +{ + char *ret; + char *path; + + if (virAuthGetConfigFilePath(conn, &path) < 0) + return NULL; + + ret = virAuthGetPasswordPath(path, auth, servicename, username, hostname); + + VIR_FREE(path); + + return ret; +} diff --git a/src/util/virauth.h b/src/util/virauth.h index a24aef7..268eb34 100644 --- a/src/util/virauth.h +++ b/src/util/virauth.h @@ -1,6 +1,7 @@ /* * virauth.h: authentication related utility functions * + * Copyright (C) 2013 Red Hat, Inc. * Copyright (C) 2010 Matthias Bolte <matthias.bolte@googlemail.com> * * This library is free software; you can redistribute it and/or @@ -23,10 +24,15 @@ # define __VIR_AUTH_H__ # include "internal.h" +# include "viruri.h" int virAuthGetConfigFilePath(virConnectPtr conn, char **path); +int virAuthGetConfigFilePathURI(virURIPtr uri, + char **path); + + char *virAuthGetUsername(virConnectPtr conn, virConnectAuthPtr auth, const char *servicename, @@ -37,5 +43,14 @@ char *virAuthGetPassword(virConnectPtr conn, const char *servicename, const char *username, const char *hostname); - +char * virAuthGetUsernamePath(const char *path, + virConnectAuthPtr auth, + const char *servicename, + const char *defaultUsername, + const char *hostname); +char * virAuthGetPasswordPath(const char *path, + virConnectAuthPtr auth, + const char *servicename, + const char *username, + const char *hostname); #endif /* __VIR_AUTH_H__ */ -- 1.8.2.1
On Wed, Jul 10, 2013 at 08:42:03AM +0200, Peter Krempa wrote:
Previously a connection object was required to retrieve the auth credentials. This patch adds the option to call the retrieval functions only using the connection URI or path to the configuration file. This will allow to use this toolkit to request passwords for ssh authentication in the libssh2 connection driver.
Changes: *virAuthGetConfigFilePathURI(): use URI to retrieve the config file path *virAuthGetCredential(): Remove the need to propagate conn object
virAuthGetPasswordPath(): *virAuthGetUsernamePath(): New functions, that use config file path instead of conn object --- src/util/virauth.c | 107 +++++++++++++++++++++++++++++++++++++---------------- src/util/virauth.h | 17 ++++++++- 2 files changed, 91 insertions(+), 33 deletions(-)
ACK I won't force you to write a test case for this, since we don't already have a test virauth.h file APIs. If you should wish to write one anyway though..... Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 07/11/13 18:10, Daniel P. Berrange wrote:
On Wed, Jul 10, 2013 at 08:42:03AM +0200, Peter Krempa wrote:
Previously a connection object was required to retrieve the auth credentials. This patch adds the option to call the retrieval functions only using the connection URI or path to the configuration file. This will allow to use this toolkit to request passwords for ssh authentication in the libssh2 connection driver.
Changes: *virAuthGetConfigFilePathURI(): use URI to retrieve the config file path *virAuthGetCredential(): Remove the need to propagate conn object
virAuthGetPasswordPath(): *virAuthGetUsernamePath(): New functions, that use config file path instead of conn object --- src/util/virauth.c | 107 +++++++++++++++++++++++++++++++++++++---------------- src/util/virauth.h | 17 ++++++++- 2 files changed, 91 insertions(+), 33 deletions(-)
ACK
I won't force you to write a test case for this, since we don't already have a test virauth.h file APIs. If you should wish to write one anyway though.....
I will put that on my to-do list. I actually was thinking about testing this while writing the code.
Daniel
Series pushed, thanks for the review. Peter
The password authentication method wasn't used as there wasn't a pleasant way to pass the password. This patch adds the option to use virAuth util functions to request the password either from a config file or uses the conf callback to request it from the user. --- src/rpc/virnetsshsession.c | 80 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 64 insertions(+), 16 deletions(-) diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index b6aedc8..113fc6b 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -36,6 +36,7 @@ #include "virfile.h" #include "virobject.h" #include "virstring.h" +#include "virauth.h" #define VIR_FROM_THIS VIR_FROM_SSH @@ -97,6 +98,7 @@ struct _virNetSSHSession { /* authentication stuff */ virConnectAuthPtr cred; + char *authPath; virNetSSHAuthCallbackError authCbErr; size_t nauths; virNetSSHAuthMethodPtr *auths; @@ -156,6 +158,7 @@ virNetSSHSessionDispose(void *obj) VIR_FREE(sess->channelCommand); VIR_FREE(sess->hostname); VIR_FREE(sess->knownHostsFile); + VIR_FREE(sess->authPath); } static virClassPtr virNetSSHSessionClass; @@ -675,7 +678,8 @@ virNetSSHAuthenticatePrivkey(virNetSSHSessionPtr sess, return 0; } -/* perform tunelled password authentication + +/* perform password authentication, either directly or request the password * * Returns: 0 on success * 1 on authentication failure @@ -685,27 +689,71 @@ static int virNetSSHAuthenticatePassword(virNetSSHSessionPtr sess, virNetSSHAuthMethodPtr priv) { + char *password = NULL; char *errmsg; - int ret; + int ret = -1; + int rc; + + if (priv->password) { + /* tunelled password authentication */ + if ((ret = libssh2_userauth_password(sess->session, + priv->username, + priv->password)) == 0) { + ret = 0; + goto cleanup; + } + } else { + /* password authentication with interactive password request */ + if (!sess->cred || !sess->cred->cb) { + virReportError(VIR_ERR_SSH, "%s", + _("Can't perform authentication: " + "Authentication callback not provided")); + goto cleanup; + } - /* tunelled password authentication */ - if ((ret = libssh2_userauth_password(sess->session, - priv->username, - priv->password)) < 0) { - libssh2_session_last_error(sess->session, &errmsg, NULL, 0); - virReportError(VIR_ERR_AUTH_FAILED, - _("tunelled password authentication failed: %s"), - errmsg); + /* Try the authenticating the set amount of times. The server breaks the + * connection if maximum number of bad auth tries is exceeded */ + while (true) { + if (!(password = virAuthGetPasswordPath(sess->authPath, sess->cred, + "ssh", priv->username, + sess->hostname))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to retrieve password")); + goto cleanup; + } - if (ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED) - return 1; - else - return -1; + /* tunelled password authentication */ + if ((rc = libssh2_userauth_password(sess->session, + priv->username, + password)) == 0) { + ret = 0; + goto cleanup; + } + + if (ret != LIBSSH2_ERROR_AUTHENTICATION_FAILED) + break; + + VIR_FREE(password); + } } - /* auth success */ - return 0; + + /* error path */ + libssh2_session_last_error(sess->session, &errmsg, NULL, 0); + virReportError(VIR_ERR_AUTH_FAILED, + _("authentication failed: %s"), errmsg); + + /* determine exist status */ + if (ret == LIBSSH2_ERROR_AUTHENTICATION_FAILED) + return 1; + else + return -1; + +cleanup: + VIR_FREE(password); + return ret; } + /* perform keyboard interactive authentication * * Returns: 0 on success -- 1.8.2.1
On Wed, Jul 10, 2013 at 08:42:04AM +0200, Peter Krempa wrote:
The password authentication method wasn't used as there wasn't a pleasant way to pass the password. This patch adds the option to use virAuth util functions to request the password either from a config file or uses the conf callback to request it from the user. --- src/rpc/virnetsshsession.c | 80 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 64 insertions(+), 16 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
This patch enables the password authentication in the libssh2 connection driver. There are a few benefits to this step: 1) Hosts with challenge response authentication will now be supported with the libssh2 connection driver. 2) Credential for hosts can now be stored in the authentication credential config file --- src/remote/remote_driver.c | 3 ++- src/rpc/virnetclient.c | 11 ++++++----- src/rpc/virnetclient.h | 4 +++- src/rpc/virnetsocket.c | 8 ++++---- src/rpc/virnetsocket.h | 3 ++- src/rpc/virnetsshsession.c | 30 ++++++++++++++++-------------- src/rpc/virnetsshsession.h | 5 +++-- 7 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7f3e833..7bd3aa5 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -659,7 +659,8 @@ doRemoteOpen(virConnectPtr conn, sshauth, netcat, sockname, - auth); + auth, + conn->uri); if (!priv->client) goto failed; diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index fed2c87..b10d090 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -389,7 +389,8 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, const char *authMethods, const char *netcatPath, const char *socketPath, - virConnectAuthPtr authPtr) + virConnectAuthPtr authPtr, + virURIPtr uri) { virNetSocketPtr sock = NULL; virNetClientPtr ret = NULL; @@ -443,9 +444,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, if (!authMethods) { if (privkey) - authMethods = "agent,privkey,keyboard-interactive"; + authMethods = "agent,privkey,password,keyboard-interactive"; else - authMethods = "agent,keyboard-interactive"; + authMethods = "agent,password,keyboard-interactive"; } DEFAULT_VALUE(host, "localhost"); @@ -471,9 +472,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, if (!(command = virBufferContentAndReset(&buf))) goto no_memory; - if (virNetSocketNewConnectLibSSH2(host, port, username, NULL, privkey, + if (virNetSocketNewConnectLibSSH2(host, port, username, privkey, knownhosts, knownHostsVerify, authMethods, - command, authPtr, &sock) != 0) + command, authPtr, uri, &sock) != 0) goto cleanup; if (!(ret = virNetClientNew(sock, NULL))) diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 4204a93..3bcde63 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -33,6 +33,7 @@ # include "virnetclientprogram.h" # include "virnetclientstream.h" # include "virobject.h" +# include "viruri.h" virNetClientPtr virNetClientNewUNIX(const char *path, @@ -61,7 +62,8 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, const char *authMethods, const char *netcatPath, const char *socketPath, - virConnectAuthPtr authPtr); + virConnectAuthPtr authPtr, + virURIPtr uri); virNetClientPtr virNetClientNewExternal(const char **cmdargv); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 27709d8..c457bbd 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -742,13 +742,13 @@ int virNetSocketNewConnectLibSSH2(const char *host, const char *port, const char *username, - const char *password, const char *privkey, const char *knownHosts, const char *knownHostsVerify, const char *authMethods, const char *command, virConnectAuthPtr auth, + virURIPtr uri, virNetSocketPtr *retsock) { virNetSocketPtr sock = NULL; @@ -810,8 +810,8 @@ virNetSocketNewConnectLibSSH2(const char *host, ret = virNetSSHSessionAuthAddKeyboardAuth(sess, username, -1); else if (STRCASEEQ(authMethod, "password")) ret = virNetSSHSessionAuthAddPasswordAuth(sess, - username, - password); + uri, + username); else if (STRCASEEQ(authMethod, "privkey")) ret = virNetSSHSessionAuthAddPrivKeyAuth(sess, username, @@ -856,13 +856,13 @@ int virNetSocketNewConnectLibSSH2(const char *host ATTRIBUTE_UNUSED, const char *port ATTRIBUTE_UNUSED, const char *username ATTRIBUTE_UNUSED, - const char *password ATTRIBUTE_UNUSED, const char *privkey ATTRIBUTE_UNUSED, const char *knownHosts ATTRIBUTE_UNUSED, const char *knownHostsVerify ATTRIBUTE_UNUSED, const char *authMethods ATTRIBUTE_UNUSED, const char *command ATTRIBUTE_UNUSED, virConnectAuthPtr auth ATTRIBUTE_UNUSED, + virURIPtr uri ATTRIBUTE_UNUSED, virNetSocketPtr *retsock ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index ea42081..ca9ae91 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -34,6 +34,7 @@ # include "virnetsaslcontext.h" # endif # include "virjson.h" +# include "viruri.h" typedef struct _virNetSocket virNetSocket; typedef virNetSocket *virNetSocketPtr; @@ -84,13 +85,13 @@ int virNetSocketNewConnectSSH(const char *nodename, int virNetSocketNewConnectLibSSH2(const char *host, const char *port, const char *username, - const char *password, const char *privkey, const char *knownHosts, const char *knownHostsVerify, const char *authMethods, const char *command, virConnectAuthPtr auth, + virURIPtr uri, virNetSocketPtr *retsock); int virNetSocketNewConnectExternal(const char **cmdargv, diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 113fc6b..9965623 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -994,25 +994,29 @@ virNetSSHSessionAuthReset(virNetSSHSessionPtr sess) int virNetSSHSessionAuthAddPasswordAuth(virNetSSHSessionPtr sess, - const char *username, - const char *password) + virURIPtr uri, + const char *username) { virNetSSHAuthMethodPtr auth; char *user = NULL; - char *pass = NULL; - if (!username || !password) { - virReportError(VIR_ERR_SSH, "%s", - _("Username and password must be provided " - "for password authentication")); - return -1; + if (uri) { + VIR_FREE(sess->authPath); + + if (virAuthGetConfigFilePathURI(uri, &sess->authPath) < 0) + goto error; } - virObjectLock(sess); + if (!username) { + if (!(user = virAuthGetUsernamePath(sess->authPath, sess->cred, + "ssh", NULL, sess->hostname))) + goto error; + } else { + if (VIR_STRDUP(user, username) < 0) + goto error; + } - if (VIR_STRDUP(user, username) < 0 || - VIR_STRDUP(pass, password) < 0) - goto error; + virObjectLock(sess); if (!(auth = virNetSSHSessionAuthMethodNew(sess))) { virReportOOMError(); @@ -1020,7 +1024,6 @@ virNetSSHSessionAuthAddPasswordAuth(virNetSSHSessionPtr sess, } auth->username = user; - auth->password = pass; auth->method = VIR_NET_SSH_AUTH_PASSWORD; virObjectUnlock(sess); @@ -1028,7 +1031,6 @@ virNetSSHSessionAuthAddPasswordAuth(virNetSSHSessionPtr sess, error: VIR_FREE(user); - VIR_FREE(pass); virObjectUnlock(sess); return -1; } diff --git a/src/rpc/virnetsshsession.h b/src/rpc/virnetsshsession.h index 8bd2445..65bd76a 100644 --- a/src/rpc/virnetsshsession.h +++ b/src/rpc/virnetsshsession.h @@ -23,6 +23,7 @@ # define __VIR_NET_SSH_SESSION_H__ # include "internal.h" +# include "viruri.h" typedef struct _virNetSSHSession virNetSSHSession; typedef virNetSSHSession *virNetSSHSessionPtr; @@ -50,8 +51,8 @@ int virNetSSHSessionAuthSetCallback(virNetSSHSessionPtr sess, virConnectAuthPtr auth); int virNetSSHSessionAuthAddPasswordAuth(virNetSSHSessionPtr sess, - const char *username, - const char *password); + virURIPtr uri, + const char *username); int virNetSSHSessionAuthAddAgentAuth(virNetSSHSessionPtr sess, const char *username); -- 1.8.2.1
On Wed, Jul 10, 2013 at 08:42:05AM +0200, Peter Krempa wrote:
This patch enables the password authentication in the libssh2 connection driver. There are a few benefits to this step:
1) Hosts with challenge response authentication will now be supported with the libssh2 connection driver.
2) Credential for hosts can now be stored in the authentication credential config file --- src/remote/remote_driver.c | 3 ++- src/rpc/virnetclient.c | 11 ++++++----- src/rpc/virnetclient.h | 4 +++- src/rpc/virnetsocket.c | 8 ++++---- src/rpc/virnetsocket.h | 3 ++- src/rpc/virnetsshsession.c | 30 ++++++++++++++++-------------- src/rpc/virnetsshsession.h | 5 +++-- 7 files changed, 36 insertions(+), 28 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Peter Krempa