Re: [Qemu-devel] SPICE session's connection_id's are not unique

[Daniel P . Berrangé]

On Wed, Jan 30, 2019 at 06:29:04PM +0300, Michael Tokarev wrote:
> Forwarding to qemu-devel@ from http://bugs.debian.org/920897
> 
> Thanks,
> 
> /mjt
> 
> -------- Forwarded message --------
> Subject:      Bug#920897: SPICE session's connection_id's are not unique
> Date:         Wed, 30 Jan 2019 11:21:54 +0000
> From:         Philip Pum <address@hidden>


> When creating a virtual machine with qemu (e.g. via libvirt) including a 
> SPICE server,
> the client_id of the SPICE session is not unique. For example, starting 
> multiple
> virtual machines on the same libvirtd, the client_id is the same for all 
> virtual
> machine's SPICE sessions.
> 
> 
> A description of the client_id can be found in
> 
> https://www.spice-space.org/static/docs/spice_protocol.pdf under section 
> 2.11. c) :
> 
> 
> "UINT32 connection_id - In case of a new session (i.e., channel type is 
> RED_CHANNEL_MAIN)
>  this field is set to zero, and in response the server will allocate session 
> id and will
>  send it via the RedLinkReply message. In case of all other channel types, 
> this field 
>  will be equal to the allocated session id"

IIUC, that is just claiming that connection_id will be unique within the
scope of clients connected to a single SPICE server.

> The relevant code for generating client ids in libspice-server1 can be
> found here: 
> https://gitlab.freedesktop.org/spice/spice/blob/v0.12.8/server/reds.c#L1614
> 
> This uses rand() to generate the random id, but qemu (at least in the
> case of qemu-system-x86) fails to initialize the RNG seed (with e.g. srand()).
> 
> The result is, that every SPICE session started (by e.g. libvirtd) has
> the same client_id. Usually, this is not a problem, but running something
> like a SPICE proxy, relying on the client_id to correctly route connections,
> this creates problems.

Presumably this means s/client_id/connection_id/ here given the quote
3 paragraphs earlier.

Each QEMU process has its own SPICE server, so the SPICE protocol spec
above does not guarantee that cconnection_id  is unique between
different QEMU process - only unique within multiple connections to
a single QEMU process, which I think is achieved even with the missing
srand() call.

Distinguishing different clients is a security critical task for a
SPICE proxy. I'm not sure I'd be comfortable with security of relying
on the 32-bit integer identifier to be unique across every SPICE server
in every QEMU when there are 100's of VMs on a host, even if QEMU did
call srand() correctly.

None the less, I'd suggest that spice server stop usin rand() entirely.
It already links to openssl, so I'd suggest they request a random value
from openssl which can provide cryptographically strong random values.

> Adding something like 'srand(time(NULL));' to qemu (in vl.c) solves this
> issue. Related (as seen in some VNC patches, e.g.
> 'CVE-2017-15124/04-ui-avoid-pointless-VNC-updates-if-framebuffer-isn-t-.patch/ui/vnc.c'
>  ):  srand(time(NULL)+getpid()+getpid()*987654+rand());

We should probably move the srand() call into vl.c in QEMU. At the same
time though, we should initialize it with a cryptoraphically strong
input we get from qcrypto_random_bytes(). Again though, we should purge
any code which uses rand() from QEMU, in favour of using qcrypto_random_bytes
directly as that#s backed by a cryptographically strong source.

Fortnuately this use of rand() is not a security issue for VNC, since
the VNC authentication scheme is already fundamentally broken by
design and should never be used. The VNC TLS + SASL extensions
provide a real auth scheme for VNC.

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