diff mbox

[STABLE,2.5] vnc: fix VNC websockets TLS handshake

Message ID 1465309053-5784-1-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé June 7, 2016, 2:17 p.m. UTC
If the VNC websockets TLS handshake blocked, we register a
callback to continue it after the socket signals data is
available. Unfortunately we were mistakenly registering
the wrong method callback, causing us to restart the TLS
handshake from the begining, which obviously fails.

Reported-by: Ben Aitchison <ben@meh.net.nz>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 ui/vnc-ws.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

NB this is a stable-2.5 only patch, with no equiv in master
since the code was totally refactored in 2.6

Comments

Eric Blake June 7, 2016, 2:47 p.m. UTC | #1
On 06/07/2016 08:17 AM, Daniel P. Berrange wrote:
> If the VNC websockets TLS handshake blocked, we register a
> callback to continue it after the socket signals data is
> available. Unfortunately we were mistakenly registering
> the wrong method callback, causing us to restart the TLS
> handshake from the begining, which obviously fails.

s/begining/beginning/

> 
> Reported-by: Ben Aitchison <ben@meh.net.nz>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  ui/vnc-ws.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> NB this is a stable-2.5 only patch, with no equiv in master
> since the code was totally refactored in 2.6

Reviewed-by: Eric Blake <eblake@redhat.com>
Daniel P. Berrangé Sept. 14, 2016, 3:02 p.m. UTC | #2
ping

Is qemu-stable dead for the 2.5 branch or was this just
missed ?

On Tue, Jun 07, 2016 at 03:17:33PM +0100, Daniel P. Berrange wrote:
> If the VNC websockets TLS handshake blocked, we register a
> callback to continue it after the socket signals data is
> available. Unfortunately we were mistakenly registering
> the wrong method callback, causing us to restart the TLS
> handshake from the begining, which obviously fails.
> 
> Reported-by: Ben Aitchison <ben@meh.net.nz>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  ui/vnc-ws.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> NB this is a stable-2.5 only patch, with no equiv in master
> since the code was totally refactored in 2.6
> 
> diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
> index 175ea50..a8cf180 100644
> --- a/ui/vnc-ws.c
> +++ b/ui/vnc-ws.c
> @@ -22,8 +22,9 @@
>  #include "qemu/main-loop.h"
>  #include "crypto/hash.h"
>  
> -static int vncws_start_tls_handshake(VncState *vs)
> +static void vncws_run_tls_handshake(void *opaque)
>  {
> +    VncState *vs = (VncState *)opaque;
>      Error *err = NULL;
>  
>      if (qcrypto_tls_session_handshake(vs->tls, &err) < 0) {
> @@ -42,22 +43,21 @@ static int vncws_start_tls_handshake(VncState *vs)
>  
>      case QCRYPTO_TLS_HANDSHAKE_RECVING:
>          VNC_DEBUG("Handshake interrupted (blocking read)\n");
> -        qemu_set_fd_handler(vs->csock, vncws_tls_handshake_io, NULL, vs);
> +        qemu_set_fd_handler(vs->csock, vncws_run_tls_handshake, NULL, vs);
>          break;
>  
>      case QCRYPTO_TLS_HANDSHAKE_SENDING:
>          VNC_DEBUG("Handshake interrupted (blocking write)\n");
> -        qemu_set_fd_handler(vs->csock, NULL, vncws_tls_handshake_io, vs);
> +        qemu_set_fd_handler(vs->csock, NULL, vncws_run_tls_handshake, vs);
>          break;
>      }
>  
> -    return 0;
> +    return;
>  
>   error:
>      VNC_DEBUG("Handshake failed %s\n", error_get_pretty(err));
>      error_free(err);
>      vnc_client_error(vs);
> -    return -1;
>  }
>  
>  void vncws_tls_handshake_io(void *opaque)
> @@ -84,7 +84,7 @@ void vncws_tls_handshake_io(void *opaque)
>                                        vs);
>  
>      VNC_DEBUG("Start TLS WS handshake process\n");
> -    vncws_start_tls_handshake(vs);
> +    vncws_run_tls_handshake(vs);
>  }
>  
>  void vncws_handshake_read(void *opaque)
> -- 
> 2.5.5
> 

Regards,
Daniel
Michael Roth Sept. 20, 2016, 4:49 p.m. UTC | #3
Quoting Daniel P. Berrange (2016-09-14 10:02:10)
> ping
> 
> Is qemu-stable dead for the 2.5 branch or was this just
> missed ?

The stable branches generally die once the next major release lands and
I switch over to supporting the new stable tree. Generally there's x.y.1
stable release mid-development cycle for the next major release, and,
depending on the severity/number of fixes or regressions in the x.y.1
release, an x.y.2 that overlaps with the x.y+1.0 major release. There
may be any number of x.y.z.n CVE releases in between, but generally
even for these we still drop support once the next major release has
firmly landed and leave it to the downstreams to handle any LTS beyond
that.

> 
> On Tue, Jun 07, 2016 at 03:17:33PM +0100, Daniel P. Berrange wrote:
> > If the VNC websockets TLS handshake blocked, we register a
> > callback to continue it after the socket signals data is
> > available. Unfortunately we were mistakenly registering
> > the wrong method callback, causing us to restart the TLS
> > handshake from the begining, which obviously fails.
> > 
> > Reported-by: Ben Aitchison <ben@meh.net.nz>
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  ui/vnc-ws.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > NB this is a stable-2.5 only patch, with no equiv in master
> > since the code was totally refactored in 2.6
> > 
> > diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
> > index 175ea50..a8cf180 100644
> > --- a/ui/vnc-ws.c
> > +++ b/ui/vnc-ws.c
> > @@ -22,8 +22,9 @@
> >  #include "qemu/main-loop.h"
> >  #include "crypto/hash.h"
> >  
> > -static int vncws_start_tls_handshake(VncState *vs)
> > +static void vncws_run_tls_handshake(void *opaque)
> >  {
> > +    VncState *vs = (VncState *)opaque;
> >      Error *err = NULL;
> >  
> >      if (qcrypto_tls_session_handshake(vs->tls, &err) < 0) {
> > @@ -42,22 +43,21 @@ static int vncws_start_tls_handshake(VncState *vs)
> >  
> >      case QCRYPTO_TLS_HANDSHAKE_RECVING:
> >          VNC_DEBUG("Handshake interrupted (blocking read)\n");
> > -        qemu_set_fd_handler(vs->csock, vncws_tls_handshake_io, NULL, vs);
> > +        qemu_set_fd_handler(vs->csock, vncws_run_tls_handshake, NULL, vs);
> >          break;
> >  
> >      case QCRYPTO_TLS_HANDSHAKE_SENDING:
> >          VNC_DEBUG("Handshake interrupted (blocking write)\n");
> > -        qemu_set_fd_handler(vs->csock, NULL, vncws_tls_handshake_io, vs);
> > +        qemu_set_fd_handler(vs->csock, NULL, vncws_run_tls_handshake, vs);
> >          break;
> >      }
> >  
> > -    return 0;
> > +    return;
> >  
> >   error:
> >      VNC_DEBUG("Handshake failed %s\n", error_get_pretty(err));
> >      error_free(err);
> >      vnc_client_error(vs);
> > -    return -1;
> >  }
> >  
> >  void vncws_tls_handshake_io(void *opaque)
> > @@ -84,7 +84,7 @@ void vncws_tls_handshake_io(void *opaque)
> >                                        vs);
> >  
> >      VNC_DEBUG("Start TLS WS handshake process\n");
> > -    vncws_start_tls_handshake(vs);
> > +    vncws_run_tls_handshake(vs);
> >  }
> >  
> >  void vncws_handshake_read(void *opaque)
> > -- 
> > 2.5.5
> > 
> 
> Regards,
> 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 :|
>
diff mbox

Patch

diff --git a/ui/vnc-ws.c b/ui/vnc-ws.c
index 175ea50..a8cf180 100644
--- a/ui/vnc-ws.c
+++ b/ui/vnc-ws.c
@@ -22,8 +22,9 @@ 
 #include "qemu/main-loop.h"
 #include "crypto/hash.h"
 
-static int vncws_start_tls_handshake(VncState *vs)
+static void vncws_run_tls_handshake(void *opaque)
 {
+    VncState *vs = (VncState *)opaque;
     Error *err = NULL;
 
     if (qcrypto_tls_session_handshake(vs->tls, &err) < 0) {
@@ -42,22 +43,21 @@  static int vncws_start_tls_handshake(VncState *vs)
 
     case QCRYPTO_TLS_HANDSHAKE_RECVING:
         VNC_DEBUG("Handshake interrupted (blocking read)\n");
-        qemu_set_fd_handler(vs->csock, vncws_tls_handshake_io, NULL, vs);
+        qemu_set_fd_handler(vs->csock, vncws_run_tls_handshake, NULL, vs);
         break;
 
     case QCRYPTO_TLS_HANDSHAKE_SENDING:
         VNC_DEBUG("Handshake interrupted (blocking write)\n");
-        qemu_set_fd_handler(vs->csock, NULL, vncws_tls_handshake_io, vs);
+        qemu_set_fd_handler(vs->csock, NULL, vncws_run_tls_handshake, vs);
         break;
     }
 
-    return 0;
+    return;
 
  error:
     VNC_DEBUG("Handshake failed %s\n", error_get_pretty(err));
     error_free(err);
     vnc_client_error(vs);
-    return -1;
 }
 
 void vncws_tls_handshake_io(void *opaque)
@@ -84,7 +84,7 @@  void vncws_tls_handshake_io(void *opaque)
                                       vs);
 
     VNC_DEBUG("Start TLS WS handshake process\n");
-    vncws_start_tls_handshake(vs);
+    vncws_run_tls_handshake(vs);
 }
 
 void vncws_handshake_read(void *opaque)