Re: Connecting to vino fails

[Guido Günther <agx sigxcpu org>]

Hi Daniel,
On Mon, Sep 13, 2010 at 11:21:28AM +0100, Daniel P. Berrange wrote:
> On Sat, Sep 11, 2010 at 09:35:59PM +0200, Guido Günther wrote:
> > On Wed, Sep 01, 2010 at 07:03:12PM +0100, Daniel P. Berrange wrote:
> > > The server initially offers 2 auth types in order 'TLS(18)',
> > > 'None(1)'. Most clients will choose 'none', since they don't
> > > support TLS auth. GTK-VNC will choose 'TLS' since that's the
> > > first reported one by the server.  The first phase of TLS
> > > auth appears successful, and Vino then offers 'None' as a
> > > sub-auth. This appears to fail which is rather odd :-)
> > > Could be a bug in either Vino or GTK-VNC handling of the
> > > 'none' type when used as a sub-auth scheme
> > It seems auth type and auth subtype are kind of reversed in the TLS
> > case. Attached patch makes things work for me.
> >  -- Guido
> 
> > diff --git a/src/vncconnection.c b/src/vncconnection.c
> > index e3835c9..69dd570 100644
> > --- a/src/vncconnection.c
> > +++ b/src/vncconnection.c
> > @@ -3832,7 +3832,7 @@ static gboolean vnc_connection_perform_auth_tls(VncConnection *conn)
> >  
> >  	if (priv->has_error)
> >  		return FALSE;
> > -	vnc_connection_choose_auth(conn, VNC_AUTH_CHOOSE_TYPE, nauth, auth);
> > +	vnc_connection_choose_auth(conn, VNC_AUTH_CHOOSE_SUBTYPE, nauth, auth);
> >  	if (priv->has_error)
> >  		return FALSE;
> 
> Yep, this is correct.
Great. Patch attached - please apply.

> >  
> > diff --git a/src/vncdisplay.c b/src/vncdisplay.c
> > index 65b8c3b..1f839cd 100644
> > --- a/src/vncdisplay.c
> > +++ b/src/vncdisplay.c
> > @@ -1104,7 +1104,7 @@ static void on_auth_choose_subtype(VncConnection *conn G_GNUC_UNUSED,
> >  	if (!subtypes->n_values)
> >  		return;
> >  
> > -	if (type == VNC_CONNECTION_AUTH_TLS) {
> > +	if (type != VNC_CONNECTION_AUTH_TLS) {
> >  		for (l = priv->preferable_auths; l; l=l->next) {
> >  			int pref = GPOINTER_TO_UINT (l->data);
> 
> This isn't right - we need to choose amongst our supported auth types.
> This code is actually pretty wrong - it should always run the loop and
> close the connection if no supported auth was found.
> 
> I think this patch should work better
> 
> diff --git a/src/vncconnection.c b/src/vncconnection.c
> index e3835c9..97a5648 100644
> --- a/src/vncconnection.c
> +++ b/src/vncconnection.c
> @@ -3832,7 +3832,7 @@ static gboolean vnc_connection_perform_auth_tls(VncConnection *conn)
>  
>  	if (priv->has_error)
>  		return FALSE;
> -	vnc_connection_choose_auth(conn, VNC_AUTH_CHOOSE_TYPE, nauth, auth);
> +	vnc_connection_choose_auth(conn, VNC_AUTH_CHOOSE_SUBTYPE, nauth, auth);
>  	if (priv->has_error)
>  		return FALSE;
>  
> diff --git a/src/vncdisplay.c b/src/vncdisplay.c
> index 65b8c3b..732349e 100644
> --- a/src/vncdisplay.c
> +++ b/src/vncdisplay.c
> @@ -1087,11 +1087,11 @@ static void on_auth_choose_type(VncConnection *conn G_GNUC_UNUSED,
>  		}
>  	}
>  
> -	GValue *type = g_value_array_get_nth(types, 0);
> -	vnc_connection_set_auth_type(priv->conn, g_value_get_enum(type));
> +	/* No sub-auth matching our supported auth so have to give up */
> +	vnc_connection_shutdown(conn);
>  }
>  
> -static void on_auth_choose_subtype(VncConnection *conn G_GNUC_UNUSED,
> +static void on_auth_choose_subtype(VncConnection *conn,
>  				   unsigned int type,
>  				   GValueArray *subtypes,
>  				   gpointer opaque)
> @@ -1101,25 +1101,29 @@ static void on_auth_choose_subtype(VncConnection *conn G_GNUC_UNUSED,
>  	GSList *l;
>  	guint i;
>  
> -	if (!subtypes->n_values)
> +	if (!subtypes->n_values) {
> +		vnc_connection_shutdown(conn);
>  		return;
> +	}
>  
> -	if (type == VNC_CONNECTION_AUTH_TLS) {
> -		for (l = priv->preferable_auths; l; l=l->next) {
> -			int pref = GPOINTER_TO_UINT (l->data);
> +	for (l = priv->preferable_auths; l; l=l->next) {
> +		int pref = GPOINTER_TO_UINT (l->data);
>  
> -			for (i=0; i< subtypes->n_values; i++) {
> -				GValue *subtype = g_value_array_get_nth(subtypes, i);
> -				if (pref == g_value_get_enum(subtype)) {
> -					vnc_connection_set_auth_type(priv->conn, pref);
> -					return;
> -				}
> +		/* Don't want to recursively do the same major auth */
> +		if (pref == type)
> +			continue;
> +
> +		for (i=0; i< subtypes->n_values; i++) {
> +			GValue *subtype = g_value_array_get_nth(subtypes, i);
> +			if (pref == g_value_get_enum(subtype)) {
> +				vnc_connection_set_auth_type(conn, pref);
> +				return;
>  			}
>  		}
>  	}
>  
> -	GValue *subtype = g_value_array_get_nth(subtypes, 0);
> -	vnc_connection_set_auth_subtype(priv->conn, g_value_get_enum(subtype));
> +	/* No sub-auth matching our supported auth so have to give up */
> +	vnc_connection_shutdown(conn);
>  }
>  
>  static void on_auth_failure(VncConnection *conn G_GNUC_UNUSED,
This doesn't work here:

(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Init VncConnection=0x8966088
(gvncviewer:29079): gtk-vnc-DEBUG: vncdisplaykeymap.c Using evdev keycode mapping
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Open host=localhost port=5900
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Open coroutine starting
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Started background coroutine
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Resolving host localhost 5900
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Trying socket 6
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Emit main context 12
Connected to server
(gvncviewer:29079): gtk-vnc-DEBUG: vncdisplay.c Connected to VNC server
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Protocol initialization
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Server version: 3.7
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Using version: 3.7
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Possible auth 18
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Possible auth 1
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Emit main context 11
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Thinking about auth type 18
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Decided on auth type 18
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Waiting for auth type
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Choose auth 18
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Do TLS handshake
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Handshake was blocking
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Handshake was blocking
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Handshake was blocking
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Handshake done
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Completed TLS setup
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Got 1 subauths
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Got 1 subauths
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Possible sub-auth 1
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Emit main context 10
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Thinking about auth type 1
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Auth failed
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Doing final VNC cleanup
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Close VncConnection=0x8966088
(gvncviewer:29079): gvnc-DEBUG: vncconnection.c Emit main context 14
(gvncviewer:29079): gtk-vnc-DEBUG: vncdisplay.c Disconnected from VNC server
Disconnected from server

I'll try to have a close look later this week.
 -- Guido