Re: [Qemu-devel] [PATCH v2] vnc: added initial websockets support

[Daniel P. Berrange]

On Tue, Nov 20, 2012 at 11:18:55AM +0100, Tim Hardeck wrote:
> On 11/20/2012 10:47 AM, Daniel P. Berrange wrote:
> > On Tue, Nov 20, 2012 at 10:21:58AM +0100, Tim Hardeck wrote:
> >> This patch adds basic Websockets version 13 - RFC 6455 - support to QEMU
> >> VNC. Binary encoding support on the client side is required.
> >>
> >> Because of the GnuTLS requirement the Websockets implementation is
> >> optional (--enable-vnc-ws).
> >>
> >> To activate Websockets during runtime the VNC option "websocket" is
> >> used, for example "-vnc :0,websocket" would activate Websockets.
> >> The port for Websockets connections is (5700 + display) so if QEMU VNC
> >> is started with :0 the websockets port would be 5700.
> > 
> > We need to be able to specify an explicit port number for the websockets
> > listen address, separately from the main VNC port number. This automatic
> > pick of websockets port might be nice for a user starting QEMU manually,
> > but for management apps we need full direct control.
> Ok, this should be no problem to add something like websocket=<port> but
> I just thought that a correlation between the vnc and websocket port
> would be quite useful especially when several QEMU instances are run on
> one machine.
> Would it be Ok to keep the correlation if no port is specified?

Yep, like I said, its useful for users starting QEMU directly. We
just need to make sure there's an override.

> >> Changes to v1
> >> * removed automatic websocket recognition
> >> * added new lwebsock socket on port 5700 + display when the vnc option
> >>   "websocket" is passed on
> >> * adapted vnc_connect vnc_listen_read to differ between websocket
> >> * added separate event handler to read the Websocket handshake
> >>
> >> Would it be Ok to use a public domain SHA1 implementation like
> >> tests/tcg/sha1.c and if so where should the sha1.c be stored?
> >> Without the GnuTLS dependency would it be Ok to make Websockets not
> >> optional because this would clean up the patch/code quite a bit?
> > 
> > IMHO using gcrypt/GNUTLS is a good thing. Creating our own copies of
> > encryption algorithms in source tree causes significant complications
> > getting QEMU security certified. GNUTLS is a common enough crypto
> > library that I don't think it is unreasonably onerous to expect it to
> > be used for WebSockets. It even works fine on Windows.
> That's one of the reasons why I have used GnuTLS but it is quite a huge
> dependency for just one algorithm and the many ifdefs don't really help
> the code quality.

One of the things I'd like to try todo for QEMU is to have an centralized
internal interface for crypto functions. eg, so you could say

    h = crypt_hash_init("sha1");

    ...

and the actual impl of crypt_hash_init() whould then be backed by GNUTLS,
OpenSSL, or the Linux kernel (via socket AF_ALG family). This would let
all the #ifdef code be centralized in one place, making the VNC code
clean again. So while I agree with you that its ugly, I think that's
OK in the short term

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