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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 21 Feb 2017, at 16:28, Snir Sheriber <ssheribe@xxxxxxxxxx> wrote:

Hi,


On 02/20/2017 07:00 PM, Christophe de Dinechin wrote:
On 19 Feb 2017, at 15:47, Snir Sheriber <ssheribe@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]