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

[Peter Krempa]

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
>