Re: [libvirt] [PATCH 1/2] util: Add possibility to call virAuthGetPassword without conn object

On 11/27/12 15:56, Daniel P. Berrange wrote: >On Tue, Nov 27, 2012 at 02:55:11PM +0000, Daniel P. Berrange wrote: >>On Tue, Nov 27, 2012 at 03:39:16PM +0100, Peter Krempa wrote: >>>To make this function callable from code that doesn't have the conn >>>object, call the conn-dependant code only if conn was provided. >>>--- >>> src/util/virauth.c | 13 ++++++++----- >>> 1 file changed, 8 insertions(+), 5 deletions(-) >>> >>>diff --git a/src/util/virauth.c b/src/util/virauth.c >>>index 6d9935d..738b3c5 100644 >>>--- a/src/util/virauth.c >>>+++ b/src/util/virauth.c >>>@@ -205,7 +205,8 @@ virAuthGetUsername(virConnectPtr conn, >>> } >>> >>> >>>- >>>+/* if this function is called with conn == NULL, the code requesting >>>+ * the password from the config file is skipped */ >>> char * >>> virAuthGetPassword(virConnectPtr conn, >>> virConnectAuthPtr auth, >>>@@ -218,10 +219,12 @@ virAuthGetPassword(virConnectPtr conn, >>> char *prompt; >>> char *ret = NULL; >>> >>>- if (virAuthGetCredential(conn, servicename, "password", &ret) < 0) >>>- return NULL; >>>- if (ret != NULL) >>>- return ret; >>>+ if (conn) { >>>+ if (virAuthGetCredential(conn, servicename, "password", &ret) < 0) >>>+ return NULL; >>>+ if (ret != NULL) >>>+ return ret; >>>+ } >>> >>> memset(&cred, 0, sizeof(virConnectCredential)); >> >>NACK, I don't think this is right. The libssh2 code that is using this >>must have a virConnectPtr instance somewhere in its callstack. We The connection object is available in the doRemoteOpen function in the remote driver and it might be passed further down through the virNetClient and virNetSocket objects until it reaches pure libssh2 transport code. >>should fix the code to pass the connection in where needed, not skip >>this. This might be needed but right now the credential storage option is not desired. The original keyboard-interactive implementation has to ask the user for various arbitrary challenges that are provided by the server. The virAuth implementation does not allow this.

The code provided in patch 2/2 is adding fallback method to do the authentication with passwords when keyboard interactive is not available. When the user will be able to store the passwords in the auth file, this will introduce dual behavior on hosts supporting keyboard interactive auth where the user won't be able to store the password as the challenge is provided by the remote server and virauth does not support getting arbitrary requests from the user. On hosts that don't support this, the user would be able to save passwords. With that change the fallback behavior wouldn't be desired. We might change the default authentication method to be password and leave keyboard-interactive for really special scenarios that wouldn't support storing the credentials. (This at the cost of polluting virNetClient and virNetSocket with the connection object (or the URI) and even more added code).