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

[Christophe de Dinechin]

> On 19 Feb 2017, at 15:47, Snir Sheriber <ssheribe at 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>”.


> +
> +    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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel