<div dir="ltr"><div>Hi<br><br><br></div>Patch looks ok to me (although I still think it's a superflous check at spice-gtk level)<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 4, 2015 at 11:44 AM, Cédric Bosdonnat <span dir="ltr"><<a href="mailto:cbosdonnat@suse.com" target="_blank">cbosdonnat@suse.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Make sure that the password length is under the maximum lenght. If not<br>
report it as an authentication failure with an adapted message.<br>
---<br>
 Diff to v4<br>
   * Applied teuf's feedback<br>
<br>
 gtk/spice-channel.c | 77 +++++++++++++++++++++++++++++++++--------------------<br>
 1 file changed, 48 insertions(+), 29 deletions(-)<br>
<br>
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c<br>
index 4e7d8b7..a835c10 100644<br>
--- a/gtk/spice-channel.c<br>
+++ b/gtk/spice-channel.c<br>
@@ -1010,7 +1010,34 @@ static int spice_channel_read(SpiceChannel *channel, void *data, size_t length)<br>
 }<br>
<br>
 /* coroutine context */<br>
-static void spice_channel_send_spice_ticket(SpiceChannel *channel)<br>
+static void spice_channel_failed_authentication(SpiceChannel *channel,<br>
+                                                gboolean invalidPassword)<br>
+{<br>
+    SpiceChannelPrivate *c = channel->priv;<br>
+<br>
+    if (c->auth_needs_username_and_password)<br>
+        g_set_error_literal(&c->error,<br>
+                            SPICE_CLIENT_ERROR,<br>
+                            SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,<br>
+                            _("Authentication failed: password and username are required"));<br>
+    else if (invalidPassword)<br>
+        g_set_error_literal(&c->error,<br>
+                            SPICE_CLIENT_ERROR,<br>
+                            SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,<br>
+                            _("Authentication failed: password is too long"));<br>
+    else<br>
+        g_set_error_literal(&c->error,<br>
+                            SPICE_CLIENT_ERROR,<br>
+                            SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,<br>
+                            _("Authentication failed: password is required"));<br>
+<br>
+    c->event = SPICE_CHANNEL_ERROR_AUTH;<br>
+<br>
+    c->has_error = TRUE; /* force disconnect */<br>
+}<br>
+<br>
+/* coroutine context */<br>
+static SpiceChannelEvent spice_channel_send_spice_ticket(SpiceChannel *channel)<br>
 {<br>
     SpiceChannelPrivate *c = channel->priv;<br>
     EVP_PKEY *pubkey;<br>
@@ -1020,13 +1047,14 @@ static void spice_channel_send_spice_ticket(SpiceChannel *channel)<br>
     char *password;<br>
     uint8_t *encrypted;<br>
     int rc;<br>
+    SpiceChannelEvent ret = SPICE_CHANNEL_ERROR_LINK;<br>
<br>
     bioKey = BIO_new(BIO_s_mem());<br>
-    g_return_if_fail(bioKey != NULL);<br>
+    g_return_val_if_fail(bioKey != NULL, ret);<br>
<br>
     BIO_write(bioKey, c->peer_msg->pub_key, SPICE_TICKET_PUBKEY_BYTES);<br>
     pubkey = d2i_PUBKEY_bio(bioKey, NULL);<br>
-    g_return_if_fail(pubkey != NULL);<br>
+    g_return_val_if_fail(pubkey != NULL, ret);<br>
<br>
     rsa = pubkey->pkey.rsa;<br>
     nRSASize = RSA_size(rsa);<br>
@@ -1039,36 +1067,24 @@ static void spice_channel_send_spice_ticket(SpiceChannel *channel)<br>
     g_object_get(c->session, "password", &password, NULL);<br>
     if (password == NULL)<br>
         password = g_strdup("");<br>
+    if (strlen(password) > SPICE_MAX_PASSWORD_LENGTH) {<br>
+        spice_channel_failed_authentication(channel, TRUE);<br>
+        ret = SPICE_CHANNEL_ERROR_AUTH;<br>
+        goto cleanup;<br>
+    }<br>
     rc = RSA_public_encrypt(strlen(password) + 1, (uint8_t*)password,<br>
                             encrypted, rsa, RSA_PKCS1_OAEP_PADDING);<br>
     g_warn_if_fail(rc > 0);<br>
<br>
     spice_channel_write(channel, encrypted, nRSASize);<br>
+    ret = SPICE_CHANNEL_NONE;<br>
+<br>
+cleanup:<br>
     memset(encrypted, 0, nRSASize);<br>
     EVP_PKEY_free(pubkey);<br>
     BIO_free(bioKey);<br>
     g_free(password);<br>
-}<br>
-<br>
-/* coroutine context */<br>
-static void spice_channel_failed_authentication(SpiceChannel *channel)<br>
-{<br>
-    SpiceChannelPrivate *c = channel->priv;<br>
-<br>
-    if (c->auth_needs_username_and_password)<br>
-        g_set_error_literal(&c->error,<br>
-                            SPICE_CLIENT_ERROR,<br>
-                            SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,<br>
-                            _("Authentication failed: password and username are required"));<br>
-    else<br>
-        g_set_error_literal(&c->error,<br>
-                            SPICE_CLIENT_ERROR,<br>
-                            SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD,<br>
-                            _("Authentication failed: password is required"));<br>
-<br>
-    c->event = SPICE_CHANNEL_ERROR_AUTH;<br>
-<br>
-    c->has_error = TRUE; /* force disconnect */<br>
+    return ret;<br>
 }<br>
<br>
 /* coroutine context */<br>
@@ -1088,7 +1104,7 @@ static gboolean spice_channel_recv_auth(SpiceChannel *channel)<br>
<br>
     if (link_res != SPICE_LINK_ERR_OK) {<br>
         CHANNEL_DEBUG(channel, "link result: reply %d", link_res);<br>
-        spice_channel_failed_authentication(channel);<br>
+        spice_channel_failed_authentication(channel, FALSE);<br>
         return FALSE;<br>
     }<br>
<br>
@@ -1662,7 +1678,7 @@ error:<br>
     if (saslconn)<br>
         sasl_dispose(&saslconn);<br>
<br>
-    spice_channel_failed_authentication(channel);<br>
+    spice_channel_failed_authentication(channel, FALSE);<br>
     ret = FALSE;<br>
<br>
 cleanup:<br>
@@ -1681,6 +1697,7 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)<br>
     SpiceChannelPrivate *c;<br>
     int rc, num_caps, i;<br>
     uint32_t *caps;<br>
+    SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK;<br>
<br>
     g_return_val_if_fail(channel != NULL, FALSE);<br>
     g_return_val_if_fail(channel->priv != NULL, FALSE);<br>
@@ -1733,7 +1750,8 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)<br>
     if (!spice_channel_test_common_capability(channel,<br>
             SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION)) {<br>
         CHANNEL_DEBUG(channel, "Server supports spice ticket auth only");<br>
-        spice_channel_send_spice_ticket(channel);<br>
+        if ((event = spice_channel_send_spice_ticket(channel)) != SPICE_CHANNEL_NONE)<br>
+            goto error;<br>
     } else {<br>
         SpiceLinkAuthMechanism auth = { 0, };<br>
<br>
@@ -1749,7 +1767,8 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)<br>
         if (spice_channel_test_common_capability(channel, SPICE_COMMON_CAP_AUTH_SPICE)) {<br>
             auth.auth_mechanism = SPICE_COMMON_CAP_AUTH_SPICE;<br>
             spice_channel_write(channel, &auth, sizeof(auth));<br>
-            spice_channel_send_spice_ticket(channel);<br>
+            if ((event = spice_channel_send_spice_ticket(channel)) != SPICE_CHANNEL_NONE)<br>
+                goto error;<br>
         } else {<br>
             g_warning("No compatible AUTH mechanism");<br>
             goto error;<br>
@@ -1762,7 +1781,7 @@ static gboolean spice_channel_recv_link_msg(SpiceChannel *channel)<br>
<br>
 error:<br>
     c->has_error = TRUE;<br>
-    c->event = SPICE_CHANNEL_ERROR_LINK;<br>
+    c->event = event;<br>
     return FALSE;<br>
 }<br>
<span class="HOEnZb"><font color="#888888"><br>
--<br>
2.1.4<br>
<br>
_______________________________________________<br>
Spice-devel mailing list<br>
<a href="mailto:Spice-devel@lists.freedesktop.org">Spice-devel@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/spice-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/spice-devel</a><br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature">Marc-André Lureau</div>
</div>