[libvirt] [PATCH 5/5] qemu: launch bridge helper from libvirtd

Paolo Bonzini pbonzini at redhat.com
Mon Mar 25 14:25:32 UTC 2013


<source type='bridge'> uses a helper application to do the necessary
TUN/TAP setup to use an existing network bridge, thus letting
unprivileged users use TUN/TAP interfaces.

However, libvirt should be preventing QEMU from running any setuid
programs at all, which would include this helper program.  From
a security POV, any setuid helper needs to be run by libvirtd itself,
not QEMU.

This is what this patch does.  libvirt now invokes the setuid helper,
gets the TAP fd and then passes it to QEMU in the normal manner.
The path to the helper is specified in qemu.conf.

As a small advantage, this adds a <target dev='tap0'/> element to the
XML of an active domain using <interface type='bridge'>.

Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
 src/qemu/qemu_command.c | 133 +++++++++++++++++++++++++++++++++++-------------
 src/qemu/qemu_command.h |   1 -
 src/qemu/qemu_hotplug.c |  25 +++------
 3 files changed, 106 insertions(+), 53 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a0c278f..fa31102 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -28,12 +28,14 @@
 #include "qemu_capabilities.h"
 #include "qemu_bridge_filter.h"
 #include "cpu/cpu.h"
+#include "passfd.h"
 #include "viralloc.h"
 #include "virlog.h"
 #include "virarch.h"
 #include "virerror.h"
 #include "virutil.h"
 #include "virfile.h"
+#include "virnetdev.h"
 #include "virstring.h"
 #include "viruuid.h"
 #include "c-ctype.h"
@@ -46,6 +48,9 @@
 #include "base64.h"
 #include "device_conf.h"
 #include "virstoragefile.h"
+#if defined(__linux__)
+# include <linux/capability.h>
+#endif
 
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -193,6 +198,77 @@ error:
 }
 
 
+/**
+ * qemuCreateInBridgePortWithHelper:
+ * @cfg: the configuration object in which the helper name is looked up
+ * @brname: the bridge name
+ * @ifname: the returned interface name
+ * @macaddr: the returned MAC address
+ * @tapfd: file descriptor return value for the new tap device
+ * @flags: OR of virNetDevTapCreateFlags:
+
+ *   VIR_NETDEV_TAP_CREATE_VNET_HDR
+ *     - Enable IFF_VNET_HDR on the tap device
+ *
+ * This function creates a new tap device on a bridge using an external
+ * helper.  The final name for the bridge will be stored in @ifname.
+ *
+ * Returns 0 in case of success or -1 on failure
+ */
+static int qemuCreateInBridgePortWithHelper(virQEMUDriverConfigPtr cfg,
+                                            const char *brname,
+                                            char **ifname,
+                                            int *tapfd,
+                                            unsigned int flags)
+{
+    virCommandPtr cmd;
+    int status;
+    int pair[2] = { -1, -1 };
+
+    if ((flags & ~VIR_NETDEV_TAP_CREATE_VNET_HDR) != VIR_NETDEV_TAP_CREATE_IFUP)
+        return -1;
+
+    if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) < 0) {
+        virReportSystemError(errno, "%s", _("failed to create socket"));
+        return -1;
+    }
+
+    cmd = virCommandNew(cfg->bridgeHelperName);
+    if (flags & VIR_NETDEV_TAP_CREATE_VNET_HDR)
+        virCommandAddArgFormat(cmd, "--use-vnet");
+    virCommandAddArgFormat(cmd, "--br=%s", brname);
+    virCommandAddArgFormat(cmd, "--fd=%d", pair[1]);
+    virCommandTransferFD(cmd, pair[1]);
+    virCommandClearCaps(cmd);
+#ifdef CAP_NET_ADMIN
+    virCommandAllowCap(cmd, CAP_NET_ADMIN);
+#endif
+    if (virCommandRunAsync(cmd, NULL) < 0) {
+        *tapfd = -1;
+        goto out;
+    }
+
+    do {
+        *tapfd = recvfd(pair[0], 0);
+    } while (*tapfd < 0 && errno == EINTR);
+    if (*tapfd < 0) {
+        virReportSystemError(errno, "%s",
+                             _("failed to retrieve file descriptor for interface"));
+        goto out;
+    }
+
+    if (virNetDevTapGetName(*tapfd, ifname) < 0 ||
+        virCommandWait(cmd, &status) < 0) {
+        VIR_FORCE_CLOSE(*tapfd);
+        *tapfd = -1;
+    }
+
+out:
+    virCommandFree(cmd);
+    VIR_FORCE_CLOSE(pair[0]);
+    return *tapfd < 0 ? -1 : 0;
+}
+
 int
 qemuNetworkIfaceConnect(virDomainDefPtr def,
                         virConnectPtr conn,
@@ -270,11 +346,17 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
         tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
     }
 
-    err = virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
-                                         def->uuid, &tapfd,
-                                         virDomainNetGetActualVirtPortProfile(net),
-                                         virDomainNetGetActualVlan(net),
-                                         tap_create_flags);
+    if (cfg->privileged)
+        err = virNetDevTapCreateInBridgePort(brname, &net->ifname, &net->mac,
+                                             def->uuid, &tapfd,
+                                             virDomainNetGetActualVirtPortProfile(net),
+                                             virDomainNetGetActualVlan(net),
+                                             tap_create_flags);
+    else
+        err = qemuCreateInBridgePortWithHelper(cfg, brname,
+                                               &net->ifname,
+                                               &tapfd, tap_create_flags);
+
     virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0);
     if (err < 0) {
         if (template_ifname)
@@ -3746,7 +3828,6 @@ error:
 char *
 qemuBuildHostNetStr(virDomainNetDefPtr net,
                     virQEMUDriverPtr driver,
-                    virQEMUCapsPtr qemuCaps,
                     char type_sep,
                     int vlan,
                     const char *tapfd,
@@ -3755,7 +3836,6 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
     bool is_tap = false;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     enum virDomainNetType netType = virDomainNetGetActualType(net);
-    const char *brname = NULL;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
     if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
@@ -3773,14 +3853,6 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
      * through, -net tap,fd
      */
     case VIR_DOMAIN_NET_TYPE_BRIDGE:
-        if (!cfg->privileged &&
-            virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE)) {
-            brname = virDomainNetGetActualBridgeName(net);
-            virBufferAsprintf(&buf, "bridge%cbr=%s", type_sep, brname);
-            type_sep = ',';
-            is_tap = true;
-            break;
-        }
     case VIR_DOMAIN_NET_TYPE_NETWORK:
     case VIR_DOMAIN_NET_TYPE_DIRECT:
         virBufferAsprintf(&buf, "tap%cfd=%s", type_sep, tapfd);
@@ -6681,26 +6753,17 @@ qemuBuildCommandLine(virConnectPtr conn,
 
             if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
                 actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
-                /*
-                 * If type='bridge' then we attempt to allocate the tap fd here only if
-                 * running under a privilged user or -netdev bridge option is not
-                 * supported.
-                 */
-                if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
-                    cfg->privileged ||
-                    (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) {
-                    int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net,
-                                                        qemuCaps);
-                    if (tapfd < 0)
-                        goto error;
+                int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net,
+                                                    qemuCaps);
+                if (tapfd < 0)
+                    goto error;
 
-                    last_good_net = i;
-                    virCommandTransferFD(cmd, tapfd);
+                last_good_net = i;
+                virCommandTransferFD(cmd, tapfd);
 
-                    if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
-                                 tapfd) >= sizeof(tapfd_name))
-                        goto no_memory;
-                }
+                if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
+                             tapfd) >= sizeof(tapfd_name))
+                    goto no_memory;
             } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
                 int tapfd = qemuPhysIfaceConnect(def, driver, net,
                                                  qemuCaps, vmop);
@@ -6744,7 +6807,7 @@ qemuBuildCommandLine(virConnectPtr conn,
             if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
                 virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
                 virCommandAddArg(cmd, "-netdev");
-                if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps,
+                if (!(host = qemuBuildHostNetStr(net, driver,
                                                  ',', vlan, tapfd_name,
                                                  vhostfd_name)))
                     goto error;
@@ -6768,7 +6831,7 @@ qemuBuildCommandLine(virConnectPtr conn,
             if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
                   virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) {
                 virCommandAddArg(cmd, "-net");
-                if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps,
+                if (!(host = qemuBuildHostNetStr(net, driver,
                                                  ',', vlan, tapfd_name,
                                                  vhostfd_name)))
                     goto error;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 17687f4..267a96e 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -71,7 +71,6 @@ qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
 /* With vlan == -1, use netdev syntax, else old hostnet */
 char * qemuBuildHostNetStr(virDomainNetDefPtr net,
                            virQEMUDriverPtr driver,
-                           virQEMUCapsPtr qemuCaps,
                            char type_sep,
                            int vlan,
                            const char *tapfd,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index de9edd4..6891efd 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -729,21 +729,12 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
 
     if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
         actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
-        /*
-         * If type=bridge then we attempt to allocate the tap fd here only if
-         * running under a privilged user or -netdev bridge option is not
-         * supported.
-         */
-        if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
-            cfg->privileged ||
-            (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) {
-            if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net,
-                                                 priv->qemuCaps)) < 0)
-                goto cleanup;
-            iface_connected = true;
-            if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
-                goto cleanup;
-        }
+        if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net,
+                                             priv->qemuCaps)) < 0)
+            goto cleanup;
+        iface_connected = true;
+        if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
+            goto cleanup;
     } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
         if ((tapfd = qemuPhysIfaceConnect(vm->def, driver, net,
                                           priv->qemuCaps,
@@ -803,12 +794,12 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
 
     if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) &&
         virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
-        if (!(netstr = qemuBuildHostNetStr(net, driver, priv->qemuCaps,
+        if (!(netstr = qemuBuildHostNetStr(net, driver,
                                            ',', -1, tapfd_name,
                                            vhostfd_name)))
             goto cleanup;
     } else {
-        if (!(netstr = qemuBuildHostNetStr(net, driver, priv->qemuCaps,
+        if (!(netstr = qemuBuildHostNetStr(net, driver,
                                            ' ', vlan, tapfd_name,
                                            vhostfd_name)))
             goto cleanup;
-- 
1.8.1.4




More information about the libvir-list mailing list