[Spice-devel] [v3: PATCH 3/3] Add support to handle username when connecting with SASL

Marc-André Lureau mlureau at redhat.com
Wed Oct 8 04:53:03 PDT 2014



----- Original Message -----
> Based on a patch from Dietmar Maurer <dietmar at proxmox.com>
> http://lists.freedesktop.org/archives/spice-devel/2013-October/015138.html
> ---
> Changes since v2:
> - Adapt the code to set errors from SPICE_CLIENT_ERROR domain instead of
>   SPICE_CHANELL_ERROR_AUTH
> ---
>  gtk/spice-channel-priv.h |  1 +
>  gtk/spice-channel.c      | 45 ++++++++++++++++++++++++++++++++++++++++-----
>  po/POTFILES.in           |  1 +
>  3 files changed, 42 insertions(+), 5 deletions(-)
> 
> diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
> index 03eed38..6067abc 100644
> --- a/gtk/spice-channel-priv.h
> +++ b/gtk/spice-channel-priv.h
> @@ -136,6 +136,7 @@ struct _SpiceChannelPrivate {
>      GSList                      *flushing;
>  
>      gboolean                    disable_channel_msg;
> +    gboolean                    auth_needs_username_and_password;
>      GError                      *error;
>  };
>  
> diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> index 5d1a86e..5718adf 100644
> --- a/gtk/spice-channel.c
> +++ b/gtk/spice-channel.c
> @@ -26,6 +26,8 @@
>  #include "spice-marshal.h"
>  #include "bio-gio.h"
>  
> +#include <glib/gi18n.h>
> +
>  #include <openssl/rsa.h>
>  #include <openssl/evp.h>
>  #include <openssl/x509.h>
> @@ -107,6 +109,7 @@ static void spice_channel_init(SpiceChannel *channel)
>      c->out_serial = 1;
>      c->in_serial = 1;
>      c->fd = -1;
> +    c->auth_needs_username_and_password = FALSE;

This is superflous, however, I would reset the value in channel_reset(), so object could be recycled in clean state.

>      strcpy(c->name, "?");
>      c->caps = g_array_new(FALSE, TRUE, sizeof(guint32));
>      c->common_caps = g_array_new(FALSE, TRUE, sizeof(guint32));
> @@ -1242,6 +1245,7 @@ spice_channel_gather_sasl_credentials(SpiceChannel
> *channel,
>  {
>      SpiceChannelPrivate *c;
>      int ninteract;
> +    gboolean ret = TRUE;
>  
>      g_return_val_if_fail(channel != NULL, FALSE);
>      g_return_val_if_fail(channel->priv != NULL, FALSE);
> @@ -1254,12 +1258,22 @@ spice_channel_gather_sasl_credentials(SpiceChannel
> *channel,
>          switch (interact[ninteract].id) {
>          case SASL_CB_AUTHNAME:
>          case SASL_CB_USER:
> -            g_warn_if_reached();
> +            c->auth_needs_username_and_password = TRUE;
> +            if (spice_session_get_username(c->session) == NULL)
> +                return FALSE;
> +
> +            interact[ninteract].result =
> spice_session_get_username(c->session);
> +            interact[ninteract].len = strlen(interact[ninteract].result);
>              break;
>  
>          case SASL_CB_PASS:
> -            if (spice_session_get_password(c->session) == NULL)
> -                return FALSE;
> +            if (spice_session_get_password(c->session) == NULL) {
> +                /* Even if we reach this point, we have to continue looking
> for
> +                 * SASL_CB_AUTHNAME|SASL_CB_USER, otherwise we would return
> a
> +                 * wrong error to the applications */
> +                ret = FALSE;
> +                continue;
> +            }
>  
>              interact[ninteract].result =
>              spice_session_get_password(c->session);
>              interact[ninteract].len = strlen(interact[ninteract].result);
> @@ -1269,7 +1283,7 @@ spice_channel_gather_sasl_credentials(SpiceChannel
> *channel,
>  
>      CHANNEL_DEBUG(channel, "Filled SASL interact");
>  
> -    return TRUE;
> +    return ret;
>  }
>  
>  /*
> @@ -1308,6 +1322,22 @@ spice_channel_gather_sasl_credentials(SpiceChannel
> *channel,
>  #define SASL_MAX_MECHNAME_LEN 100
>  #define SASL_MAX_DATA_LEN (1024 * 1024)
>  
> +static void spice_channel_set_detailed_authentication_error(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"));
> +}
> +
>  /* Perform the SASL authentication process
>   */
>  static gboolean spice_channel_perform_auth_sasl(SpiceChannel *channel)
> @@ -1323,6 +1353,8 @@ static gboolean
> spice_channel_perform_auth_sasl(SpiceChannel *channel)
>      const void *val;
>      sasl_ssf_t ssf;
>      static const sasl_callback_t saslcb[] = {
> +        { .id = SASL_CB_USER },
> +        { .id = SASL_CB_AUTHNAME },
>          { .id = SASL_CB_PASS },
>          { .id = 0 },
>      };
> @@ -1624,8 +1656,10 @@ restart:
>  complete:
>      CHANNEL_DEBUG(channel, "%s", "SASL authentication complete");
>      spice_channel_read(channel, &len, sizeof(len));
> -    if (len != SPICE_LINK_ERR_OK)
> +    if (len != SPICE_LINK_ERR_OK) {
> +        spice_channel_set_detailed_authentication_error(channel);
>          g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0,
>          SPICE_CHANNEL_ERROR_AUTH);
> +    }
>      ret = len == SPICE_LINK_ERR_OK;
>      /* This must come *after* check-auth-result, because the former
>       * is defined to be sent unencrypted, and setting saslconn turns
> @@ -1636,6 +1670,7 @@ complete:
>  error:
>      if (saslconn)
>          sasl_dispose(&saslconn);
> +    spice_channel_set_detailed_authentication_error(channel);
>      g_coroutine_signal_emit(channel, signals[SPICE_CHANNEL_EVENT], 0,
>      SPICE_CHANNEL_ERROR_AUTH);
>      c->has_error = TRUE; /* force disconnect */
>      ret = FALSE;
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 8809121..3375ab5 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -2,6 +2,7 @@ data/spice-mime.xml.in
>  data/spicy.desktop.in.in
>  gtk/channel-usbredir.c
>  gtk/desktop-integration.c
> +gtk/spice-channel.c
>  gtk/spice-cmdline.c
>  gtk/spice-option.c
>  gtk/spicy-screenshot.c
> --
> 2.1.0

looks good otherwise, ack 

> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 


More information about the Spice-devel mailing list