[Spice-devel] [PATCH spice-server v4 6/9] Handle SASL initialisation mainly in red-stream.c

[Christophe Fergeau]

On Fri, Jan 05, 2018 at 03:45:31PM +0000, Frediano Ziglio wrote:
> -static void reds_handle_auth_sasl_start(void *opaque)
> +static void reds_handle_sasl_result(void *opaque, RedSaslError status)
>  {
>      RedLinkInfo *link = (RedLinkInfo *)opaque;
> -    RedSaslError status;
> -
> -    status = red_sasl_handle_auth_start(link->stream, reds_handle_auth_sasl_steplen, link);
> -    if (status == RED_SASL_ERROR_OK) {
> -        reds_handle_link(link);
> -    } else if (status != RED_SASL_ERROR_CONTINUE) {
> -        reds_link_free(link);
> -    }
> -}
>  
> -static void reds_handle_auth_startlen(void *opaque)
> -{
> -    RedLinkInfo *link = (RedLinkInfo *)opaque;
> -    RedSaslError status;
> -
> -    status = red_sasl_handle_auth_startlen(link->stream, reds_handle_auth_sasl_start, link);
>      switch (status) {
> -        case RED_SASL_ERROR_OK:
> -            break;
> -        case RED_SASL_ERROR_RETRY:
> -            reds_handle_auth_sasl_start(opaque);
> -            break;
> -        case RED_SASL_ERROR_GENERIC:
> -        case RED_SASL_ERROR_INVALID_DATA:
> -            reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
> -            reds_link_free(link);
> -            break;
> -        default:
> -            g_warn_if_reached();
> -            reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
> -            reds_link_free(link);
> -            break;
> -    }
> -}
> -
> -static void reds_handle_auth_mechname(void *opaque)
> -{
> -    RedLinkInfo *link = (RedLinkInfo *)opaque;
> -
> -    if (!red_sasl_handle_auth_mechname(link->stream, reds_handle_auth_startlen, link)) {
> -            reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
> +    case RED_SASL_ERROR_OK:
> +        reds_handle_link(link);
> +        break;
> +    case RED_SASL_ERROR_GENERIC:
> +    case RED_SASL_ERROR_INVALID_DATA:
> +        reds_send_link_error(link, SPICE_LINK_ERR_INVALID_DATA);
>          reds_link_free(link);

The only time when the initial code is calling reds_send_link_error() is
if reds_handle_auth_startlen() returns an error. If any of the other
methods returns an error, we don't call reds_send_link_error().
I tested authentication failures with digest-md5 and your v4, and this
extra call is causing warnings from the client:
(remote-viewer:24029): GSpice-CRITICAL **: SASL negotiation data too long: 1363428690 bytes

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180105/28795b3c/attachment.sig>