All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Tim Hardeck <thardeck@suse.de>
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
Subject: Re: [Qemu-devel] [PATCH v2] vnc: added initial websockets support
Date: Tue, 20 Nov 2012 10:23:09 +0000	[thread overview]
Message-ID: <20121120102309.GG3461@redhat.com> (raw)
In-Reply-To: <50AB590F.90203@suse.de>

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

      reply	other threads:[~2012-11-20 10:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-20  9:21 [Qemu-devel] [PATCH v2] vnc: added initial websockets support Tim Hardeck
2012-11-20  9:47 ` Daniel P. Berrange
2012-11-20 10:18   ` Tim Hardeck
2012-11-20 10:23     ` Daniel P. Berrange [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121120102309.GG3461@redhat.com \
    --to=berrange@redhat.com \
    --cc=alevy@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=corentin.chary@gmail.com \
    --cc=github@martintribe.org \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=thardeck@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.