Hi Philipp, thanks for looking into this!
On Thu, Mar 24, 2016 at 05:51:21PM +0100, Philipp Hahn wrote: > diff -Nru libvirt-1.2.9/debian/changelog libvirt-1.2.9/debian/changelog > --- libvirt-1.2.9/debian/changelog 2015-08-26 08:34:22.000000000 +0200 > +++ libvirt-1.2.9/debian/changelog 2016-03-05 08:18:07.000000000 +0100 > @@ -1,3 +1,13 @@ > +libvirt (1.2.9-9+deb8u2) jessie; urgency=medium > + > + * Non-maintainer upload. > + * Fix CVE-2015-5313 (Closes: #808273) > + * libvirt-daemon: Expects qemu-bridge-helper in /usr/libexec/ > + (Closes: #816602) > + * Fix several FTBFS errors Could you elaborate on the FTBFS errors? I just rebuilt the current jessie version without any changes so "Fix several FTBFS errors" sounds very vague and exaggeratet. We already had a point release in Jessie so I wonder how these would get introduced? > + > + -- Philipp Matthias Hahn <[email protected]> Fri, 04 Mar 2016 12:01:36 > +0100 > + > libvirt (1.2.9-9+deb8u1) jessie; urgency=medium > > [ Guido Günther ] > diff -Nru libvirt-1.2.9/debian/control libvirt-1.2.9/debian/control > --- libvirt-1.2.9/debian/control 2015-08-24 16:20:54.000000000 +0200 > +++ libvirt-1.2.9/debian/control 2016-03-04 13:42:30.000000000 +0100 > @@ -5,6 +5,7 @@ > Uploaders: Guido Günther <[email protected]>, Laurent Léonard > <[email protected]> > Build-Depends: > debhelper (>= 7), > + dh-autoreconf, > dh-systemd (>= 1.18~), > libxml2-dev, > libncurses5-dev, > diff -Nru > libvirt-1.2.9/debian/patches/debian/Debianize-bridge-helper-path.patch > libvirt-1.2.9/debian/patches/debian/Debianize-bridge-helper-path.patch > --- libvirt-1.2.9/debian/patches/debian/Debianize-bridge-helper-path.patch > 1970-01-01 01:00:00.000000000 +0100 > +++ libvirt-1.2.9/debian/patches/debian/Debianize-bridge-helper-path.patch > 2016-03-05 08:18:07.000000000 +0100 > @@ -0,0 +1,42 @@ > +libvirt-daemon: Expects qemu-bridge-helper in /usr/libexec/ > + > +$ strings /usr/lib/libvirt/connection-driver/libvirt_driver_qemu.so | grep > bridge-helper > +/usr/libexec/qemu-bridge-helper > + > +$ dpkg -S bridge-helper > +qemu-system-common: /usr/lib/qemu/qemu-bridge-helper > + > +Closes #816602 > +--- a/src/qemu/qemu.conf > ++++ b/src/qemu/qemu.conf > +@@ -357,7 +357,7 @@ > + # is used to create <source type='bridge'> interfaces when libvirtd is > + # running unprivileged. libvirt invokes the helper directly, instead > + # of using "-netdev bridge", for security reasons. > +-#bridge_helper = "/usr/libexec/qemu-bridge-helper" > ++#bridge_helper = "/usr/lib/qemu/qemu-bridge-helper" > + > + > + > +--- a/src/qemu/qemu_conf.c > ++++ b/src/qemu/qemu_conf.c > +@@ -244,7 +244,7 @@ virQEMUDriverConfigPtr virQEMUDriverConf > + goto error; > + } > + > +- if (VIR_STRDUP(cfg->bridgeHelperName, > "/usr/libexec/qemu-bridge-helper") < 0) > ++ if (VIR_STRDUP(cfg->bridgeHelperName, > "/usr/lib/qemu/qemu-bridge-helper") < 0) > + goto error; > + > + cfg->clearEmulatorCapabilities = true; > +--- a/src/qemu/test_libvirtd_qemu.aug.in > ++++ b/src/qemu/test_libvirtd_qemu.aug.in > +@@ -56,7 +56,7 @@ module Test_libvirtd_qemu = > + { "auto_dump_bypass_cache" = "0" } > + { "auto_start_bypass_cache" = "0" } > + { "hugetlbfs_mount" = "/dev/hugepages" } > +-{ "bridge_helper" = "/usr/libexec/qemu-bridge-helper" } > ++{ "bridge_helper" = "/usr/lib/qemu/qemu-bridge-helper" } > + { "clear_emulator_capabilities" = "1" } > + { "set_process_name" = "1" } > + { "max_processes" = "0" } > diff -Nru libvirt-1.2.9/debian/patches/Disable-failing-virnetsockettest.patch > libvirt-1.2.9/debian/patches/Disable-failing-virnetsockettest.patch > --- libvirt-1.2.9/debian/patches/Disable-failing-virnetsockettest.patch > 2015-08-24 16:20:54.000000000 +0200 > +++ libvirt-1.2.9/debian/patches/Disable-failing-virnetsockettest.patch > 2016-03-04 14:47:12.000000000 +0100 > @@ -7,11 +7,25 @@ > tests/virnetsockettest.c | 2 ++ > 1 file changed, 2 insertions(+) > > -diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c > -index 5d91f26..1f283a3 100644 > --- a/tests/virnetsockettest.c > +++ b/tests/virnetsockettest.c > -@@ -501,10 +501,12 @@ mymain(void) > +@@ -333,6 +333,7 @@ static int testSocketUNIXAddrs(const voi > + return ret; > + } > + > ++#if 0 > + static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) > + { > + virNetSocketPtr csock = NULL; /* Client socket */ > +@@ -383,6 +384,7 @@ static int testSocketCommandFail(const v > + virObjectUnref(csock); > + return ret; > + } > ++#endif Why did you disable this one? > + > + struct testSSHData { > + const char *nodename; > +@@ -501,10 +503,12 @@ mymain(void) > if (virtTestRun("Socket UNIX Addrs", testSocketUNIXAddrs, NULL) < 0) > ret = -1; > > diff -Nru > libvirt-1.2.9/debian/patches/security/CVE-2015-5313-storage-don-t-allow-in-filesystem-volu.patch > > libvirt-1.2.9/debian/patches/security/CVE-2015-5313-storage-don-t-allow-in-filesystem-volu.patch > --- > libvirt-1.2.9/debian/patches/security/CVE-2015-5313-storage-don-t-allow-in-filesystem-volu.patch > 1970-01-01 01:00:00.000000000 +0100 > +++ > libvirt-1.2.9/debian/patches/security/CVE-2015-5313-storage-don-t-allow-in-filesystem-volu.patch > 2016-03-04 14:46:20.000000000 +0100 > @@ -0,0 +1,72 @@ > +From 034e47c338b13a95cf02106a3af912c1c5f818d7 Mon Sep 17 00:00:00 2001 > +Message-Id: > <034e47c338b13a95cf02106a3af912c1c5f818d7.1457088964.git.h...@univention.de> > +From: Eric Blake <[email protected]> > +Date: Tue, 8 Dec 2015 17:46:31 -0700 > +Subject: [PATCH] CVE-2015-5313: storage: don't allow '/' in filesystem volume > + names > +Organization: Univention GmbH, Bremen, Germany > +To: [email protected] > + > +The libvirt file system storage driver determines what file to > +act on by concatenating the pool location with the volume name. > +If a user is able to pick names like "../../../etc/passwd", then > +they can escape the bounds of the pool. For that matter, > +virStoragePoolListVolumes() doesn't descend into subdirectories, > +so a user really shouldn't use a name with a slash. > + > +Normally, only privileged users can coerce libvirt into creating > +or opening existing files using the virStorageVol APIs; and such > +users already have full privilege to create any domain XML (so it > +is not an escalation of privilege). But in the case of > +fine-grained ACLs, it is feasible that a user can be granted > +storage_vol:create but not domain:write, and it violates > +assumptions if such a user can abuse libvirt to access files > +outside of the storage pool. > + > +Therefore, prevent all use of volume names that contain "/", > +whether or not such a name is actually attempting to escape the > +pool. > + > +This changes things from: > + > +$ virsh vol-create-as default ../../../../../../etc/haha --capacity 128 > +Vol ../../../../../../etc/haha created > +$ rm /etc/haha > + > +to: > + > +$ virsh vol-create-as default ../../../../../../etc/haha --capacity 128 > +error: Failed to create vol ../../../../../../etc/haha > +error: Requested operation is not valid: volume name > '../../../../../../etc/haha' cannot contain '/' > + > +Signed-off-by: Eric Blake <[email protected]> > +--- > + src/storage/storage_backend_fs.c | 10 +++++++++- > + 1 file changed, 9 insertions(+), 1 deletion(-) > + > +--- a/src/storage/storage_backend_fs.c > ++++ b/src/storage/storage_backend_fs.c > +@@ -1,7 +1,7 @@ > + /* > + * storage_backend_fs.c: storage backend for FS and directory handling > + * > +- * Copyright (C) 2007-2014 Red Hat, Inc. > ++ * Copyright (C) 2007-2015 Red Hat, Inc. > + * Copyright (C) 2007-2008 Daniel P. Berrange > + * > + * This library is free software; you can redistribute it and/or > +@@ -1005,6 +1005,14 @@ virStorageBackendFileSystemVolCreate(vir > + > + vol->type = VIR_STORAGE_VOL_FILE; > + > ++ /* Volumes within a directory pools are not recursive; do not > ++ * allow escape to ../ or a subdir */ > ++ if (strchr(vol->name, '/')) { > ++ virReportError(VIR_ERR_OPERATION_INVALID, > ++ _("volume name '%s' cannot contain '/'"), vol->name); > ++ return -1; > ++ } > ++ > + VIR_FREE(vol->target.path); > + if (virAsprintf(&vol->target.path, "%s/%s", > + pool->def->target.path, > diff -Nru libvirt-1.2.9/debian/patches/series > libvirt-1.2.9/debian/patches/series > --- libvirt-1.2.9/debian/patches/series 2015-08-24 16:20:54.000000000 > +0200 > +++ libvirt-1.2.9/debian/patches/series 2016-03-05 08:18:07.000000000 > +0100 > @@ -31,3 +31,5 @@ > Allow-access-to-libnl-3-config-files.patch > Fix-crash-on-live-migration.patch > upstream/Report-original-error-when-QMP-probing-fails-with-ne.patch > +security/CVE-2015-5313-storage-don-t-allow-in-filesystem-volu.patch > +debian/Debianize-bridge-helper-path.patch > diff -Nru > libvirt-1.2.9/debian/patches/upstream/Report-original-error-when-QMP-probing-fails-with-ne.patch > > libvirt-1.2.9/debian/patches/upstream/Report-original-error-when-QMP-probing-fails-with-ne.patch > --- > libvirt-1.2.9/debian/patches/upstream/Report-original-error-when-QMP-probing-fails-with-ne.patch > 2015-08-24 16:20:54.000000000 +0200 > +++ > libvirt-1.2.9/debian/patches/upstream/Report-original-error-when-QMP-probing-fails-with-ne.patch > 2016-03-04 14:47:12.000000000 +0100 > @@ -176,7 +176,7 @@ > > if (virQEMUCapsParseHelpStr("QEMU", help, flags, > - &version, &is_kvm, &kvm_version, false) == > -1) > -+ &version, &is_kvm, &kvm_version, false, > NULL) == -1) { > ++ &version, &is_kvm, &kvm_version, false, > NULL) == -1) > goto cleanup; I wonder why this one changed as well. > > # ifndef WITH_YAJL > diff -Nru libvirt-1.2.9/debian/README.Debian > libvirt-1.2.9/debian/README.Debian > --- libvirt-1.2.9/debian/README.Debian 2015-08-24 16:20:54.000000000 > +0200 > +++ libvirt-1.2.9/debian/README.Debian 2016-03-05 08:18:07.000000000 > +0100 > @@ -51,6 +51,18 @@ > This makes dnsmasq only bind to the loopback interface by default so libvirtd > can handle the virtual bridges. > > +Bridged network > +=============== > +libvirt can use the qemu-bridge-helper to create bridged network interfaces > for > +session domains. For this to work the helper must have the capability to > create > +TUN/TAP devices or must have the SUID permission set. > +This can be done by running the following command as the user root: > + > + setcap cap_net_admin+ep /usr/lib/qemu/qemu-bridge-helper > + > +The allowed bridges must be configured in the file '/etc/qemu/bridge.conf'. > For > +each bridge add a line like 'allow br0'. > + > Access Control > ============== > Access to the libvirt managing tasks is controlled by PolicyKit. To ease > diff -Nru libvirt-1.2.9/debian/rules libvirt-1.2.9/debian/rules > --- libvirt-1.2.9/debian/rules 2015-08-24 16:20:54.000000000 +0200 > +++ libvirt-1.2.9/debian/rules 2016-03-04 13:42:30.000000000 +0100 > @@ -123,7 +123,7 @@ > EXAMPLES_DIR = > $(CURDIR)/debian/libvirt-doc/usr/share/doc/libvirt-doc/examples/ > > %: > - dh $@ --builddirectory=$(DEB_BUILDDIR) --parallel > + dh $@ --builddirectory=$(DEB_BUILDDIR) --parallel --with autoreconf That shouldn't be needed either. The two proposed fixes for: #808273 and #816602 make a lot of sense but I'm worried about the other changes. Cheers, -- Guido

