From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TakzP-0004pZ-Jh for qemu-devel@nongnu.org; Tue, 20 Nov 2012 05:23:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TakzG-0008PR-IO for qemu-devel@nongnu.org; Tue, 20 Nov 2012 05:23:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:23172) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TakzG-0008PA-Av for qemu-devel@nongnu.org; Tue, 20 Nov 2012 05:23:26 -0500 Date: Tue, 20 Nov 2012 10:23:09 +0000 From: "Daniel P. Berrange" Message-ID: <20121120102309.GG3461@redhat.com> References: <1353403318-2877-1-git-send-email-thardeck@suse.de> <20121120094734.GD3461@redhat.com> <50AB590F.90203@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <50AB590F.90203@suse.de> Subject: Re: [Qemu-devel] [PATCH v2] vnc: added initial websockets support Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tim Hardeck Cc: aliguori@us.ibm.com, stefanha@gmail.com, github@martintribe.org, qemu-devel@nongnu.org, alevy@redhat.com, kraxel@redhat.com, corentin.chary@gmail.com 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= 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 :|