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