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).
Or probably better is to change the API signature to take a virURIPtr
instead of a virConnectPtr, since the URI is the things that's really
wanted here.
Daniel