Re: [PATCH] ui/vnc.c: Allow websocket connections over AF_UNIX sockets

[Daniel P . Berrangé]

On Fri, Dec 02, 2022 at 04:12:04PM +0100, Pierre-Yves Ritschard wrote:
> Hi,
> 
> The provided patch allows the VNC websocket server of a qemu process to
> be provided over AF_UNIX as it is already possible for standard TCP VNC
> servers.
> 
> Now that many clients support websocket connections, some exclusively,
> it can be useful to expose the VNC server. One such case is when a
> proxy is present on a host machine, allowing it to proxy to a
> deterministic location withouth having to keep track of port mappings.
> 
> Removing the condition as is done in the provided patch creates a 
> functional server. If there's a downside to this approach I could not
> figure it out while reading the code. My hunch was that the condition
> was there for a reason, but it did not jump at me.
> 
> Cheers,
>   - pyr
> 
> ---
>  ui/vnc.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git ui/vnc.c ui/vnc.c
> index 88f55cbf3c..b01a655400 100644
> --- ui/vnc.c
> +++ ui/vnc.c
> @@ -3729,11 +3729,6 @@ static int vnc_display_get_address(const char *addrstr,
>          addr->type = SOCKET_ADDRESS_TYPE_UNIX;
>          addr->u.q_unix.path = g_strdup(addrstr + 5);
>  
> -        if (websocket) {
> -            error_setg(errp, "UNIX sockets not supported with websock");
> -            goto cleanup;
> -        }
> -

Allowing websockets is fine, but just removing this check is not
sufficient

The 'websocket=XXXX' parameter for -vnc takes two formats

   websocket=on|off

or

   websocket=portnum

In the case of on|off, the code takes the original VNC display
num and listens on   5700 + display for websockets, 590 + display
for non-websockets.

In the case of a explicit port, the code listens on that port.

Also we fail to actually handle 'off' correctly, just treating
it as a named port

$ qemu-system-x86_64  -vnc :1,websocket=off
qemu-system-x86_64: -vnc :1,websocket=off: address resolution failed for :off: 
Servname not supported for ai_socktype


Anyway given an argument

   -vnc  unix:/some/path,websocket=on

this cause causes QEMU to listen on a relative path 'on'. We need
to define what the semantics for websockets=on are going to be
for UNIX sockets. Should it append '.ws' to the main path ? Should
we just not allow websockets=on and document it must be an explicit
path at all times ?

We also need to document this in qemu-options.hx.

>          if (to) {
>              error_setg(errp, "Port range not support with UNIX socket");
>              goto cleanup;
> -- 
> 2.38.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|