[Spice-devel] [PATCH v5] Check too long password

Marc-André Lureau marcandre.lureau at gmail.com
Mon Jun 15 09:46:35 PDT 2015


My bad Cédric, I applied your patch and I had to modify it after the
gtk->src move.

Unfortunately, during this churning, I changed the author name, and I
didn't notcied before I pushed it..

Sorry, thank you very much for your contribution, at least it made it!

On Thu, Jun 4, 2015 at 5:35 PM, Marc-André Lureau <
marcandre.lureau at gmail.com> wrote:

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



-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150615/acd7a4d2/attachment-0001.html>


More information about the Spice-devel mailing list