> On 21 Feb 2017, at 16:28, Snir Sheriber <ssher...@redhat.com> wrote:
>
> Hi,
>
>
> On 02/20/2017 07:00 PM, Christophe de Dinechin wrote:
>>> On 19 Feb 2017, at 15:47, Snir Sheriber <ssher...@redhat.com> wrote:
>>>
>>> Remove handling with failures in the SASL authentication
>>> process to separate function
>>> ---
>>> src/spice-channel.c | 44 +++++++++++++++++++++++++++-----------------
>>> 1 file changed, 27 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/src/spice-channel.c b/src/spice-channel.c
>>> index af67931..cbf1291 100644
>>> --- a/src/spice-channel.c
>>> +++ b/src/spice-channel.c
>>> @@ -1113,28 +1113,38 @@ 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)
>>> {
>>> SpiceChannelPrivate *c = channel->priv;
>>> + gint err_code; /* Affects the authentication window activated 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)
>>> - 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)
>>> - g_set_error_literal(&c->error,
>>> - SPICE_CLIENT_ERROR,
>>> - SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
>>> - _("Authentication failed: password is
>>> required"));
>>> - else if (invalidPassword)
>>> + err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME;
>>> + else
>>> + err_code = SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD;
>>> +
>>> + g_set_error_literal(&c->error,
>>> + SPICE_CLIENT_ERROR,
>>> + err_code,
>>> + _("SASL authentication failed"));
>> Per the recent discussion (Feb 14 with Christophe F), can’t we map common
>> SASL errors to Spice messages? To me, it’s different if the problem is that
>> I used a wrong password or if the server is down. The message as is seems
>> quite terse.
>>
>> Errors that seem be reportable (although not all of them seem relevant to
>> Spice):
>>
>> SASL_BADAUTH Authentication failure.
>> SASL_NOAUTHZ Authorization failure.
>> SASL_EXPIRED The passphrase expired and must be reset.
>> SASL_DISABLED Account disabled.
>> SASL_NOUSER User not found.
>> SASL_BADVERS Version mismatch with plug-in.
>> SASL_NOVERIFY The user exists, but there is no verifier for the user.
>> SASL_WEAKPASS The passphrase is too weak for security policy.
>> SASL_NOUSERPASS User supplied passwords are not permitted.
>>
>>
>> Some that may need to be “translated” in Spicese if they ever get back to us:
>>
>> SASL_TOOWEAK The mechanism is too weak for this user.
>> SASL_ENCRYPT Encryption is needed to use this mechanism.
>> SASL_TRANS One time use of a plaintext password will enable
>> requested mechanism for user.
>>
>> Others should probably collected into a “default” in a switch statement,
>> something like “Unexpected SASL error code <blah>”.
>>
> As link result/error? I guess it would be the best , but first it requires to
> inform the client in some other way that it can stop waiting for the sasl
> server start\step result (currently it just freeing the link)
> btw according to sasl docs the full error string should be sent
Yes, but not translated. We can always add the SASL errors to the po files.
Christophe
>>> +
>>> + c->event = SPICE_CHANNEL_ERROR_AUTH;
>>> +
>>> + c->has_error = TRUE; /* force disconnect */
>>> +}
>>> +#endif
>>> +
>>> +/* coroutine context */
>>> +static void spice_channel_failed_authentication(SpiceChannel *channel,
>>> + gboolean invalidPassword)
>>> +{
>>> + SpiceChannelPrivate *c = channel->priv;
>>> +
>>> + if (invalidPassword)
>>> g_set_error_literal(&c->error,
>>> SPICE_CLIENT_ERROR,
>>> SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,
>>> @@ -1808,7 +1818,7 @@ error:
>>> if (saslconn)
>>> sasl_dispose(&saslconn);
>>>
>>> - spice_channel_failed_authentication(channel, FALSE);
>>> + spice_channel_failed_sasl_authentication(channel);
>>> ret = FALSE;
>>>
>>> cleanup:
>>> --
>>> 2.9.3
>>>
>>> _______________________________________________
>>> Spice-devel mailing list
>>> Spice-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
