[Spice-devel] [PATCH v5] Check too long password
Marc-André Lureau
marcandre.lureau at gmail.com
Thu Jun 4 08:35:45 PDT 2015
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150604/1282b3d7/attachment.html>
More information about the Spice-devel
mailing list