* Re: [Qemu-devel] SPICE session's connection_id's are not unique
2019-01-30 15:29 ` [Qemu-devel] SPICE session's connection_id's are not unique Michael Tokarev
@ 2019-01-30 15:50 ` Daniel P. Berrangé
0 siblings, 0 replies; 2+ messages in thread
From: Daniel P. Berrangé @ 2019-01-30 15:50 UTC (permalink / raw)
To: Michael Tokarev; +Cc: qemu-devel
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 <Philip.Pum@radarservices.com>
> 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 :|
^ permalink raw reply [flat|nested] 2+ messages in thread