Re: Connecting to vino fails

["Daniel P. Berrange" <dan berrange com>]

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.

>  
> 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,


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|