[Spice-devel] [PATCH 1/2] authentication: Handle failed SASL authentication separately

[Snir Sheriber]

Hi,


On 02/14/2017 06:42 PM, Christophe Fergeau wrote:
> On Mon, Feb 13, 2017 at 03:49:44PM +0200, Snir Sheriber wrote:
>> Remove handling with failures in the SASL authentication
>> process to separate function and display the error message
>> as reported by the SASL client (could also display SASL
>> server error message if error number was sent to the client)
>> ---
>>   src/spice-channel.c | 42 +++++++++++++++++++++++++++++-------------
>>   1 file changed, 29 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/spice-channel.c b/src/spice-channel.c
>> index 6556db3..37e36d9 100644
>> --- a/src/spice-channel.c
>> +++ b/src/spice-channel.c
>> @@ -1113,28 +1113,44 @@ static int spice_channel_read(SpiceChannel *channel, void *data, size_t length)
>>       return length;
>>   }
>>   
>> +#if HAVE_SASL
>>   /* coroutine context */
>> -static void spice_channel_failed_authentication(SpiceChannel *channel,
>> -                                                gboolean invalidPassword)
>> +static void spice_channel_failed_sasl_authentication(SpiceChannel *channel, int err)
>>   {
>>       SpiceChannelPrivate *c = channel->priv;
>> +    gint err_code; /* Affects the authentication window fileds */
>>   
>>       if (c->auth_needs_username && c->auth_needs_password)
>> -        g_set_error_literal(&c->error,
>> -                            SPICE_CLIENT_ERROR,
>> -                            SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
>> -                            _("Authentication failed: password and username are required"));
>> +        err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME;
>>       else if (c->auth_needs_username)
>> +        err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME;
>> +    else
>> +        err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD;
>> +
>> +    if (err < 0)
>>           g_set_error_literal(&c->error,
>>                               SPICE_CLIENT_ERROR,
>> -                            SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
>> -                            _("Authentication failed: username is required"));
>> -    else if (c->auth_needs_password)
>> +                            err_code,
>> +                            sasl_errstring(err, NULL, NULL));
> I'm not sure what you mean by "display" in the commit log. If you want
> this string to appear in the debug log, this sounds fine to me. If the
> goal is to show that to the user, I'm not so sure about this, the errors
> listed at
> https://docs.oracle.com/cd/E53394_01/html/E54774/sasl-errors-3sasl.html#REFMAN3Bsasl-errors-3sasl
> do not seem so useful.
>
> Christophe

yes, the idea is to present errors which are generated on the sasl 
server side, in the err window on the user/sasl-client side (only 
errors- without sasl_ok, continue , interact) by sending the error 
number to the client and print the relevant string (i'll send these 
patches again with another one that do this later so it will be clearer )
imho, this would be better then the current err msg that is being 
printed.. e.g "Authentication failure" would printed if password is 
incorrect or "User not found"  for wrong\nonexist user in oppose to 
"password and user-name are required" that currently printed for both.
It is possible that other message will show up and i think it's also 
fine but it is also possible to filter only specific errors and print 
whatever we want.

Snir.