[MIR] spice

[Serge Hallyn]

Source package: spice

Availability: Currently in universe and in debian
Rationale: Many users wantn to use qemu-kvm with spice for fast, high-quality
  graphics, including unity.
Security: no security history for the server (which the MIR is for)
QA: package works out of the box with no prompting. There are no major bugs in Debian or Ubuntu.
Standards Compliance: FHS and Debian Policy compliant.
Maintenance: The server team will help to maintain this package, which we hope
  to mainly keep in sync with debian.
Dependencies: All are in main

[Jamie Strandboge (jdstrand)]

MIR review:

 * Does it FTBFS currently? build fine
 * Does it have a test suite? yes. 'Automated tests: no'
 * Does Ubuntu carry a delta? we have a delta, but it is so that we have a newer version
 * If it's a library, does it either have a symbols file or use an empty argument to dh_makeshlibs -V? has a symbols file
 * Does it have a watch file? yes
 * Is its update history slow or sporadic? it is fairly slow, but not unacceptably so
 * Is the current release packaged? no. 0.12.2 is available in Debian experimental
 * Will entering main make it harder for the people currently keeping it up to date? not really, the server team is taking care of it
 * Lintian clean
 * debian/rules is clean
 * Errors/warnings during the build (audit-log.sh <logfile>)
  * has the following compiler warnings. should fix these:
backtrace.c:81:16: warning: ignoring return value of 'seteuid', declared with attribute warn_unused_result [-Wunused-result]
lz_compress_tmpl.c:486:18: warning: assignment from incompatible pointer type [enabled by default]
lz_compress_tmpl.c:486:18: warning: assignment from incompatible pointer type [enabled by default]
lz_compress_tmpl.c:486:18: warning: assignment from incompatible pointer type [enabled by default]
lz_compress_tmpl.c:486:18: warning: assignment from incompatible pointer type [enabled by default]
lz_compress_tmpl.c:486:18: warning: assignment from incompatible pointer type [enabled by default]
lz_compress_tmpl.c:486:18: warning: assignment from incompatible pointer type [enabled by default]
 * Incautious use of malloc/sprintf: server is ok
 * Uses of sudo (see audit-packaging.sh) or LD_LIBRARY_PATH (see audit-code.sh): no
 * Important bugs (crashers, etc) in Debian or Ubuntu: none

[Jamie Strandboge (jdstrand)]

After a high-level limited review:
 * no initscripts, no use of sudo/pkexec/gksudo, no cron jobs, no daemons or
setuid/fscaps, no dbus services
 * is memory management sane?
  - client: not particularly defensive. lots of string operations are not
    checking for return codes
  - server: is defensive - aborts on memory allocations in spice_malloc() and
    that is used almost everywhere (where it isn't, we check the return code)
 * spice_logv() is not verifying the environment variables it is honoring but doesn't seem to be abusable
 * There is setuid(0) call which underscores that the server code can be running as root within qemu, so this code is sensitive. However, the supported virtualization solution for qemu-kvm is to run under libvirt, which provides apparmor confinement by default and doesn't run qemu as root by default. Regular users typically won't run qemu as root when used alone
 * is there any use of encryption? yes. it seems to be checking return codes and doing ssl verification correctly
 * has a predictable filenames in:
  - client/playback_channel.cpp: sprintf(file_name, "/tmp/%u.wav", ++file_id);
  - spice-common/common/canvas_base.c: sprintf(file_str, "/tmp/spice_dump/%u.jpg", id);
  - ... etc

I'd prefer spice-client stay out of main. The server code has some areas of improvement, but it is defensively programmed for the most part, the code will be run confined via AppArmor and upstream (RedHat) seems committed to the project.

Conditional ACK once compiler warnings are addressed and enabling the test suite is investigated/implemented. It would be good to address the /tmp filename issues even though we've got symlink restrictions in Ubuntu. At a minimum these should reported upstream or shown to not be an issue.

[Serge Hallyn (serge-hallyn)]

Unfortunately building the automated tests (--enable-automated-tests) requires snappy to be installed, which is in universe (and huge).

[Serge Hallyn (serge-hallyn)]

The compiler fixes are proposed at https://code.launchpad.net/~serge-hallyn/ubuntu/raring/spice/spice-compiler-warnings/+merge/146220

[Serge Hallyn (serge-hallyn)]

The un-checked seteuid(0) call is apparently a feature. It was copied as is out of the xserver code, and, particularly with the spice server, I assume that (a) the task to be traced is owned by the same user and so (b) uid 0 is not actually necessary.

Furthermore, as Jamie pointed out there is no gstack in the archive :)

So I've pushed a new merge proposal which removes the change to seteuid(0) failure handling.

I will look a bit more into the tests and see if any can be usefully run without snappy.

[Serge Hallyn (serge-hallyn)]

The last two revisions at

https://code.launchpad.net/~serge-hallyn/ubuntu/raring/spice/spice-compiler-warnings/

cause the one actual automated test to build+run. A build in ppa can be seen at https://launchpad.net/~serge-hallyn/+archive/virt/+build/4268753

[Serge Hallyn (serge-hallyn)]

I've cleaned up the remaining pointer mismatches in the test code. However the resulting test case still is hit-or-miss in ppas. It succeeds locally, and sometimes in ppas, but has been seen to segfault as well as simply be killed when run in ppa.

[Serge Hallyn (serge-hallyn)]

(runs and package can be seen in ppa:serge-hallyn/kernel, and source is at http://bazaar.launchpad.net/~serge-hallyn/ubuntu/raring/spice/spice-fixed2/revision/18

[Serge Hallyn (serge-hallyn)]

The patches to address the warnings have made their way upstream and into debian, and the debian package has been synced into raring.

[Jamie Strandboge (jdstrand)]

ACK based on irc feedback. Please feel free to move to main (except spice-client).

[Jamie Strandboge (jdstrand)]

Override component to main
libspice-server-dev 0.12.2-0nocelt2exp in raring amd64: universe/libdevel/optional -> main
libspice-server-dev 0.12.2-0nocelt2exp in raring i386: universe/libdevel/optional -> main
libspice-server1 0.12.2-0nocelt2exp in raring amd64: universe/libs/optional -> main
libspice-server1 0.12.2-0nocelt2exp in raring i386: universe/libs/optional -> main
Override [y|N]? y
4 publications overridden.

[Adam Conrad (adconrad)]

Override component to main
spice 0.12.2-0nocelt2exp in raring: universe/misc -> main
Override [y|N]? y
1 publication overridden.