[Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

This series solves a problem that I've been struggling with for a few years now.
One of the best things about qemu is that it's possible to run guests as an
unprivileged user to improve security.  However, if you want to have your guests
communicate with the outside world, you're pretty much forced to run qemu as
root.

At least with KVM support, this is probably the most common use case which means
that most of our users are running qemu as root.  That's terrible.

We address this problem by introducing a new network backend: -net bridge.  This
backend is less flexible than -net tap because it relies on a helper with
elevated privileges to do the heavy lifting of allocating and attaching a tap
device to a bridge.  We use a special purpose helper because we don't want
to elevate the privileges of more generic tools like brctl.

>From a user perspective, to use bridged networking with a guest, you simply use:

  qemu -hda linux.img -net bridge -net nic

And assuming a bridge is defined named qemubr0 and the administrator has setup
permissions accordingly, it will Just Work.  My hope is that distributions will
do this work as part of the qemu packaging process such that for most users,
the out-of-the-box experience will also Just Work.

More details are included in individual patches.  I broke up the helper into
a series of patches to improve reviewabilty.

[Qemu-devel] [PATCH 1/4] Add basic version of bridge helper

[Anthony Liguori]

This patch adds a helper that can be used to create a tap device attached to
a bridge device.  Since this helper is minimal in what it does, it can be
given CAP_NET_ADMIN which allows qemu to avoid running as root while still
satisfying the majority of what users tend to want to do with tap devices.

The way this all works is that qemu launches this helper passing a bridge
name and the name of an inherited file descriptor.  The descriptor is one
end of a socketpair() of domain sockets.  This domain socket is used to
transmit a file descriptor of the opened tap device from the helper to qemu.

The helper can then exit and let qemu use the tap device.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 Makefile             |   12 +++-
 configure            |    1 +
 qemu-bridge-helper.c |  205 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 216 insertions(+), 2 deletions(-)
 create mode 100644 qemu-bridge-helper.c

diff --git a/Makefile b/Makefile
index c783aa4..9b15e67 100644
--- a/Makefile
+++ b/Makefile
@@ -26,6 +26,8 @@ VPATH=$(SRC_PATH):$(SRC_PATH)/hw
 
 LIBS+=-lz $(LIBS_TOOLS)
 
+HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF)
+
 ifdef BUILD_DOCS
 DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8
 else
@@ -40,7 +42,7 @@ config-all-devices.mak: $(SUBDIR_DEVICES_MAK)
 
 -include config-all-devices.mak
 
-build-all: config-host.h config-all-devices.h $(DOCS) $(TOOLS)
+build-all: config-host.h config-all-devices.h $(DOCS) $(TOOLS) $(HELPERS-y)
 	$(call quiet-command, $(MAKE) $(SUBDIR_MAKEFLAGS) recurse-all,)
 
 config-host.h: config-host.h-timestamp
@@ -217,6 +219,8 @@ qemu-nbd$(EXESUF):  qemu-nbd.o qemu-tool.o $(block-obj-y)
 
 qemu-io$(EXESUF):  qemu-io.o qemu-tool.o cmd.o $(block-obj-y)
 
+qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
+
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $@")
 
@@ -228,7 +232,7 @@ check-qlist: check-qlist.o qlist.o qint.o qemu-malloc.o
 clean:
 # avoid old build problems by removing potentially incorrect old files
 	rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
-	rm -f *.o *.d *.a $(TOOLS) TAGS cscope.* *.pod *~ */*~
+	rm -f *.o *.d *.a $(TOOLS) $(HELPERS-y) TAGS cscope.* *.pod *~ */*~
 	rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d
 	rm -f qemu-img-cmds.h
 	$(MAKE) -C tests clean
@@ -274,6 +278,10 @@ install: all $(if $(BUILD_DOCS),install-doc)
 ifneq ($(TOOLS),)
 	$(INSTALL_PROG) $(STRIP_OPT) $(TOOLS) "$(DESTDIR)$(bindir)"
 endif
+ifneq ($(HELPERS-y),)
+	$(INSTALL_DIR) "$(DESTDIR)$(libexecdir)"
+	$(INSTALL_PROG) $(STRIP_OPT) $(HELPERS-y) "$(DESTDIR)$(libexecdir)"
+endif
 ifneq ($(BLOBS),)
 	$(INSTALL_DIR) "$(DESTDIR)$(datadir)"
 	set -e; for x in $(BLOBS); do \
diff --git a/configure b/configure
index aa2cc43..a341e77 100755
--- a/configure
+++ b/configure
@@ -2106,6 +2106,7 @@ echo "bindir=\${prefix}$binsuffix" >> $config_host_mak
 echo "mandir=\${prefix}$mansuffix" >> $config_host_mak
 echo "datadir=\${prefix}$datasuffix" >> $config_host_mak
 echo "docdir=\${prefix}$docsuffix" >> $config_host_mak
+echo "libexecdir=\${prefix}/libexec" >> $config_host_mak
 echo "MAKE=$make" >> $config_host_mak
 echo "INSTALL=$install" >> $config_host_mak
 echo "INSTALL_DIR=$install -d -m0755 -p" >> $config_host_mak
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
new file mode 100644
index 0000000..f10d37c
--- /dev/null
+++ b/qemu-bridge-helper.c
@@ -0,0 +1,205 @@
+/*
+ * QEMU Bridge Helper
+ *
+ * Copyright IBM, Corp. 2009
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "config-host.h"
+
+#include <stdio.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdlib.h>
+#include <ctype.h>
+
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <sys/prctl.h>
+
+#include <net/if.h>
+
+#include <linux/sockios.h>
+
+#include "net/tap-linux.h"
+
+static int has_vnet_hdr(int fd)
+{
+    unsigned int features;
+    struct ifreq ifreq;
+
+    if (ioctl(fd, TUNGETFEATURES, &features) == -1) {
+        return -errno;
+    }
+
+    if (!(features & IFF_VNET_HDR)) {
+        return -ENOTSUP;
+    }
+
+    if (ioctl(fd, TUNGETIFF, &ifreq) != -1 || errno != EBADFD) {
+        return -ENOTSUP;
+    }
+
+    return 1;
+}
+
+static void prep_ifreq(struct ifreq *ifr, const char *ifname)
+{
+    memset(ifr, 0, sizeof(*ifr));
+    snprintf(ifr->ifr_name, IFNAMSIZ, "%s", ifname);
+}
+
+static int send_fd(int c, int fd)
+{
+    char msgbuf[CMSG_SPACE(sizeof(fd))];
+    struct msghdr msg = {
+        .msg_control = msgbuf,
+        .msg_controllen = sizeof(msgbuf),
+    };
+    struct cmsghdr *cmsg;
+    struct iovec iov;
+    char req[1] = { 0x00 };
+
+    cmsg = CMSG_FIRSTHDR(&msg);
+    cmsg->cmsg_level = SOL_SOCKET;
+    cmsg->cmsg_type = SCM_RIGHTS;
+    cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
+    msg.msg_controllen = cmsg->cmsg_len;
+
+    iov.iov_base = req;
+    iov.iov_len = sizeof(req);
+
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+    memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
+
+    return sendmsg(c, &msg, 0);
+}
+
+int main(int argc, char **argv)
+{
+    struct ifreq ifr;
+    int fd, ctlfd, unixfd;
+    int use_vnet = 0;
+    int mtu;
+    const char *bridge;
+    char iface[IFNAMSIZ];
+    int index;
+
+    /* parse arguments */
+    if (argc < 3 || argc > 4) {
+        fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
+        return 1;
+    }
+
+    index = 1;
+    if (strcmp(argv[index], "--use-vnet") == 0) {
+        use_vnet = 1;
+        index++;
+        if (argc == 3) {
+            fprintf(stderr, "invalid number of arguments\n");
+            return -1;
+        }
+    }
+
+    bridge = argv[index++];
+    unixfd = atoi(argv[index++]);
+
+    /* open a socket to use to control the network interfaces */
+    ctlfd = socket(AF_INET, SOCK_STREAM, 0);
+    if (ctlfd == -1) {
+        fprintf(stderr, "failed to open control socket\n");
+        return -errno;
+    }
+
+    /* open the tap device */
+    fd = open("/dev/net/tun", O_RDWR);
+    if (fd == -1) {
+        fprintf(stderr, "failed to open /dev/net/tun\n");
+        return -errno;
+    }
+
+    /* request a tap device, disable PI, and add vnet header support if
+     * requested and it's available. */
+    prep_ifreq(&ifr, "tap%d");
+    ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
+    if (use_vnet && has_vnet_hdr(fd)) {
+        ifr.ifr_flags |= IFF_VNET_HDR;
+    }
+
+    if (ioctl(fd, TUNSETIFF, &ifr) == -1) {
+        fprintf(stderr, "failed to create tun device\n");
+        return -errno;
+    }
+
+    /* save tap device name */
+    snprintf(iface, sizeof(iface), "%s", ifr.ifr_name);
+
+    /* get the mtu of the bridge */
+    prep_ifreq(&ifr, bridge);
+    if (ioctl(ctlfd, SIOCGIFMTU, &ifr) == -1) {
+        fprintf(stderr, "failed to get mtu of bridge `%s'\n", bridge);
+        return -errno;
+    }
+
+    /* save mtu */
+    mtu = ifr.ifr_mtu;
+
+    /* set the mtu of the interface based on the bridge */
+    prep_ifreq(&ifr, iface);
+    ifr.ifr_mtu = mtu;
+    if (ioctl(ctlfd, SIOCSIFMTU, &ifr) == -1) {
+        fprintf(stderr, "failed to set mtu of device `%s' to %d\n",
+                iface, mtu);
+        return -errno;
+    }
+
+    /* add the interface to the bridge */
+    prep_ifreq(&ifr, bridge);
+    ifr.ifr_ifindex = if_nametoindex(iface);
+
+    if (ioctl(ctlfd, SIOCBRADDIF, &ifr) == -1) {
+        fprintf(stderr, "failed to add interface `%s' to bridge `%s'\n",
+                iface, bridge);
+        return -errno;
+    }
+
+    /* bring the interface up */
+    prep_ifreq(&ifr, iface);
+    if (ioctl(ctlfd, SIOCGIFFLAGS, &ifr) == -1) {
+        fprintf(stderr, "failed to get interface flags for `%s'\n", iface);
+        return -errno;
+    }
+
+    ifr.ifr_flags |= IFF_UP;
+    if (ioctl(ctlfd, SIOCSIFFLAGS, &ifr) == -1) {
+        fprintf(stderr, "failed to set bring up interface `%s'\n", iface);
+        return -errno;
+    }
+
+    /* write fd to the domain socket */
+    if (send_fd(unixfd, fd) == -1) {
+        fprintf(stderr, "failed to write fd to unix socket\n");
+        return -errno;
+    }
+
+    /* ... */
+
+    /* profit! */
+
+    close(fd);
+
+    close(ctlfd);
+
+    return 0;
+}
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/4] Add access control support to qemu-bridge-helper

[Anthony Liguori]

We go to great lengths to restrict ourselves to just cap_net_admin as an OS
enforced security mechanism.  However, we further restrict what we allow users
to do to simply adding a tap device to a bridge interface by virtue of the fact
that this is the only functionality we expose.

This is not good enough though.  An administrator is likely to want to restrict
the bridges that an unprivileged user can access, in particular, to restrict
an unprivileged user from putting a guest on what should be isolated networks.

This patch implements a ACL mechanism that is enforced by qemu-bridge-helper.
The ACLs are fairly simple whitelist/blacklist mechanisms with a wildcard of
'all'.

An interesting feature of this ACL mechanism is that you can include external
ACL files.  The main reason to support this is so that you can set different
file system permissions on those external ACL files.  This allows an
administrator to implement rather sophisicated ACL policies based on user/group
policies via the file system.

If we fail to include an acl file, we are silent about it making this mechanism
work pretty seamlessly.  As an example:

/etc/qemu/bridge.conf root:qemu 0640

 deny all
 allow br0
 include /etc/qemu/alice.conf
 include /etc/qemu/bob.conf

/etc/qemu/alice.conf root:alice 0640
 allow br1

/etc/qemu/bob.conf root:bob 0640
 allow br2

This ACL pattern allows any user in the qemu group to get a tap device
connected to br0 (which is bridged to the physical network).

Users in the alice group can additionally get a tap device connected to br1.
This allows br1 to act as a private bridge for the alice group.

Users in the bob group can additionally get a tap device connected to br2.
This allows br2 to act as a private bridge for the bob group.

Under no circumstance can the bob group get access to br1 or can the alice
group get access to br2.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 configure            |    1 +
 qemu-bridge-helper.c |  138 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 139 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index a341e77..7c98257 100755
--- a/configure
+++ b/configure
@@ -1864,6 +1864,7 @@ printf " '%s'" "$0" "$@" >> $config_host_mak
 echo >> $config_host_mak
 
 echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> $config_host_mak
+echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak
 
 case "$cpu" in
   i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|ppc|ppc64|s390|sparc|sparc64)
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index f10d37c..0d059ed 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -33,6 +33,106 @@
 
 #include "net/tap-linux.h"
 
+#define MAX_ACLS (128)
+#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
+
+enum {
+    ACL_ALLOW = 0,
+    ACL_ALLOW_ALL,
+    ACL_DENY,
+    ACL_DENY_ALL,
+};
+
+typedef struct ACLRule
+{
+    int type;
+    char iface[IFNAMSIZ];
+} ACLRule;
+
+static int parse_acl_file(const char *filename, ACLRule *acls, int *pacl_count)
+{
+    int acl_count = *pacl_count;
+    FILE *f;
+    char line[4096];
+
+    f = fopen(filename, "r");
+    if (f == NULL) {
+        return -1;
+    }
+
+    while (acl_count != MAX_ACLS &&
+           fgets(line, sizeof(line), f) != NULL) {
+        char *ptr = line;
+        char *cmd, *arg, *argend;
+
+        while (isspace(*ptr)) {
+            ptr++;
+        }
+
+        /* skip comments and empty lines */
+        if (*ptr == '#' || *ptr == 0) {
+            continue;
+        }
+
+        cmd = ptr;
+        arg = strchr(cmd, ' ');
+        if (arg == NULL) {
+            arg = strchr(cmd, '\t');
+        }
+
+        if (arg == NULL) {
+            fprintf(stderr, "Invalid config line:\n  %s\n", line);
+            fclose(f);
+            errno = EINVAL;
+            return -1;
+        }
+
+        *arg = 0;
+        arg++;
+        while (isspace(*arg)) {
+            arg++;
+        }
+
+        argend = arg + strlen(arg);
+        while (arg != argend && isspace(*(argend - 1))) {
+            argend--;
+        }
+        *argend = 0;
+
+        if (strcmp(cmd, "deny") == 0) {
+            if (strcmp(arg, "all") == 0) {
+                acls[acl_count].type = ACL_DENY_ALL;
+            } else {
+                acls[acl_count].type = ACL_DENY;
+                snprintf(acls[acl_count].iface, IFNAMSIZ, "%s", arg);
+            }
+            acl_count++;
+        } else if (strcmp(cmd, "allow") == 0) {
+            if (strcmp(arg, "all") == 0) {
+                acls[acl_count].type = ACL_ALLOW_ALL;
+            } else {
+                acls[acl_count].type = ACL_ALLOW;
+                snprintf(acls[acl_count].iface, IFNAMSIZ, "%s", arg);
+            }
+            acl_count++;
+        } else if (strcmp(cmd, "include") == 0) {
+            /* ignore errors */
+            parse_acl_file(arg, acls, &acl_count);
+        } else {
+            fprintf(stderr, "Unknown command `%s'\n", cmd);
+            fclose(f);
+            errno = EINVAL;
+            return -1;
+        }
+    }
+
+    *pacl_count = acl_count;
+
+    fclose(f);
+
+    return 0;
+}
+
 static int has_vnet_hdr(int fd)
 {
     unsigned int features;
@@ -95,6 +195,9 @@ int main(int argc, char **argv)
     const char *bridge;
     char iface[IFNAMSIZ];
     int index;
+    ACLRule acls[MAX_ACLS];
+    int acl_count = 0;
+    int i, access_allowed;
 
     /* parse arguments */
     if (argc < 3 || argc > 4) {
@@ -115,6 +218,41 @@ int main(int argc, char **argv)
     bridge = argv[index++];
     unixfd = atoi(argv[index++]);
 
+    /* parse default acl file */
+    if (parse_acl_file(DEFAULT_ACL_FILE, acls, &acl_count) == -1) {
+        fprintf(stderr, "failed to parse default acl file `%s'\n",
+                DEFAULT_ACL_FILE);
+        return -errno;
+    }
+
+    /* validate bridge against acl -- default policy is to deny */
+    access_allowed = 0;
+    for (i = 0; i < acl_count; i++) {
+        switch (acls[i].type) {
+        case ACL_ALLOW_ALL:
+            access_allowed = 1;
+            break;
+        case ACL_ALLOW:
+            if (strcmp(bridge, acls[i].iface) == 0) {
+                access_allowed = 1;
+            }
+            break;
+        case ACL_DENY_ALL:
+            access_allowed = 0;
+            break;
+        case ACL_DENY:
+            if (strcmp(bridge, acls[i].iface) == 0) {
+                access_allowed = 0;
+            }
+            break;
+        }
+    }
+
+    if (access_allowed == 0) {
+        fprintf(stderr, "access denied by acl file\n");
+        return -EPERM;
+    }
+
     /* open a socket to use to control the network interfaces */
     ctlfd = socket(AF_INET, SOCK_STREAM, 0);
     if (ctlfd == -1) {
-- 
1.6.2.5

RE: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper

[Krumme, Chris]

Hello Anthony,

Cool patch series.
 

> -----Original Message-----
> From: 
> qemu-devel-bounces+chris.krumme=windriver.com@nongnu.org 
> [mailto:qemu-devel-bounces+chris.krumme=windriver.com@nongnu.o
> rg] On Behalf Of Anthony Liguori
> Sent: Tuesday, November 03, 2009 6:28 PM
> To: qemu-devel@nongnu.org
> Cc: Mark McLoughlin; Anthony Liguori; Arnd Bergmann; Dustin 
> Kirkland; Michael Tsirkin; Juan Quintela
> Subject: [Qemu-devel] [PATCH 2/4] Add access control support 
> toqemu-bridge-helper
> 
> We go to great lengths to restrict ourselves to just 
> cap_net_admin as an OS
> enforced security mechanism.  However, we further restrict 
> what we allow users
> to do to simply adding a tap device to a bridge interface by 
> virtue of the fact
> that this is the only functionality we expose.
> 
> This is not good enough though.  An administrator is likely 
> to want to restrict
> the bridges that an unprivileged user can access, in 
> particular, to restrict
> an unprivileged user from putting a guest on what should be 
> isolated networks.
> 
> This patch implements a ACL mechanism that is enforced by 
> qemu-bridge-helper.
> The ACLs are fairly simple whitelist/blacklist mechanisms 
> with a wildcard of
> 'all'.
> 
> An interesting feature of this ACL mechanism is that you can 
> include external
> ACL files.  The main reason to support this is so that you 
> can set different
> file system permissions on those external ACL files.  This allows an
> administrator to implement rather sophisicated ACL policies 
> based on user/group
> policies via the file system.
> 
> If we fail to include an acl file, we are silent about it 
> making this mechanism
> work pretty seamlessly.  As an example:
> 
> /etc/qemu/bridge.conf root:qemu 0640
> 
>  deny all
>  allow br0
>  include /etc/qemu/alice.conf
>  include /etc/qemu/bob.conf
> 
> /etc/qemu/alice.conf root:alice 0640
>  allow br1
> 
> /etc/qemu/bob.conf root:bob 0640
>  allow br2
> 
> This ACL pattern allows any user in the qemu group to get a tap device
> connected to br0 (which is bridged to the physical network).
> 
> Users in the alice group can additionally get a tap device 
> connected to br1.
> This allows br1 to act as a private bridge for the alice group.
> 
> Users in the bob group can additionally get a tap device 
> connected to br2.
> This allows br2 to act as a private bridge for the bob group.
> 
> Under no circumstance can the bob group get access to br1 or 
> can the alice
> group get access to br2.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  configure            |    1 +
>  qemu-bridge-helper.c |  138 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 139 insertions(+), 0 deletions(-)
> 
> diff --git a/configure b/configure
> index a341e77..7c98257 100755
> --- a/configure
> +++ b/configure
> @@ -1864,6 +1864,7 @@ printf " '%s'" "$0" "$@" >> $config_host_mak
>  echo >> $config_host_mak
>  
>  echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> 
> $config_host_mak
> +echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak
>  
>  case "$cpu" in
>    
> i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|p
> pc|ppc64|s390|sparc|sparc64)
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index f10d37c..0d059ed 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -33,6 +33,106 @@
>  
>  #include "net/tap-linux.h"
>  
> +#define MAX_ACLS (128)
> +#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
> +
> +enum {
> +    ACL_ALLOW = 0,
> +    ACL_ALLOW_ALL,
> +    ACL_DENY,
> +    ACL_DENY_ALL,
> +};
> +
> +typedef struct ACLRule
> +{
> +    int type;
> +    char iface[IFNAMSIZ];
> +} ACLRule;
> +
> +static int parse_acl_file(const char *filename, ACLRule 
> *acls, int *pacl_count)
> +{
> +    int acl_count = *pacl_count;
> +    FILE *f;
> +    char line[4096];
> +
> +    f = fopen(filename, "r");
> +    if (f == NULL) {
> +        return -1;
> +    }
> +
> +    while (acl_count != MAX_ACLS &&
> +           fgets(line, sizeof(line), f) != NULL) {
> +        char *ptr = line;
> +        char *cmd, *arg, *argend;
> +
> +        while (isspace(*ptr)) {
> +            ptr++;
> +        }
> +
> +        /* skip comments and empty lines */
> +        if (*ptr == '#' || *ptr == 0) {
> +            continue;
> +        }
> +
> +        cmd = ptr;
> +        arg = strchr(cmd, ' ');
> +        if (arg == NULL) {
> +            arg = strchr(cmd, '\t');
> +        }
> +
> +        if (arg == NULL) {
> +            fprintf(stderr, "Invalid config line:\n  %s\n", line);
> +            fclose(f);
> +            errno = EINVAL;
> +            return -1;
> +        }
> +
> +        *arg = 0;

No check is made for arg being in bounds.

Thanks

Chris

> +        arg++;
> +        while (isspace(*arg)) {
> +            arg++;
> +        }
> +
> +        argend = arg + strlen(arg);
> +        while (arg != argend && isspace(*(argend - 1))) {
> +            argend--;
> +        }
> +        *argend = 0;
> +
> +        if (strcmp(cmd, "deny") == 0) {
> +            if (strcmp(arg, "all") == 0) {
> +                acls[acl_count].type = ACL_DENY_ALL;
> +            } else {
> +                acls[acl_count].type = ACL_DENY;
> +                snprintf(acls[acl_count].iface, IFNAMSIZ, "%s", arg);
> +            }
> +            acl_count++;
> +        } else if (strcmp(cmd, "allow") == 0) {
> +            if (strcmp(arg, "all") == 0) {
> +                acls[acl_count].type = ACL_ALLOW_ALL;
> +            } else {
> +                acls[acl_count].type = ACL_ALLOW;
> +                snprintf(acls[acl_count].iface, IFNAMSIZ, "%s", arg);
> +            }
> +            acl_count++;
> +        } else if (strcmp(cmd, "include") == 0) {
> +            /* ignore errors */
> +            parse_acl_file(arg, acls, &acl_count);
> +        } else {
> +            fprintf(stderr, "Unknown command `%s'\n", cmd);
> +            fclose(f);
> +            errno = EINVAL;
> +            return -1;
> +        }
> +    }
> +
> +    *pacl_count = acl_count;
> +
> +    fclose(f);
> +
> +    return 0;
> +}
> +
>  static int has_vnet_hdr(int fd)
>  {
>      unsigned int features;
> @@ -95,6 +195,9 @@ int main(int argc, char **argv)
>      const char *bridge;
>      char iface[IFNAMSIZ];
>      int index;
> +    ACLRule acls[MAX_ACLS];
> +    int acl_count = 0;
> +    int i, access_allowed;
>  
>      /* parse arguments */
>      if (argc < 3 || argc > 4) {
> @@ -115,6 +218,41 @@ int main(int argc, char **argv)
>      bridge = argv[index++];
>      unixfd = atoi(argv[index++]);
>  
> +    /* parse default acl file */
> +    if (parse_acl_file(DEFAULT_ACL_FILE, acls, &acl_count) == -1) {
> +        fprintf(stderr, "failed to parse default acl file `%s'\n",
> +                DEFAULT_ACL_FILE);
> +        return -errno;
> +    }
> +
> +    /* validate bridge against acl -- default policy is to deny */
> +    access_allowed = 0;
> +    for (i = 0; i < acl_count; i++) {
> +        switch (acls[i].type) {
> +        case ACL_ALLOW_ALL:
> +            access_allowed = 1;
> +            break;
> +        case ACL_ALLOW:
> +            if (strcmp(bridge, acls[i].iface) == 0) {
> +                access_allowed = 1;
> +            }
> +            break;
> +        case ACL_DENY_ALL:
> +            access_allowed = 0;
> +            break;
> +        case ACL_DENY:
> +            if (strcmp(bridge, acls[i].iface) == 0) {
> +                access_allowed = 0;
> +            }
> +            break;
> +        }
> +    }
> +
> +    if (access_allowed == 0) {
> +        fprintf(stderr, "access denied by acl file\n");
> +        return -EPERM;
> +    }
> +
>      /* open a socket to use to control the network interfaces */
>      ctlfd = socket(AF_INET, SOCK_STREAM, 0);
>      if (ctlfd == -1) {
> -- 
> 1.6.2.5
> 
> 
> 
> 

Re: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper

[Anthony Liguori]

Krumme, Chris wrote:
> Hello Anthony,
>
> Cool patch series.
>   

Thanks.

>> +        cmd = ptr;
>> +        arg = strchr(cmd, ' ');
>> +        if (arg == NULL) {
>> +            arg = strchr(cmd, '\t');
>> +        }
>> +
>> +        if (arg == NULL) {
>> +            fprintf(stderr, "Invalid config line:\n  %s\n", line);
>> +            fclose(f);
>> +            errno = EINVAL;
>> +            return -1;
>> +        }
>> +
>> +        *arg = 0;
>>     
>
> No check is made for arg being in bounds.
>   

I don't get it.  arg is either going to be NULL (no ' ' or '\t' found in 
the string) or it will point to the first ' ' or '\t' in the string.  It 
will always be in bound in this second case and the first case is 
handled by the if().

Regards,

Anthony Liguori

RE: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper

[Krumme, Chris]

Hello Anthony, 

> -----Original Message-----
> From: Anthony Liguori [mailto:anthony@codemonkey.ws] 
> Sent: Wednesday, November 04, 2009 8:23 AM
> To: Krumme, Chris
> Cc: qemu-devel@nongnu.org; Mark McLoughlin; Arnd Bergmann; 
> Michael Tsirkin; Juan Quintela; Dustin Kirkland
> Subject: Re: [Qemu-devel] [PATCH 2/4] Add access control 
> support toqemu-bridge-helper
> 
> Krumme, Chris wrote:
> > Hello Anthony,
> >
> > Cool patch series.
> >   
> 
> Thanks.
> 
> >> +        cmd = ptr;
> >> +        arg = strchr(cmd, ' ');
> >> +        if (arg == NULL) {
> >> +            arg = strchr(cmd, '\t');
> >> +        }
> >> +
> >> +        if (arg == NULL) {
> >> +            fprintf(stderr, "Invalid config line:\n  %s\n", line);
> >> +            fclose(f);
> >> +            errno = EINVAL;
> >> +            return -1;
> >> +        }
> >> +
> >> +        *arg = 0;
> >>     
> >
> > No check is made for arg being in bounds.
> >   
> 
> I don't get it.  arg is either going to be NULL (no ' ' or 
> '\t' found in 
> the string) or it will point to the first ' ' or '\t' in the 
> string.  It 

My concern is that the first space or tab may not be in the first
sizeof(line) characters.

Thanks

Chris

> will always be in bound in this second case and the first case is 
> handled by the if().
> 
> Regards,
> 
> Anthony Liguori
> 

Re: [Qemu-devel] [PATCH 2/4] Add access control support to qemu-bridge-helper

[Daniel P. Berrange]

On Tue, Nov 03, 2009 at 06:28:03PM -0600, Anthony Liguori wrote:
> We go to great lengths to restrict ourselves to just cap_net_admin as an OS
> enforced security mechanism.  However, we further restrict what we allow users
> to do to simply adding a tap device to a bridge interface by virtue of the fact
> that this is the only functionality we expose.
> 
> This is not good enough though.  An administrator is likely to want to restrict
> the bridges that an unprivileged user can access, in particular, to restrict
> an unprivileged user from putting a guest on what should be isolated networks.
> 
> This patch implements a ACL mechanism that is enforced by qemu-bridge-helper.
> The ACLs are fairly simple whitelist/blacklist mechanisms with a wildcard of
> 'all'.
> 
> An interesting feature of this ACL mechanism is that you can include external
> ACL files.  The main reason to support this is so that you can set different
> file system permissions on those external ACL files.  This allows an
> administrator to implement rather sophisicated ACL policies based on user/group
> policies via the file system.
> 
> If we fail to include an acl file, we are silent about it making this mechanism
> work pretty seamlessly.  As an example:
> 
> /etc/qemu/bridge.conf root:qemu 0640
> 
>  deny all
>  allow br0
>  include /etc/qemu/alice.conf
>  include /etc/qemu/bob.conf
> 
> /etc/qemu/alice.conf root:alice 0640
>  allow br1
> 
> /etc/qemu/bob.conf root:bob 0640
>  allow br2
> 
> This ACL pattern allows any user in the qemu group to get a tap device
> connected to br0 (which is bridged to the physical network).
> 
> Users in the alice group can additionally get a tap device connected to br1.
> This allows br1 to act as a private bridge for the alice group.
> 
> Users in the bob group can additionally get a tap device connected to br2.
> This allows br2 to act as a private bridge for the bob group.
> 
> Under no circumstance can the bob group get access to br1 or can the alice
> group get access to br2.

If we're going to define an ACL file for this, then I'd like us to
try and get a file format that is suitable for all possible ACL
needs in QEMU. In particular to allow coverage of VNC server ACLs
which I previously did a proof of concept for

http://article.gmane.org/gmane.comp.emulators.qemu/38173

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

[Qemu-devel] [PATCH 3/4] Add cap reduction support to enable use as SUID binary

[Anthony Liguori]

The ideal way to use qemu-bridge-helper is to give it an fscap of using:

 setcap cap_net_admin=ep qemu-bridge-helper

Unfortunately, most distros still do not have a mechanism to package files
with fscaps applied.  This means they'll have to SUID the qemu-bridge-helper
binary.

To improve security, use libcap to reduce our capability set to just
cap_net_admin, then reduce privileges down to the calling user.  This is
hopefully close to equivalent to fscap support from a security perspective.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 configure            |   34 ++++++++++++++++++++++++++++++
 qemu-bridge-helper.c |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 7c98257..7c9d3a2 100755
--- a/configure
+++ b/configure
@@ -194,6 +194,7 @@ vnc_tls=""
 vnc_sasl=""
 xen=""
 linux_aio=""
+cap=""
 
 gprof="no"
 debug_tcg="no"
@@ -480,6 +481,10 @@ for opt do
   ;;
   --enable-kvm) kvm="yes"
   ;;
+  --disable-cap) cap="no"
+  ;;
+  --enable-cap) cap="yes"
+  ;;
   --enable-profiler) profiler="yes"
   ;;
   --enable-cocoa)
@@ -710,6 +715,8 @@ echo "  --disable-vde            disable support for vde network"
 echo "  --enable-vde             enable support for vde network"
 echo "  --disable-linux-aio      disable Linux AIO support"
 echo "  --enable-linux-aio       enable Linux AIO support"
+echo "  --disable-libcap         disable support for libcap"
+echo "  --enable-libcap          enable support for libcap"
 echo "  --enable-io-thread       enable IO thread"
 echo "  --disable-blobs          disable installing provided firmware blobs"
 echo "  --kerneldir=PATH         look for kernel includes in PATH"
@@ -1108,6 +1115,29 @@ EOF
 fi
 
 ##########################################
+# cap library probe
+if test "$cap" != "no" ; then
+  cap_libs="-lcap"
+  cat > $TMPC << EOF
+#include <sys/capability.h>
+int main(void)
+{
+    cap_init();
+    return 0;
+}
+EOF
+  if compile_prog "" "$cap_libs" ; then
+    cap=yes
+    libs_tools="$cap_libs $libs_tools"
+  else
+    if test "$cap" = "yes" ; then
+      feature_not_found "cap"
+    fi
+    cap=no
+  fi
+fi
+
+##########################################
 # Sound support libraries probe
 
 audio_drv_probe()
@@ -1850,6 +1880,7 @@ echo "fdt support       $fdt"
 echo "preadv support    $preadv"
 echo "fdatasync         $fdatasync"
 echo "uuid support      $uuid"
+echo "libcap support    $cap"
 
 if test $sdl_too_old = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -1931,6 +1962,9 @@ fi
 if test "$vde" = "yes" ; then
   echo "CONFIG_VDE=y" >> $config_host_mak
 fi
+if test "$cap" = "yes" ; then
+  echo "CONFIG_LIBCAP=y" >> $config_host_mak
+fi
 for card in $audio_card_list; do
     def=CONFIG_`echo $card | tr '[:lower:]' '[:upper:]'`
     echo "$def=y" >> $config_host_mak
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 0d059ed..73e1c5a 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -33,6 +33,10 @@
 
 #include "net/tap-linux.h"
 
+#ifdef CONFIG_LIBCAP
+#include <sys/capability.h>
+#endif
+
 #define MAX_ACLS (128)
 #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
 
@@ -186,6 +190,47 @@ static int send_fd(int c, int fd)
     return sendmsg(c, &msg, 0);
 }
 
+#ifdef CONFIG_LIBCAP
+static int drop_privileges(void)
+{
+    cap_t cap;
+    cap_value_t new_caps[] = {CAP_NET_ADMIN};
+
+    cap = cap_init();
+
+    /* set capabilities to be permitted and inheritable.  we don't need the
+     * caps to be effective right now as they'll get reset when we seteuid
+     * anyway */
+    cap_set_flag(cap, CAP_PERMITTED, 1, new_caps, CAP_SET);
+    cap_set_flag(cap, CAP_INHERITABLE, 1, new_caps, CAP_SET);
+
+    if (cap_set_proc(cap) == -1) {
+        return -1;
+    }
+
+    cap_free(cap);
+
+    /* reduce our privileges to a normal user */
+    setegid(getgid());
+    seteuid(getuid());
+
+    cap = cap_init();
+
+    /* enable the our capabilities.  we marked them as inheritable earlier
+     * which is what allows this to work. */
+    cap_set_flag(cap, CAP_EFFECTIVE, 1, new_caps, CAP_SET);
+    cap_set_flag(cap, CAP_PERMITTED, 1, new_caps, CAP_SET);
+
+    if (cap_set_proc(cap) == -1) {
+        return -1;
+    }
+
+    cap_free(cap);
+
+    return 0;
+}
+#endif
+
 int main(int argc, char **argv)
 {
     struct ifreq ifr;
@@ -199,6 +244,17 @@ int main(int argc, char **argv)
     int acl_count = 0;
     int i, access_allowed;
 
+#ifdef CONFIG_LIBCAP
+    /* if we're run from an suid binary, immediately drop privileges preserving
+     * cap_net_admin */
+    if (geteuid() == 0 && getuid() != geteuid()) {
+        if (drop_privileges() == -1) {
+            fprintf(stderr, "failed to drop privileges\n");
+            return 1;
+        }
+    }
+#endif
+
     /* parse arguments */
     if (argc < 3 || argc > 4) {
         fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Anthony Liguori]

The most common use of -net tap is to connect a tap device to a bridge.  This
requires the use of a script and running qemu as root in order to allocate a
tap device to pass to the script.

This model is great for portability and flexibility but it's incredibly
difficult to eliminate the need to run qemu as root.  The only really viable
mechanism is to use tunctl to create a tap device, attach it to a bridge as
root, and then hand that tap device to qemu.  The problem with this mechanism
is that it requires administrator intervention whenever a user wants to create
a guest.

By essentially writing a helper that implements the most common qemu-ifup
script that can be safely given cap_net_admin, we can dramatically simplify
things for non-privileged users.  We still support -net tap as a mechanism
for advanced users and backwards compatibility.

Currently, this is very Linux centric but there's really no reason why it
couldn't be extended for other Unixes.

A typical invocation of -net bridge would be:

  qemu -net bridge -net nic,model=virtio

The default bridge that we attach to is qemubr0.  The thinking is that a distro
could preconfigure such an interface to allow out-of-the-box bridged networking.

Alternatively, if a user wants to use a different bridge, they can say:

  qemu -net bridge,br=br0 -net nic,model=virtio

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 configure       |    1 +
 net.c           |   20 +++++++-
 net/tap.c       |  142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/tap.h       |    2 +
 qemu-options.hx |    3 +
 5 files changed, 167 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 7c9d3a2..55a1a4f 100755
--- a/configure
+++ b/configure
@@ -1896,6 +1896,7 @@ echo >> $config_host_mak
 
 echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> $config_host_mak
 echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak
+echo "CONFIG_QEMU_HELPERDIR=\"$prefix/libexec\"" >> $config_host_mak
 
 case "$cpu" in
   i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|ppc|ppc64|s390|sparc|sparc64)
diff --git a/net.c b/net.c
index 37662c6..54a7a5b 100644
--- a/net.c
+++ b/net.c
@@ -2541,6 +2541,22 @@ static struct {
             },
             { /* end of list */ }
         },
+    }, {
+        .type = "bridge",
+        .init = net_init_bridge,
+        .desc = {
+            NET_COMMON_PARAMS_DESC,
+            {
+                .name = "br",
+                .type = QEMU_OPT_STRING,
+                .help = "bridge name",
+            }, {
+                .name = "helper",
+                .type = QEMU_OPT_STRING,
+                .help = "command to execute to configure bridge",
+            },
+            { /* end of list */ }
+        },
     },
     { /* end of list */ }
 };
@@ -2565,7 +2581,8 @@ int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
 #ifdef CONFIG_VDE
             strcmp(type, "vde") != 0 &&
 #endif
-            strcmp(type, "socket") != 0) {
+            strcmp(type, "socket") != 0 &&
+            strcmp(type, "bridge") != 0) {
             qemu_error("The '%s' network backend type is not valid with -netdev\n",
                        type);
             return -1;
@@ -2641,6 +2658,7 @@ static int net_host_check_device(const char *device)
 #ifdef CONFIG_VDE
                                        ,"vde"
 #endif
+                                       , "bridge"
     };
     for (i = 0; i < sizeof(valid_param_list) / sizeof(char *); i++) {
         if (!strncmp(valid_param_list[i], device,
diff --git a/net/tap.c b/net/tap.c
index bdb4a15..f5abed6 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -436,3 +436,145 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
 
     return 0;
 }
+
+#define DEFAULT_BRIDGE_INTERFACE "qemubr0"
+#define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR "/qemu-bridge-helper"
+
+static int recv_fd(int c)
+{
+    int fd;
+    uint8_t msgbuf[CMSG_SPACE(sizeof(fd))];
+    struct msghdr msg = {
+        .msg_control = msgbuf,
+        .msg_controllen = sizeof(msgbuf),
+    };
+    struct cmsghdr *cmsg;
+    struct iovec iov;
+    uint8_t req[1];
+    ssize_t len;
+
+    cmsg = CMSG_FIRSTHDR(&msg);
+    cmsg->cmsg_level = SOL_SOCKET;
+    cmsg->cmsg_type = SCM_RIGHTS;
+    cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
+    msg.msg_controllen = cmsg->cmsg_len;
+
+    iov.iov_base = req;
+    iov.iov_len = sizeof(req);
+
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+
+    len = recvmsg(c, &msg, 0);
+    if (len > 0) {
+        memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd));
+
+        return fd;
+    }
+
+    return len;
+}
+
+static int net_bridge_run_helper(const char *helper, const char *bridge)
+{
+    sigset_t oldmask, mask;
+    int pid, status;
+    char *args[5];
+    char **parg;
+    int sv[2];
+
+    sigemptyset(&mask);
+    sigaddset(&mask, SIGCHLD);
+    sigprocmask(SIG_BLOCK, &mask, &oldmask);
+
+    if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
+        return -1;
+    }
+
+    /* try to launch network script */
+    pid = fork();
+    if (pid == 0) {
+        int open_max = sysconf(_SC_OPEN_MAX), i;
+        char buf[32];
+
+        snprintf(buf, sizeof(buf), "%d", sv[1]);
+
+        for (i = 0; i < open_max; i++) {
+            if (i != STDIN_FILENO &&
+                i != STDOUT_FILENO &&
+                i != STDERR_FILENO &&
+                i != sv[1]) {
+                close(i);
+            }
+        }
+        parg = args;
+        *parg++ = (char *)helper;
+        *parg++ = (char *)"--use-vnet";
+        *parg++ = (char *)bridge;
+        *parg++ = buf;
+        *parg++ = NULL;
+        execv(helper, args);
+        _exit(1);
+    } else if (pid > 0) {
+        int fd;
+
+        close(sv[1]);
+
+        do {
+            fd = recv_fd(sv[0]);
+        } while (fd == -1 && errno == EINTR);
+
+        close(sv[0]);
+
+        while (waitpid(pid, &status, 0) != pid) {
+            /* loop */
+        }
+        sigprocmask(SIG_SETMASK, &oldmask, NULL);
+
+        if (fd < 0) {
+            fprintf(stderr, "failed to recv file descriptor\n");
+            return -1;
+        }
+
+        if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
+            return fd;
+        }
+    }
+    fprintf(stderr, "failed to launch bridge helper\n");
+    return -1;
+}
+
+int net_init_bridge(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan)
+{
+    TAPState *s;
+    int fd, vnet_hdr;
+
+    if (!qemu_opt_get(opts, "br")) {
+        qemu_opt_set(opts, "br", DEFAULT_BRIDGE_INTERFACE);
+    }
+    if (!qemu_opt_get(opts, "helper")) {
+        qemu_opt_set(opts, "helper", DEFAULT_BRIDGE_HELPER);
+    }
+
+    fd = net_bridge_run_helper(qemu_opt_get(opts, "helper"),
+                               qemu_opt_get(opts, "br"));
+
+    fcntl(fd, F_SETFL, O_NONBLOCK);
+
+    vnet_hdr = tap_probe_vnet_hdr(fd);
+
+    s = net_tap_fd_init(vlan, "bridge", name, fd, vnet_hdr);
+    if (!s) {
+        close(fd);
+        return -1;
+    }
+
+    snprintf(s->vc->info_str, sizeof(s->vc->info_str),
+             "br=%s", qemu_opt_get(opts, "br"));
+
+    if (vlan) {
+        vlan->nb_host_devs++;
+    }
+
+    return 0;
+}
diff --git a/net/tap.h b/net/tap.h
index 538a562..9527db4 100644
--- a/net/tap.h
+++ b/net/tap.h
@@ -48,4 +48,6 @@ int tap_probe_vnet_hdr(int fd);
 int tap_probe_has_ufo(int fd);
 void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int ufo);
 
+int net_init_bridge(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan);
+
 #endif /* QEMU_NET_TAP_H */
diff --git a/qemu-options.hx b/qemu-options.hx
index d78b738..8f20aa6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -820,6 +820,9 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
     "                default of 'sndbuf=1048576' can be disabled using 'sndbuf=0'\n"
     "                use vnet_hdr=off to avoid enabling the IFF_VNET_HDR tap flag; use\n"
     "                vnet_hdr=on to make the lack of IFF_VNET_HDR support an error condition\n"
+    "-net bridge[,vlan=n][,name=str][,br=bridge][,helper=helper]\n"
+    "                connects to a host bridge device 'br' using the program 'helper'\n"
+    "                to create a tap device and attach it to the bridge\n"
 #endif
     "-net socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connect=host:port]\n"
     "                connect the vlan 'n' to another VLAN using a socket connection\n"
-- 
1.6.2.5

RE: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Krumme, Chris]

Hello Anthony,

Now that I have read the whole series I say again great patch. 

> -----Original Message-----
> From: 
> qemu-devel-bounces+chris.krumme=windriver.com@nongnu.org 
> [mailto:qemu-devel-bounces+chris.krumme=windriver.com@nongnu.o
> rg] On Behalf Of Anthony Liguori
> Sent: Tuesday, November 03, 2009 6:28 PM
> To: qemu-devel@nongnu.org
> Cc: Mark McLoughlin; Anthony Liguori; Arnd Bergmann; Dustin 
> Kirkland; Michael Tsirkin; Juan Quintela
> Subject: [Qemu-devel] [PATCH 4/4] Add support for -net bridge
> 
> The most common use of -net tap is to connect a tap device to 
> a bridge.  This
> requires the use of a script and running qemu as root in 
> order to allocate a
> tap device to pass to the script.
> 
> This model is great for portability and flexibility but it's 
> incredibly
> difficult to eliminate the need to run qemu as root.  The 
> only really viable
> mechanism is to use tunctl to create a tap device, attach it 
> to a bridge as
> root, and then hand that tap device to qemu.  The problem 
> with this mechanism
> is that it requires administrator intervention whenever a 
> user wants to create
> a guest.
> 
> By essentially writing a helper that implements the most 
> common qemu-ifup
> script that can be safely given cap_net_admin, we can 
> dramatically simplify
> things for non-privileged users.  We still support -net tap 
> as a mechanism
> for advanced users and backwards compatibility.
> 
> Currently, this is very Linux centric but there's really no 
> reason why it
> couldn't be extended for other Unixes.
> 
> A typical invocation of -net bridge would be:
> 
>   qemu -net bridge -net nic,model=virtio
> 
> The default bridge that we attach to is qemubr0.  The 
> thinking is that a distro
> could preconfigure such an interface to allow out-of-the-box 
> bridged networking.
> 
> Alternatively, if a user wants to use a different bridge, 
> they can say:
> 
>   qemu -net bridge,br=br0 -net nic,model=virtio
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  configure       |    1 +
>  net.c           |   20 +++++++-
>  net/tap.c       |  142 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/tap.h       |    2 +
>  qemu-options.hx |    3 +
>  5 files changed, 167 insertions(+), 1 deletions(-)
> 
> diff --git a/configure b/configure
> index 7c9d3a2..55a1a4f 100755
> --- a/configure
> +++ b/configure
> @@ -1896,6 +1896,7 @@ echo >> $config_host_mak
>  
>  echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> 
> $config_host_mak
>  echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak
> +echo "CONFIG_QEMU_HELPERDIR=\"$prefix/libexec\"" >> $config_host_mak
>  
>  case "$cpu" in
>    
> i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|p
> pc|ppc64|s390|sparc|sparc64)
> diff --git a/net.c b/net.c
> index 37662c6..54a7a5b 100644
> --- a/net.c
> +++ b/net.c
> @@ -2541,6 +2541,22 @@ static struct {
>              },
>              { /* end of list */ }
>          },
> +    }, {
> +        .type = "bridge",
> +        .init = net_init_bridge,
> +        .desc = {
> +            NET_COMMON_PARAMS_DESC,
> +            {
> +                .name = "br",
> +                .type = QEMU_OPT_STRING,
> +                .help = "bridge name",
> +            }, {
> +                .name = "helper",
> +                .type = QEMU_OPT_STRING,
> +                .help = "command to execute to configure bridge",
> +            },
> +            { /* end of list */ }
> +        },
>      },
>      { /* end of list */ }
>  };
> @@ -2565,7 +2581,8 @@ int net_client_init(Monitor *mon, 
> QemuOpts *opts, int is_netdev)
>  #ifdef CONFIG_VDE
>              strcmp(type, "vde") != 0 &&
>  #endif
> -            strcmp(type, "socket") != 0) {
> +            strcmp(type, "socket") != 0 &&
> +            strcmp(type, "bridge") != 0) {
>              qemu_error("The '%s' network backend type is not 
> valid with -netdev\n",
>                         type);
>              return -1;
> @@ -2641,6 +2658,7 @@ static int net_host_check_device(const 
> char *device)
>  #ifdef CONFIG_VDE
>                                         ,"vde"
>  #endif
> +                                       , "bridge"
>      };
>      for (i = 0; i < sizeof(valid_param_list) / sizeof(char *); i++) {
>          if (!strncmp(valid_param_list[i], device,
> diff --git a/net/tap.c b/net/tap.c
> index bdb4a15..f5abed6 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -436,3 +436,145 @@ int net_init_tap(QemuOpts *opts, 
> Monitor *mon, const char *name, VLANState *vlan
>  
>      return 0;
>  }
> +
> +#define DEFAULT_BRIDGE_INTERFACE "qemubr0"
> +#define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR 
> "/qemu-bridge-helper"
> +
> +static int recv_fd(int c)
> +{
> +    int fd;
> +    uint8_t msgbuf[CMSG_SPACE(sizeof(fd))];
> +    struct msghdr msg = {
> +        .msg_control = msgbuf,
> +        .msg_controllen = sizeof(msgbuf),
> +    };
> +    struct cmsghdr *cmsg;
> +    struct iovec iov;
> +    uint8_t req[1];
> +    ssize_t len;
> +
> +    cmsg = CMSG_FIRSTHDR(&msg);
> +    cmsg->cmsg_level = SOL_SOCKET;
> +    cmsg->cmsg_type = SCM_RIGHTS;
> +    cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
> +    msg.msg_controllen = cmsg->cmsg_len;
> +
> +    iov.iov_base = req;
> +    iov.iov_len = sizeof(req);
> +
> +    msg.msg_iov = &iov;
> +    msg.msg_iovlen = 1;
> +
> +    len = recvmsg(c, &msg, 0);
> +    if (len > 0) {
> +        memcpy(&fd, CMSG_DATA(cmsg), sizeof(fd));
> +
> +        return fd;
> +    }
> +
> +    return len;
> +}
> +
> +static int net_bridge_run_helper(const char *helper, const 
> char *bridge)
> +{
> +    sigset_t oldmask, mask;
> +    int pid, status;
> +    char *args[5];
> +    char **parg;
> +    int sv[2];
> +
> +    sigemptyset(&mask);
> +    sigaddset(&mask, SIGCHLD);
> +    sigprocmask(SIG_BLOCK, &mask, &oldmask);
> +
> +    if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
> +        return -1;
> +    }
> +
> +    /* try to launch network script */
> +    pid = fork();
> +    if (pid == 0) {
> +        int open_max = sysconf(_SC_OPEN_MAX), i;
> +        char buf[32];
> +
> +        snprintf(buf, sizeof(buf), "%d", sv[1]);
> +
> +        for (i = 0; i < open_max; i++) {
> +            if (i != STDIN_FILENO &&
> +                i != STDOUT_FILENO &&
> +                i != STDERR_FILENO &&
> +                i != sv[1]) {
> +                close(i);
> +            }
> +        }
> +        parg = args;
> +        *parg++ = (char *)helper;
> +        *parg++ = (char *)"--use-vnet";
> +        *parg++ = (char *)bridge;
> +        *parg++ = buf;
> +        *parg++ = NULL;
> +        execv(helper, args);
> +        _exit(1);
> +    } else if (pid > 0) {
> +        int fd;
> +
> +        close(sv[1]);
> +
> +        do {
> +            fd = recv_fd(sv[0]);
> +        } while (fd == -1 && errno == EINTR);
> +
> +        close(sv[0]);
> +
> +        while (waitpid(pid, &status, 0) != pid) {
> +            /* loop */
> +        }
> +        sigprocmask(SIG_SETMASK, &oldmask, NULL);
> +
> +        if (fd < 0) {
> +            fprintf(stderr, "failed to recv file descriptor\n");
> +            return -1;
> +        }
> +
> +        if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
> +            return fd;
> +        }
> +    }
> +    fprintf(stderr, "failed to launch bridge helper\n");
> +    return -1;
> +}
> +
> +int net_init_bridge(QemuOpts *opts, Monitor *mon, const char 
> *name, VLANState *vlan)
> +{
> +    TAPState *s;
> +    int fd, vnet_hdr;
> +
> +    if (!qemu_opt_get(opts, "br")) {
> +        qemu_opt_set(opts, "br", DEFAULT_BRIDGE_INTERFACE);
> +    }
> +    if (!qemu_opt_get(opts, "helper")) {
> +        qemu_opt_set(opts, "helper", DEFAULT_BRIDGE_HELPER);
> +    }
> +
> +    fd = net_bridge_run_helper(qemu_opt_get(opts, "helper"),
> +                               qemu_opt_get(opts, "br"));
> +
> +    fcntl(fd, F_SETFL, O_NONBLOCK);
> +
> +    vnet_hdr = tap_probe_vnet_hdr(fd);
> +
> +    s = net_tap_fd_init(vlan, "bridge", name, fd, vnet_hdr);
> +    if (!s) {
> +        close(fd);
> +        return -1;
> +    }
> +
> +    snprintf(s->vc->info_str, sizeof(s->vc->info_str),
> +             "br=%s", qemu_opt_get(opts, "br"));
> +
> +    if (vlan) {
> +        vlan->nb_host_devs++;
> +    }
> +
> +    return 0;
> +}
> diff --git a/net/tap.h b/net/tap.h
> index 538a562..9527db4 100644
> --- a/net/tap.h
> +++ b/net/tap.h
> @@ -48,4 +48,6 @@ int tap_probe_vnet_hdr(int fd);
>  int tap_probe_has_ufo(int fd);
>  void tap_fd_set_offload(int fd, int csum, int tso4, int 
> tso6, int ecn, int ufo);
>  
> +int net_init_bridge(QemuOpts *opts, Monitor *mon, const char 
> *name, VLANState *vlan);
> +
>  #endif /* QEMU_NET_TAP_H */
> diff --git a/qemu-options.hx b/qemu-options.hx
> index d78b738..8f20aa6 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -820,6 +820,9 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
>      "                default of 'sndbuf=1048576' can be 
> disabled using 'sndbuf=0'\n"
>      "                use vnet_hdr=off to avoid enabling the 
> IFF_VNET_HDR tap flag; use\n"
>      "                vnet_hdr=on to make the lack of 
> IFF_VNET_HDR support an error condition\n"
> +    "-net bridge[,vlan=n][,name=str][,br=bridge][,helper=helper]\n"
> +    "                connects to a host bridge device 'br' 
> using the program 'helper'\n"

Do you need to mention the default name qemubr0 here?

Thanks

Chris


> +    "                to create a tap device and attach it to 
> the bridge\n"
>  #endif
>      "-net 
> socket[,vlan=n][,name=str][,fd=h][,listen=[host]:port][,connec
> t=host:port]\n"
>      "                connect the vlan 'n' to another VLAN 
> using a socket connection\n"
> -- 
> 1.6.2.5
> 
> 
> 
> 

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Anthony Liguori]

Krumme, Chris wrote:
> Do you need to mention the default name qemubr0 here?
>   

Good suggestion.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Avi Kivity]

On 11/04/2009 02:28 AM, Anthony Liguori wrote:
> The most common use of -net tap is to connect a tap device to a bridge.  This
> requires the use of a script and running qemu as root in order to allocate a
> tap device to pass to the script.
>
> This model is great for portability and flexibility but it's incredibly
> difficult to eliminate the need to run qemu as root.  The only really viable
> mechanism is to use tunctl to create a tap device, attach it to a bridge as
> root, and then hand that tap device to qemu.  The problem with this mechanism
> is that it requires administrator intervention whenever a user wants to create
> a guest.
>
> By essentially writing a helper that implements the most common qemu-ifup
> script that can be safely given cap_net_admin, we can dramatically simplify
> things for non-privileged users.  We still support -net tap as a mechanism
> for advanced users and backwards compatibility.
>
> Currently, this is very Linux centric but there's really no reason why it
> couldn't be extended for other Unixes.
>
> A typical invocation of -net bridge would be:
>
>    qemu -net bridge -net nic,model=virtio
>
> The default bridge that we attach to is qemubr0.  The thinking is that a distro
> could preconfigure such an interface to allow out-of-the-box bridged networking.
>
> Alternatively, if a user wants to use a different bridge, they can say:
>
>    qemu -net bridge,br=br0 -net nic,model=virtio
>
>
> +int net_init_bridge(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan);
> +
>    

Don't we need to tear the interface down after shutdown?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Anthony Liguori]

Avi Kivity wrote:
>> +int net_init_bridge(QemuOpts *opts, Monitor *mon, const char *name, 
>> VLANState *vlan);
>> +
>>    
>
> Don't we need to tear the interface down after shutdown?

net_init_bridge calls net_tap_fd_init which registers tap_cleanup.  That 
closes the fd and frees associated memory.

The helper does not allocate a persistent tap device so closing the file 
descriptor is sufficient for cleanup.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Avi Kivity]

On 11/05/2009 04:45 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>>> +int net_init_bridge(QemuOpts *opts, Monitor *mon, const char *name, 
>>> VLANState *vlan);
>>> +
>>
>> Don't we need to tear the interface down after shutdown?
>
> net_init_bridge calls net_tap_fd_init which registers tap_cleanup.  
> That closes the fd and frees associated memory.
>
> The helper does not allocate a persistent tap device so closing the 
> file descriptor is sufficient for cleanup.
>

Ah, so unclean shutdown will clean up as well.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Jamie Lokier]

Anthony Liguori wrote:
> Avi Kivity wrote:
> >>+int net_init_bridge(QemuOpts *opts, Monitor *mon, const char *name, 
> >>VLANState *vlan);
> >>+
> >>   
> >
> >Don't we need to tear the interface down after shutdown?
> 
> net_init_bridge calls net_tap_fd_init which registers tap_cleanup.  That 
> closes the fd and frees associated memory.
> 
> The helper does not allocate a persistent tap device so closing the file 
> descriptor is sufficient for cleanup.

I think you should at least have the option to call a/the helper when
cleaning up too.

For the same reason that -net downscript= was added.  (From the names
I get the impression that was added later than -net script=, perhaps
due to the same kind of oversight ;-)

A user-supplied helper (I'm thinking of my needs) would create per-tap
iptables/ebtables rules and perhaps routing table entries, in addition
to creating the tap interface itself.  Routing table entries are
automatically deleted when the interface is, but iptables/ebtables
rules are not.  (Although it would be a nice little kernel addition if
they could be flagged to be.)

But now, thinking a bit more clearly... why is the helper separate
from "-net script="?

Overall, I envisage this happening (sorry for making up the name
tap-up-helper, as I seem to have lost your patch mails):

    tap-up-helper=      # Sets up tap interface, adds to bridge if needed.
    script=             # Sets up IP config

       ....QEMU runs....

    downscript=         # Removes IP config
    tap-down-helper=    # If only for symmetry!

We does tap-up-helper need to be separate from script?  Does QEMU need
to do something in between calling the two?

If not, then the existing "-net script=" script could just send the
tap descriptor as well as anything else it does, couldn't it?

-- Jamie

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[David Woodhouse]

On Tue, 2009-11-03 at 18:28 -0600, Anthony Liguori wrote:
> The most common use of -net tap is to connect a tap device to a bridge.  This
> requires the use of a script and running qemu as root in order to allocate a
> tap device to pass to the script. 

Does it?

Tap devices can be created (and configured) in advance, and can be
chowned so that they can be opened by an otherwise unprivileged user (or
group). You don't need root privileges to use a tap device.

-- 
dwmw2

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Anthony Liguori]

David Woodhouse wrote:
> On Tue, 2009-11-03 at 18:28 -0600, Anthony Liguori wrote:
>   
>> The most common use of -net tap is to connect a tap device to a bridge.  This
>> requires the use of a script and running qemu as root in order to allocate a
>> tap device to pass to the script. 
>>     
>
> Does it?
>
> Tap devices can be created (and configured) in advance, and can be
> chowned so that they can be opened by an otherwise unprivileged user (or
> group).

But that requires prior administrative access.

>  You don't need root privileges to use a tap device.
>   

You can access a preconfigured tap device but you cannot allocate a tap 
device and connect it to a bridge without CAP_NET_ADMIN.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Avi Kivity]

On 11/08/2009 12:11 AM, Anthony Liguori wrote:
>
>>  You don't need root privileges to use a tap device.
>
> You can access a preconfigured tap device but you cannot allocate a 
> tap device and connect it to a bridge without CAP_NET_ADMIN.

btw, shouldn't we, in the general case, create a bridge per user and use 
IP NAT?  If we have a global bridge, users can spoof each other's MAC 
addresses and interfere with their virtual machines.  They can also 
interfere with the real network.

That's not a concern with most one-user-per-machine configurations, but 
the default configuration should be safe.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Arnd Bergmann]

On Sunday 08 November 2009 08:27:41 Avi Kivity wrote:
> On 11/08/2009 12:11 AM, Anthony Liguori wrote:
> >
> >>  You don't need root privileges to use a tap device.
> >
> > You can access a preconfigured tap device but you cannot allocate a 
> > tap device and connect it to a bridge without CAP_NET_ADMIN.
> 
> btw, shouldn't we, in the general case, create a bridge per user and use 
> IP NAT?  If we have a global bridge, users can spoof each other's MAC 
> addresses and interfere with their virtual machines.  They can also 
> interfere with the real network.
> 
> That's not a concern with most one-user-per-machine configurations, but 
> the default configuration should be safe.

It also depends a lot on what you want to do with the virtual machine.
If you want to run a game or a legacy application in a different operating
system on your desktop, a NATed bridge is ideal, but it does not work
on a server if the guest wants to listen on a socket with its own IP address.

	Arnd <><

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Avi Kivity]

On 11/08/2009 10:43 AM, Arnd Bergmann wrote:
>> btw, shouldn't we, in the general case, create a bridge per user and use
>> IP NAT?  If we have a global bridge, users can spoof each other's MAC
>> addresses and interfere with their virtual machines.  They can also
>> interfere with the real network.
>>
>> That's not a concern with most one-user-per-machine configurations, but
>> the default configuration should be safe.
>>      
> It also depends a lot on what you want to do with the virtual machine.
> If you want to run a game or a legacy application in a different operating
> system on your desktop, a NATed bridge is ideal, but it does not work
> on a server if the guest wants to listen on a socket with its own IP address.
>    

Yes.  It also depends on what the system administrator wants you to be 
able to do.  On desktop machines you are usually the system 
administrator so there is no problem.  But we should beware of making it 
easy to subvert security.

There is also the problem of accidental MAC overlap - qemu uses the same 
MAC address for all virtual machines unless overridden, so if two users 
create a virtual machine without specifying MAC addresses they will 
trample each other.  A single user could also have trouble launching two 
guests; that's not a security problem, but will lead to a lot of 
annoyance and false bug reports ("networking dies as soon as I launch a 
second guest").

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Anthony Liguori]

Avi Kivity wrote:
> On 11/08/2009 12:11 AM, Anthony Liguori wrote:
>>
>>>  You don't need root privileges to use a tap device.
>>
>> You can access a preconfigured tap device but you cannot allocate a 
>> tap device and connect it to a bridge without CAP_NET_ADMIN.
>
> btw, shouldn't we, in the general case, create a bridge per user and 
> use IP NAT?  If we have a global bridge, users can spoof each other's 
> MAC addresses and interfere with their virtual machines.

qemu-bridge-helper supports that model quite well :-)  You would create 
a NAT'd bridge for each user as the administrator, then create a 
bridge.conf that consisted of per-user includes with appropriate 
permissions set on each of those files.

>   They can also interfere with the real network.
>
> That's not a concern with most one-user-per-machine configurations, 
> but the default configuration should be safe.

Let's not kid ourselves, no matter what we do we're giving a user 
elevated privileges.  Even with NAT, if the host can access the NAT'ed 
network, then you can run a privileged service (like NFS) in that 
network.  Like it or not, some networks rely on privileged services 
being trusted as part of their security model (consider NIS).

I think the best we can do is provide a tool that allows an 
administrator to grant users additional privileges in the tiniest 
increments possible.  Putting people in wheel just so they can do 
virtualization is too much.

I don't see having an fscap-based helper as creating policy.  I see it 
as adding a mechanism for administrators to create policy.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Jamie Lokier]

Anthony Liguori wrote:
> Let's not kid ourselves, no matter what we do we're giving a user 
> elevated privileges.  Even with NAT, if the host can access the NAT'ed 
> network, then you can run a privileged service (like NFS) in that 
> network.

I don't see how outgoing NAT (SNAT), where the guest can make
_outgoing_ connections to the network, allows the guest to run a
privileged service accessible to the network.  Sure, the guest can run
an NFS server, but it means nothing to the outside - it's on the
guest's own private little network.  Same as Slirp.

The guest cannot even make an outgoing request which appears to come
from an privileged port - if the SNAT rule has the appropriate options
to force the port into an unprivileged range.

For the guest's NFS server to be visible to the network requires
incoming NAT (DNAT) on the host, often called "port forwarding".  But
that is done by explicit administration; if you can do that, you can
run a privileged service on the host anyway.

> I think the best we can do is provide a tool that allows an 
> administrator to grant users additional privileges in the tiniest 
> increments possible.  Putting people in wheel just so they can do 
> virtualization is too much.
> 
> I don't see having an fscap-based helper as creating policy.  I see it 
> as adding a mechanism for administrators to create policy.

I agree with both of these.

-- Jamie

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Anthony Liguori]

Jamie Lokier wrote:
> Anthony Liguori wrote:
>   
>> Let's not kid ourselves, no matter what we do we're giving a user 
>> elevated privileges.  Even with NAT, if the host can access the NAT'ed 
>> network, then you can run a privileged service (like NFS) in that 
>> network.
>>     
>
> I don't see how outgoing NAT (SNAT), where the guest can make
> _outgoing_ connections to the network, allows the guest to run a
> privileged service accessible to the network.  Sure, the guest can run
> an NFS server, but it means nothing to the outside - it's on the
> guest's own private little network.  Same as Slirp.
>
> The guest cannot even make an outgoing request which appears to come
> from an privileged port - if the SNAT rule has the appropriate options
> to force the port into an unprivileged range.
>
> For the guest's NFS server to be visible to the network requires
> incoming NAT (DNAT) on the host, often called "port forwarding".  But
> that is done by explicit administration; if you can do that, you can
> run a privileged service on the host anyway.
>   

You are correct except that I qualified this as NAT with host access 
which so far is the common model.  If the host can access the NAT'd 
network behind the NAT, then port privileges are important.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Jamie Lokier]

Anthony Liguori wrote:
> You are correct except that I qualified this as NAT with host access 
> which so far is the common model.  If the host can access the NAT'd 
> network behind the NAT, then port privileges are important.

You're right.

This is why QEMU guests should be run inside an LXC container :-)

Or in the general case, a security-conscious net-setup script should
ensure general user invocations are limited to admin-decided subnets
with admin-decided firewall rules, so that they just look like
processes with ordinary access to everything else.

Iptables being what it is, that'd have to be distro specific and
sometimes site specific.

-- Jamie

Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge

[Avi Kivity]

On 11/09/2009 04:20 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>> On 11/08/2009 12:11 AM, Anthony Liguori wrote:
>>>
>>>>  You don't need root privileges to use a tap device.
>>>
>>> You can access a preconfigured tap device but you cannot allocate a 
>>> tap device and connect it to a bridge without CAP_NET_ADMIN.
>>
>> btw, shouldn't we, in the general case, create a bridge per user and 
>> use IP NAT?  If we have a global bridge, users can spoof each other's 
>> MAC addresses and interfere with their virtual machines.
>
> qemu-bridge-helper supports that model quite well :-)  You would 
> create a NAT'd bridge for each user as the administrator, then create 
> a bridge.conf that consisted of per-user includes with appropriate 
> permissions set on each of those files.

Except that the out-of-the-box experience is "go and configure things" 
again.

Really, bridging is fairly complex.  You have to set up the bridge, 
perhaps with NAT, perhaps to the physical device (possibly fighting a 
bit with NetworkManager in that case), allocate MAC addresses in a 
nonconflicting way, set up DHCP, and if you are multiuser, protect users 
from each other, and protect the host from guests.

>
>>   They can also interfere with the real network.
>>
>> That's not a concern with most one-user-per-machine configurations, 
>> but the default configuration should be safe.
>
> Let's not kid ourselves, no matter what we do we're giving a user 
> elevated privileges.  Even with NAT, if the host can access the NAT'ed 
> network, then you can run a privileged service (like NFS) in that 
> network.  Like it or not, some networks rely on privileged services 
> being trusted as part of their security model (consider NIS).

At least on commercial Linux vendor exports NFS home directories with 
the 'secure' option, requiring a privileged port to mount NFS shares.  
If your patch falls into the wrong hands, virtualization developers 
working at such a vendor might be able to access their home directories 
from guests, which would annoy IT immensely.

> I think the best we can do is provide a tool that allows an 
> administrator to grant users additional privileges in the tiniest 
> increments possible.  Putting people in wheel just so they can do 
> virtualization is too much.

I'm concerned that we're confusing the situation instead of clearing 
it.  Telling users to "edit /etc/foo/bar" so you can run bridging 
without fully explaining the impact is not responsible.  Trusting 
libvirt to do it end-to-end is fine, but in this case it can also set up 
the tap itself.

>
> I don't see having an fscap-based helper as creating policy.  I see it 
> as adding a mechanism for administrators to create policy.
>

I'll summarize my current issues with the patchset:

- by introducing an suid/fscap helper we're expanding the scope of 
security in qemu.  In addition to protecting qemu against malicious 
guests trying to gain local user privileges, we need to protect it 
against malicious local users trying to gain root (or CAP_NET_ADMIN) 
privileges

- we're introducing yet another authorization mechanism.  We have 
enough.  We already have feedback that policykit is wanted in at least 
some cases.  Given that authorization is best kept consistent in a 
distro, and that distros try to be as inconsistent from each other as 
possible, we can't please everyone.

- for the command-line user, we aren't providing a full solution; they 
still have a lot of configuration to do before things work (especially 
if they don't already have a network segment with DHCP)

- for libvirt and other management systems, it is insufficient since 
they will at least in some cases need to perform privileged 
configuration before handing the tap to the guest.  So they still need 
to be privileged and they can't use -net bridge for hotplug.

I would be happier if we drop suid/fscap from the helper.  We can have a 
user-provided script or binary gain the necessary privileges (using sudo 
or policykit or whatever).  This way we don't need to deal with local 
privilege escalation vulnerabilities.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Alexander Graf]

On 04.11.2009, at 01:28, Anthony Liguori wrote:

> This series solves a problem that I've been struggling with for a  
> few years now.
> One of the best things about qemu is that it's possible to run  
> guests as an
> unprivileged user to improve security.  However, if you want to have  
> your guests
> communicate with the outside world, you're pretty much forced to run  
> qemu as
> root.
>
> At least with KVM support, this is probably the most common use case  
> which means
> that most of our users are running qemu as root.  That's terrible.

Yeah. Worse than the "run as root" part is the "it's hard" part  
though. I hate how I feel when I try to explain someone how to use non- 
slirp networking :-(.

The response to that is then usually "oh whatever, it's too  
complicated anyways".

> We address this problem by introducing a new network backend: -net  
> bridge.  This
> backend is less flexible than -net tap because it relies on a helper  
> with
> elevated privileges to do the heavy lifting of allocating and  
> attaching a tap
> device to a bridge.  We use a special purpose helper because we  
> don't want
> to elevate the privileges of more generic tools like brctl.
>
> From a user perspective, to use bridged networking with a guest, you  
> simply use:
>
>  qemu -hda linux.img -net bridge -net nic
>
> And assuming a bridge is defined named qemubr0 and the administrator  
> has setup
> permissions accordingly, it will Just Work.  My hope is that  
> distributions will
> do this work as part of the qemu packaging process such that for  
> most users,
> the out-of-the-box experience will also Just Work.

Yeah, that won't work as easily.

When your customer has 2 NICs this will already break. But let's  
imagine we only have a single NIC.

So that NIC is a wifi card. When I set up the qemubr0 bridge for that  
one now, how does network manager configure my wifi access? It can't  
use the bridge device, as that doesn't have wifi extensions. It also  
can't use the wifi device, because setting up networking on that will  
break.

IMHO the only customer friendly choice I see is the ugly way. The way  
VMware and Vbox do it.
To make it a bit less ugly, maybe we could create some way to "glue" a  
tap device and an eth together, the same way the bridge code does,  
just without the extra device.


Alex

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Alexander Graf wrote:
> Yeah. Worse than the "run as root" part is the "it's hard" part 
> though. I hate how I feel when I try to explain someone how to use 
> non-slirp networking :-(.
>
> The response to that is then usually "oh whatever, it's too 
> complicated anyways".

I agree and it's a problem I would like to solve too.

>> And assuming a bridge is defined named qemubr0 and the administrator 
>> has setup
>> permissions accordingly, it will Just Work.  My hope is that 
>> distributions will
>> do this work as part of the qemu packaging process such that for most 
>> users,
>> the out-of-the-box experience will also Just Work.
>
> Yeah, that won't work as easily.
>
> When your customer has 2 NICs this will already break. But let's 
> imagine we only have a single NIC.
>
> So that NIC is a wifi card. When I set up the qemubr0 bridge for that 
> one now, how does network manager configure my wifi access? It can't 
> use the bridge device, as that doesn't have wifi extensions. It also 
> can't use the wifi device, because setting up networking on that will 
> break.
>
> IMHO the only customer friendly choice I see is the ugly way. The way 
> VMware and Vbox do it.
> To make it a bit less ugly, maybe we could create some way to "glue" a 
> tap device and an eth together, the same way the bridge code does, 
> just without the extra device.

I don't think that helps either.  At the end of the day, this is really 
a policy decision.  You want to expose the subset of functionality that 
the majority of your users require.  Inevitably, you'll leave some users 
who need more complex setups on their own.

For instance, users with one NIC are certainly common.  Two or more NICs 
are also pretty common.  But what about people that require guests to be 
on separate VLANs?

This is where management tools come in.  When running qemu by hand, the 
management tool is the distro more or less as they will be choosing the 
default policies.  What we need to think about is how to make sure we 
can seamlessly integrate with whatever policies they want to implement.

I think the one thing we could add is a configurable message to print 
when a user tries to use a bridge that isn't configured yet.  For 
instance, if a user runs qemu without a bridge setup, what would be nice 
is the following:

$ qemu -net bridge -net nic -hda ~/images/linux.img
qemu: error running bridge helper

You currently do not have 'qemubr0' configured.  To setup qemubr0, run 
(as root):
  zypper install qemu-network-setup
  qemu-network-setup qemubr0

$ sudo qemu-network-setup
Which interface do you want to configure [qemubr0]: qemubr0
Do you want to configure qemubr0 as NAT or Bridge [NAT]: Bridge
Which physical interface do you want to bridge qemubr0 to [eth0]: eth0
qemubr0 is now configured.

I expect that this is going to be different for every distro.  netcf my 
make this easier but for now, I think this is the most realistic approach.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Alexander Graf]

Anthony Liguori wrote:
> Alexander Graf wrote:
>> Yeah. Worse than the "run as root" part is the "it's hard" part
>> though. I hate how I feel when I try to explain someone how to use
>> non-slirp networking :-(.
>>
>> The response to that is then usually "oh whatever, it's too
>> complicated anyways".
>
> I agree and it's a problem I would like to solve too.
>
>>> And assuming a bridge is defined named qemubr0 and the administrator
>>> has setup
>>> permissions accordingly, it will Just Work.  My hope is that
>>> distributions will
>>> do this work as part of the qemu packaging process such that for
>>> most users,
>>> the out-of-the-box experience will also Just Work.
>>
>> Yeah, that won't work as easily.
>>
>> When your customer has 2 NICs this will already break. But let's
>> imagine we only have a single NIC.
>>
>> So that NIC is a wifi card. When I set up the qemubr0 bridge for that
>> one now, how does network manager configure my wifi access? It can't
>> use the bridge device, as that doesn't have wifi extensions. It also
>> can't use the wifi device, because setting up networking on that will
>> break.
>>
>> IMHO the only customer friendly choice I see is the ugly way. The way
>> VMware and Vbox do it.
>> To make it a bit less ugly, maybe we could create some way to "glue"
>> a tap device and an eth together, the same way the bridge code does,
>> just without the extra device.
>
> I don't think that helps either.  At the end of the day, this is
> really a policy decision.  You want to expose the subset of
> functionality that the majority of your users require.  Inevitably,
> you'll leave some users who need more complex setups on their own.
>
> For instance, users with one NIC are certainly common.  Two or more
> NICs are also pretty common.  But what about people that require
> guests to be on separate VLANs?
>
> This is where management tools come in.  When running qemu by hand,
> the management tool is the distro more or less as they will be
> choosing the default policies.  What we need to think about is how to
> make sure we can seamlessly integrate with whatever policies they want
> to implement.
>
> I think the one thing we could add is a configurable message to print
> when a user tries to use a bridge that isn't configured yet.  For
> instance, if a user runs qemu without a bridge setup, what would be
> nice is the following:
>
> $ qemu -net bridge -net nic -hda ~/images/linux.img
> qemu: error running bridge helper
>
> You currently do not have 'qemubr0' configured.  To setup qemubr0, run
> (as root):
>  zypper install qemu-network-setup
>  qemu-network-setup qemubr0
>
> $ sudo qemu-network-setup
> Which interface do you want to configure [qemubr0]: qemubr0
> Do you want to configure qemubr0 as NAT or Bridge [NAT]: Bridge
> Which physical interface do you want to bridge qemubr0 to [eth0]: eth0
> qemubr0 is now configured.
>
> I expect that this is going to be different for every distro.  netcf
> my make this easier but for now, I think this is the most realistic
> approach.

Well I'm not that familiar with the bridging stuff as I'm rather scared
by it myself, but last time I tried if I

# brctl addif br0 eth0
# ifconfig br0 up

eth0 stopped working, so I had to stop network manager, assign an IP to
br0 manually and hope network manager doesn't kick in again. I don't see
how we can solve that easily, as most people will want to use NM.

Alex

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Alexander Graf wrote:
> Well I'm not that familiar with the bridging stuff as I'm rather scared
> by it myself, but last time I tried if I
>
> # brctl addif br0 eth0
> # ifconfig br0 up
>
> eth0 stopped working, so I had to stop network manager, assign an IP to
> br0 manually and hope network manager doesn't kick in again. I don't see
> how we can solve that easily, as most people will want to use NM.
>   

netcf will address this properly because it will allow NM to understand 
bridges and provide a common framework for NM to know about when changes 
are made to static networking configuration.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Michael S. Tsirkin]

On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote:
> This series solves a problem that I've been struggling with for a few years now.
> One of the best things about qemu is that it's possible to run guests as an
> unprivileged user to improve security.  However, if you want to have your guests
> communicate with the outside world, you're pretty much forced to run qemu as
> root.
> 
> At least with KVM support, this is probably the most common use case which means
> that most of our users are running qemu as root.  That's terrible.
> 
> We address this problem by introducing a new network backend: -net bridge.  This
> backend is less flexible than -net tap because it relies on a helper with
> elevated privileges to do the heavy lifting of allocating and attaching a tap
> device to a bridge.  We use a special purpose helper because we don't want
> to elevate the privileges of more generic tools like brctl.
> 
> >From a user perspective, to use bridged networking with a guest, you simply use:
> 
>   qemu -hda linux.img -net bridge -net nic
> 
> And assuming a bridge is defined named qemubr0 and the administrator has setup
> permissions accordingly, it will Just Work.  My hope is that distributions will
> do this work as part of the qemu packaging process such that for most users,
> the out-of-the-box experience will also Just Work.
> 
> More details are included in individual patches.  I broke up the helper into
> a series of patches to improve reviewabilty.

Would raw backend attached to a bridge mostly do the same?

-- 
MST

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Michael S. Tsirkin wrote:
> On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote:
>   
>> This series solves a problem that I've been struggling with for a few years now.
>> One of the best things about qemu is that it's possible to run guests as an
>> unprivileged user to improve security.  However, if you want to have your guests
>> communicate with the outside world, you're pretty much forced to run qemu as
>> root.
>>
>> At least with KVM support, this is probably the most common use case which means
>> that most of our users are running qemu as root.  That's terrible.
>>
>> We address this problem by introducing a new network backend: -net bridge.  This
>> backend is less flexible than -net tap because it relies on a helper with
>> elevated privileges to do the heavy lifting of allocating and attaching a tap
>> device to a bridge.  We use a special purpose helper because we don't want
>> to elevate the privileges of more generic tools like brctl.
>>
>> >From a user perspective, to use bridged networking with a guest, you simply use:
>>
>>   qemu -hda linux.img -net bridge -net nic
>>
>> And assuming a bridge is defined named qemubr0 and the administrator has setup
>> permissions accordingly, it will Just Work.  My hope is that distributions will
>> do this work as part of the qemu packaging process such that for most users,
>> the out-of-the-box experience will also Just Work.
>>
>> More details are included in individual patches.  I broke up the helper into
>> a series of patches to improve reviewabilty.
>>     
>
> Would raw backend attached to a bridge mostly do the same?
>   

Well it doesn't really help with the issue of privileges which is what 
this series is really about.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Michael S. Tsirkin]

On Wed, Nov 04, 2009 at 01:48:01PM -0600, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote:
>>   
>>> This series solves a problem that I've been struggling with for a few years now.
>>> One of the best things about qemu is that it's possible to run guests as an
>>> unprivileged user to improve security.  However, if you want to have your guests
>>> communicate with the outside world, you're pretty much forced to run qemu as
>>> root.
>>>
>>> At least with KVM support, this is probably the most common use case which means
>>> that most of our users are running qemu as root.  That's terrible.
>>>
>>> We address this problem by introducing a new network backend: -net bridge.  This
>>> backend is less flexible than -net tap because it relies on a helper with
>>> elevated privileges to do the heavy lifting of allocating and attaching a tap
>>> device to a bridge.  We use a special purpose helper because we don't want
>>> to elevate the privileges of more generic tools like brctl.
>>>
>>> >From a user perspective, to use bridged networking with a guest, you simply use:
>>>
>>>   qemu -hda linux.img -net bridge -net nic
>>>
>>> And assuming a bridge is defined named qemubr0 and the administrator has setup
>>> permissions accordingly, it will Just Work.  My hope is that distributions will
>>> do this work as part of the qemu packaging process such that for most users,
>>> the out-of-the-box experience will also Just Work.
>>>
>>> More details are included in individual patches.  I broke up the helper into
>>> a series of patches to improve reviewabilty.
>>>     
>>
>> Would raw backend attached to a bridge mostly do the same?
>>   
>
> Well it doesn't really help with the issue of privileges which is what  
> this series is really about.
>
> Regards,
>
> Anthony Liguori

I note that by default you grant all users all access.
If you do that, just give them net cap admin already?

-- 
MST

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Michael S. Tsirkin wrote:
>> Well it doesn't really help with the issue of privileges which is what  
>> this series is really about.
>>
>> Regards,
>>
>> Anthony Liguori
>>     
>
> I note that by default you grant all users all access.
> If you do that, just give them net cap admin already?
>   

By default, I give no users any access.

qemu-bridge-helper carries cap_net_admin but it doesn't do everything 
cap_net_admin does.  Since an administrator has to set that capability, 
the admin is going to make it owned by root so that an unprivileged user 
cannot change it.  Modulo bugs, it's a very restricted subset of 
cap_net_admin.

In order for a user to be able to get a tap device connected to a 
bridge, the following things must be true:

1) the user must have execute privileges for qemu-bridge-helper
2) the user must have read/write access to /dev/net/tun
3) there must be an /etc/qemu/bridge.conf that is readable by the user
4) the config must have an explicit rule allowing access to the required 
bridge device

So the user is very restricted in what they can do and they must be 
granted these permissions explicitly by an administrator.  By using 
multiple bridge.conf files, an administrator can also create policies 
based on filesystem permissions allowing certain user/groups to access 
only certain bridges.

With raw, qemu must carry cap_net_raw.   That is definitely not safe for 
an untrusted user.  Allowing an untrusted user to connect a VM to a 
bridged physical network, on the other hand, seems to be a rather safe 
thing to do as long as there are strongly ways to control which bridges 
they can connect to.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Michael S. Tsirkin]

On Wed, Nov 04, 2009 at 02:44:26PM -0600, Anthony Liguori wrote:
> Michael S. Tsirkin wrote:
>>> Well it doesn't really help with the issue of privileges which is 
>>> what  this series is really about.
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>     
>>
>> I note that by default you grant all users all access.
>> If you do that, just give them net cap admin already?
>>   
>
> By default, I give no users any access.

Oh, I misunderstood. This is what gave me the idea:

] If we fail to include an acl file, we are silent about it making this mechanism
] work pretty seamlessly.

What did you mean, in fact?

-- 
MST

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Michael S. Tsirkin wrote:
> On Wed, Nov 04, 2009 at 02:44:26PM -0600, Anthony Liguori wrote:
>   
>> Michael S. Tsirkin wrote:
>>     
>>>> Well it doesn't really help with the issue of privileges which is 
>>>> what  this series is really about.
>>>>
>>>> Regards,
>>>>
>>>> Anthony Liguori
>>>>     
>>>>         
>>> I note that by default you grant all users all access.
>>> If you do that, just give them net cap admin already?
>>>   
>>>       
>> By default, I give no users any access.
>>     
>
> Oh, I misunderstood. This is what gave me the idea:
>
> ] If we fail to include an acl file, we are silent about it making this mechanism
> ] work pretty seamlessly.
>
> What did you mean, in fact?
>   

The default policy is deny all.  If we fail to include the main acl 
file, we throw an error.  If the main acl file includes another acl 
file, and that file cannot be read (because of EPERM), we are silent.  
This allows the use of additional included acl files that have different 
file permissions.  This is how we use filesystem permissions to 
implement more sophisticated acls.

It's kind of weird, but I like the fact that the enforce is done by the 
OS as opposed to having the enforcement done by the helper.

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1925 bytes --]

On Tue, 2009-11-03 at 18:28 -0600, Anthony Liguori wrote:
> This series solves a problem that I've been struggling with for a few years now.
> One of the best things about qemu is that it's possible to run guests as an
> unprivileged user to improve security.  However, if you want to have your guests
> communicate with the outside world, you're pretty much forced to run qemu as
> root.
> 
> At least with KVM support, this is probably the most common use case which means
> that most of our users are running qemu as root.  That's terrible.

Ack.

> We address this problem by introducing a new network backend: -net bridge.  This
> backend is less flexible than -net tap because it relies on a helper with
> elevated privileges to do the heavy lifting of allocating and attaching a tap
> device to a bridge.  We use a special purpose helper because we don't want
> to elevate the privileges of more generic tools like brctl.
> 
> From a user perspective, to use bridged networking with a guest, you simply use:
> 
>   qemu -hda linux.img -net bridge -net nic

I know that this patch is less than a day old and untested, but would it
be reasonable to make this the "default" network configuration at some
point in the future?  This certainly seems to be what I want 99% of the
time when I launch qemu or kvm by hand from the command line.

> And assuming a bridge is defined named qemubr0 and the administrator has setup
> permissions accordingly, it will Just Work.  My hope is that distributions will
> do this work as part of the qemu packaging process such that for most users,
> the out-of-the-box experience will also Just Work.

Also, ack.  I'll handle the Ubuntu packaging to enable this support in
Lucid by the time qemu-0.12-rc1 is available.  As Alexander mentions,
there's a bit more complexity we'll need to account for (wifi, network
manager, multiple nic's).

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Dustin Kirkland wrote:
>> We address this problem by introducing a new network backend: -net bridge.  This
>> backend is less flexible than -net tap because it relies on a helper with
>> elevated privileges to do the heavy lifting of allocating and attaching a tap
>> device to a bridge.  We use a special purpose helper because we don't want
>> to elevate the privileges of more generic tools like brctl.
>>
>> From a user perspective, to use bridged networking with a guest, you simply use:
>>
>>   qemu -hda linux.img -net bridge -net nic
>>     
>
> I know that this patch is less than a day old and untested, but would it
> be reasonable to make this the "default" network configuration at some
> point in the future?  This certainly seems to be what I want 99% of the
> time when I launch qemu or kvm by hand from the command line.
>   

I'd rather make the "default" network configurable via a global 
configuration file.  That way, if a distribution knew that it had a 
bridge setup for its users, it could make -net bridge the default.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

On Wed, 2009-11-04 at 18:52 -0600, Anthony Liguori wrote:
> I'd rather make the "default" network configurable via a global 
> configuration file.  That way, if a distribution knew that it had a 
> bridge setup for its users, it could make -net bridge the default.

Fair enough.

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Anthony Liguori wrote:
> This series solves a problem that I've been struggling with for a
> few years now.  One of the best things about qemu is that it's
> possible to run guests as an unprivileged user to improve security.
> However, if you want to have your guests communicate with the
> outside world, you're pretty much forced to run qemu as root.
> 
> At least with KVM support, this is probably the most common use case
> which means that most of our users are running qemu as root.  That's
> terrible.

I've been using KVM for a year or so with bridged networking,
without needing to run it as root.

So there's obviously been some bad advice floating around :-)

What I do is have tun interfaces owned by the user...

> We address this problem by introducing a new network backend: -net bridge.  This
> backend is less flexible than -net tap because it relies on a helper with
> elevated privileges to do the heavy lifting of allocating and attaching a tap
> device to a bridge.  We use a special purpose helper because we don't want
> to elevate the privileges of more generic tools like brctl.

I think it's a great idea.

It would be even more useful if the "bridge setup" and "bridge
teardown" helper commands could be specified as qemu options, and
eventually as config file entries, in the same way as script= and
downscript= are already.

That way it would be useful for all sorts of network configurations
where a user-allocatable tap+bridge interface is needed.

You can do it now by wrapping _around_ qemu using the -net tap,fd=N
option, but calling a helper to get assigned a tap interface is much neater.

By the way, would it make sense for the helper to return an open file
descriptor rather than an interface name, just like -net tap,fd=N
uses?  Or does qemu need to access the interface by name anyway?

I notice that if we eventually teach the kernel to have unnamed
bridges (just attach interfaces to each other), only the helper
commands will need changing to use it :-)

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Michael S. Tsirkin]

On Thu, Nov 05, 2009 at 04:12:36AM +0000, Jamie Lokier wrote:
> I notice that if we eventually teach the kernel to have unnamed
> bridges (just attach interfaces to each other), only the helper
> commands will need changing to use it :-)

What do you mean by "attach interfaces to each other"?
Which interfaces do you want to attach to each other?

-- 
MST

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Michael S. Tsirkin wrote:
> On Thu, Nov 05, 2009 at 04:12:36AM +0000, Jamie Lokier wrote:
> > I notice that if we eventually teach the kernel to have unnamed
> > bridges (just attach interfaces to each other), only the helper
> > commands will need changing to use it :-)
> 
> What do you mean by "attach interfaces to each other"?
> Which interfaces do you want to attach to each other?

For example, attaching eth0 to tap0 without having to define br0,
instead of the current method where you define br0 then attach eth0
and tap0 to it, then transfer eth0's IP configuration to br0, then
tell all the network management tools about it.

Even Anthony's helper program only makes it easier to do dynamic taps
attaching to br0; it won't work on a vanilla Linux system.  A vanilla
Linux system still needs to be modified to have br0 before the helper
can work, and while that modification would be easy for distros to do
for most home systems, which run NetworkManager and are limited to
exactly one active interface, it's not a pretty change for anything
else and cannot be fully automated.

(Even on home systems, sometimes you'd want vMs bridged to your mobile
phone internet connection, and sometimes you wouldn't.  There are
still policy issues).

Unnamed bridges would simplify the problem on systems which aren't
running something like NetworkManager, by keeping eth0's IP
configuration on eth0, and Anthony's helper would just attach tap0 to
eth0 (or whatever interface you prefer).  All you specify is which
real network you want the VM's virtual NIC to appear on.

It'd behave like VMware etc., but cleaner :-)

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Arnd Bergmann]

On Friday 06 November 2009, Jamie Lokier wrote:
> 
> Michael S. Tsirkin wrote:
> > On Thu, Nov 05, 2009 at 04:12:36AM +0000, Jamie Lokier wrote:
> > > I notice that if we eventually teach the kernel to have unnamed
> > > bridges (just attach interfaces to each other), only the helper
> > > commands will need changing to use it :-)
> > 
> > What do you mean by "attach interfaces to each other"?
> > Which interfaces do you want to attach to each other?
> 
> For example, attaching eth0 to tap0 without having to define br0,
> instead of the current method where you define br0 then attach eth0
> and tap0 to it, then transfer eth0's IP configuration to br0, then
> tell all the network management tools about it.

"Attaching" two network interfaces to each other does not work the
way you'd like it to. The tap device is on it's own unless you use
either a bridge or configure IP forwarding, which has another set
of problems.

I posted an experimental 'macvtap' driver a few some time ago that
would allow you to do something like that, i.e. add another logical
interface to an existing eth0 device, with a separate mac address,
and export that as a character device that is compatible with /dev/tun.

I probably need to start that one over from scratch, because I also
want to make it work with vhost-net, and to allow the guest and
the host to communicate with each other.
Note that this would be transparent to qemu, we'd only need a different
helper script to call.

	Arnd <><

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Arnd Bergmann wrote:
> On Friday 06 November 2009, Jamie Lokier wrote:
> > 
> > Michael S. Tsirkin wrote:
> > > On Thu, Nov 05, 2009 at 04:12:36AM +0000, Jamie Lokier wrote:
> > > > I notice that if we eventually teach the kernel to have unnamed
> > > > bridges (just attach interfaces to each other), only the helper
> > > > commands will need changing to use it :-)
> > > 
> > > What do you mean by "attach interfaces to each other"?
> > > Which interfaces do you want to attach to each other?
> > 
> > For example, attaching eth0 to tap0 without having to define br0,
> > instead of the current method where you define br0 then attach eth0
> > and tap0 to it, then transfer eth0's IP configuration to br0, then
> > tell all the network management tools about it.
> 
> "Attaching" two network interfaces to each other does not work the
> way you'd like it to. The tap device is on it's own unless you use
> either a bridge or configure IP forwarding, which has another set
> of problems.

That's why I called it "unnamed bridges".  There still needs to be a
bridge doing the usual MAC learning, possibly spanning tree, etc. to
have the right behaviour.

The only trouble is having to configure the local IP settings on the
bridge itself, which entails modifying every network configuration
tool, every script and so on.

Hence the idea of allowing attached physical interfaces to have
individual local IP settings, and (perhaps) not bothering with a
separate named interface for the bridge, instead just allow brctl to
be given a physical interface and instantiate a "slave" bridge which
is always attached to that interface and referenced through it.

So instead of:

    brctl addbr br0
    brctl addif br0 eth0 # Already inherits MAC from the first attached if.
    brctl addif br0 tap0
    ifconfig br0 1.2.3.4  # Move IP configuration from eth0 to br0
    ifconfig eth0 0.0.0.0

You'd do:

    brctl addif eth0 tap0

Much simpler, and would Just Work with so many more things.  It's just
configuration sugar really.  Underneath there would still be a bridge,
using the Linux bridge code, with two interfaces attached.

> I posted an experimental 'macvtap' driver a few some time ago that
> would allow you to do something like that, i.e. add another logical
> interface to an existing eth0 device, with a separate mac address,
> and export that as a character device that is compatible with /dev/tun.

(That's not as useful as unnamed bridges because it only works with tun
devices, but that's not relevant to QEMU/KVM).

If the guest contains it's own bridges, or even it's own macvtaps, you
need bridge MAC learning (and perhaps STP to avoid cycles) to forward
packets efficiently.  Otherwise you have to either forward all packets
to the guest (won't do that obviously), or some guest configurations
just won't work as they'd expect.

Of course it's ok if "interesting" guests don't work properly.
macvtap can come with documented restrictions (they are a useful form
of security anyway), and people can use proper host bridges like they
already do for guests who need to behave like they're plugged into the
host network properly.

But imho it'd be far nicer not to have "specialness" like that unless
you want it for some positive reason like security.

> I probably need to start that one over from scratch, because I also
> want to make it work with vhost-net, and to allow the guest and
> the host to communicate with each other.
> Note that this would be transparent to qemu, we'd only need a different
> helper script to call.

Helper scripts are the new management tools I hear ;-)

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Michael S. Tsirkin]

On Fri, Nov 06, 2009 at 08:26:29PM +0000, Jamie Lokier wrote:
> But imho it'd be far nicer not to have "specialness" like that unless
> you want it for some positive reason like security.

OTOH, if you do want this for security, there's currently no
way to do this besides using raw sockets + macvlan.

-- 
MST

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Jamie Lokier wrote:
> Anthony Liguori wrote:
>   
>> At least with KVM support, this is probably the most common use case
>> which means that most of our users are running qemu as root.  That's
>> terrible.
>>     
>
> I've been using KVM for a year or so with bridged networking,
> without needing to run it as root.
>
> So there's obviously been some bad advice floating around :-)
>
> What I do is have tun interfaces owned by the user...
>   

I mention this in one of my patches.  The problem with persistent tun 
devices is that every time a user needs to create a guest, an 
adminstrator action is needed.  That doesn't extend well to a system 
that Just Works.

>> We address this problem by introducing a new network backend: -net bridge.  This
>> backend is less flexible than -net tap because it relies on a helper with
>> elevated privileges to do the heavy lifting of allocating and attaching a tap
>> device to a bridge.  We use a special purpose helper because we don't want
>> to elevate the privileges of more generic tools like brctl.
>>     
>
> I think it's a great idea.
>
> It would be even more useful if the "bridge setup" and "bridge
> teardown" helper commands could be specified as qemu options, and
> eventually as config file entries, in the same way as script= and
> downscript= are already.
>   

The problem with this is that the helper runs with increased 
capabilities but as a lesser user.  Various networking commands don't 
necessarily behave well when run like this.  But more importantly, you 
would have to run the helper with inheritable privileges which is a 
bigger security risk than what we do at the moment.

There's only one way to add a device to a bridge so I'm not sure it's 
really necessary either.

> That way it would be useful for all sorts of network configurations
> where a user-allocatable tap+bridge interface is needed.
>
> You can do it now by wrapping _around_ qemu using the -net tap,fd=N
> option, but calling a helper to get assigned a tap interface is much neater.
>
> By the way, would it make sense for the helper to return an open file
> descriptor rather than an interface name, just like -net tap,fd=N
> uses?  Or does qemu need to access the interface by name anyway?
>   

The helper does return an fd.  The internal implementation is pretty 
much -net tap,fd=N.  You can point it to a different helper if you'd 
like by saying -net bridge,helper=my-helper.

That said, the helper has to have greater privileges than qemu itself so 
it's something that needs to be very carefully implemented/designed.  I 
don't think it's something you want normal users mucking around with.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/04/2009 02:28 AM, Anthony Liguori wrote:
> This series solves a problem that I've been struggling with for a few years now.
> One of the best things about qemu is that it's possible to run guests as an
> unprivileged user to improve security.  However, if you want to have your guests
> communicate with the outside world, you're pretty much forced to run qemu as
> root.
>    

Only is you run an unmanaged system.

> At least with KVM support, this is probably the most common use case which means
> that most of our users are running qemu as root.  That's terrible.
>    

Most of our users run managed systems.

> We address this problem by introducing a new network backend: -net bridge.  This
> backend is less flexible than -net tap because it relies on a helper with
> elevated privileges to do the heavy lifting of allocating and attaching a tap
> device to a bridge.  We use a special purpose helper because we don't want
> to elevate the privileges of more generic tools like brctl.
>    

But you're essentially allowing any user to send packets through the 
physical interface.  Including, say, nobody (who's just broken in 
through an unrelated service).

>  From a user perspective, to use bridged networking with a guest, you simply use:
>
>    qemu -hda linux.img -net bridge -net nic
>
> And assuming a bridge is defined named qemubr0 and the administrator has setup
> permissions accordingly, it will Just Work.  My hope is that distributions will
> do this work as part of the qemu packaging process such that for most users,
> the out-of-the-box experience will also Just Work.
>    

For most distributions, the ootb experience is already much better than 
running qemu on the command line.  I'm not sure who we're optimizing for 
here, and concerned that we're loosening security for qemu non-users.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 04:33 PM, Avi Kivity wrote:
> and concerned that we're loosening security for qemu non-users.
>

I see you've addressed this via an acl system.  Still, this is IMO 
should be outside qemu, esp. as security is now much more than 
users/groups (i.e. selinux and friends).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Daniel P. Berrange]

On Thu, Nov 05, 2009 at 04:36:19PM +0200, Avi Kivity wrote:
> On 11/05/2009 04:33 PM, Avi Kivity wrote:
> >and concerned that we're loosening security for qemu non-users.
> >
> 
> I see you've addressed this via an acl system.  Still, this is IMO 
> should be outside qemu, esp. as security is now much more than 
> users/groups (i.e. selinux and friends).

IMHO this needs to hook into PolicyKit, since that is the access control
framework that is being standardized on across the desktop. It is quite
easy to work with - all you need do is provide a policy file, and to
authorize a user, you'd run the 'pkcheck' program and its exit status
gives the result. 

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Thu, Nov 05, 2009 at 04:36:19PM +0200, Avi Kivity wrote:
>   
>> On 11/05/2009 04:33 PM, Avi Kivity wrote:
>>     
>>> and concerned that we're loosening security for qemu non-users.
>>>
>>>       
>> I see you've addressed this via an acl system.  Still, this is IMO 
>> should be outside qemu, esp. as security is now much more than 
>> users/groups (i.e. selinux and friends).
>>     
>
> IMHO this needs to hook into PolicyKit, since that is the access control
> framework that is being standardized on across the desktop. It is quite
> easy to work with - all you need do is provide a policy file, and to
> authorize a user, you'd run the 'pkcheck' program and its exit status
> gives the result. 
>   

Absolutely.  I wanted to not have a hard dependency on PolicyKit to 
start out with but that's always been the plan.  I'd like to eventually 
add an optional PolicyKit dependency and when that's available not even 
bother with the qemu acl file.  The nice thing about PolicyKit is the 
desktop integration.  It's a much better user experience to allow a user 
to be prompted to allow qemu to access a bridge vs. having to error out 
to the user and tell them to muck with a config file.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Anthony Liguori wrote:
> Absolutely.  I wanted to not have a hard dependency on PolicyKit to 
> start out with but that's always been the plan.  I'd like to eventually 
> add an optional PolicyKit dependency and when that's available not even 
> bother with the qemu acl file.  The nice thing about PolicyKit is the 
> desktop integration.  It's a much better user experience to allow a user 
> to be prompted to allow qemu to access a bridge vs. having to error out 
> to the user and tell them to muck with a config file.

Please do keep it optional.

PolicyKit is actively unhelpful when you're configuring a remote
server which doesn't have a desktop, or you don't have access to it's
desktop.

It's also unhelpful when you're trying to script something.  The last
thing you want a test harness script to do is prompt the user.

I much prefer command line errors in both those cases :-)

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Daniel P. Berrange]

On Thu, Nov 05, 2009 at 04:41:45PM +0000, Jamie Lokier wrote:
> Anthony Liguori wrote:
> > Absolutely.  I wanted to not have a hard dependency on PolicyKit to 
> > start out with but that's always been the plan.  I'd like to eventually 
> > add an optional PolicyKit dependency and when that's available not even 
> > bother with the qemu acl file.  The nice thing about PolicyKit is the 
> > desktop integration.  It's a much better user experience to allow a user 
> > to be prompted to allow qemu to access a bridge vs. having to error out 
> > to the user and tell them to muck with a config file.
> 
> Please do keep it optional.
> 
> PolicyKit is actively unhelpful when you're configuring a remote
> server which doesn't have a desktop, or you don't have access to it's
> desktop.
> 
> It's also unhelpful when you're trying to script something.  The last
> thing you want a test harness script to do is prompt the user.

PolicyKit has no fundamental requirement for a graphical prompt. The
default policy file for an app might require prompting, but is it
easy to add a admin defined policy override. The separation of app
logic from the authentication policy also makes it easy to provide
different ways of prompting, whether graphical, or command line based,
or totally disabled.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Daniel P. Berrange wrote:
> On Thu, Nov 05, 2009 at 04:41:45PM +0000, Jamie Lokier wrote:
> > Anthony Liguori wrote:
> > > Absolutely.  I wanted to not have a hard dependency on PolicyKit to 
> > > start out with but that's always been the plan.  I'd like to eventually 
> > > add an optional PolicyKit dependency and when that's available not even 
> > > bother with the qemu acl file.  The nice thing about PolicyKit is the 
> > > desktop integration.  It's a much better user experience to allow a user 
> > > to be prompted to allow qemu to access a bridge vs. having to error out 
> > > to the user and tell them to muck with a config file.
> > 
> > Please do keep it optional.
> > 
> > PolicyKit is actively unhelpful when you're configuring a remote
> > server which doesn't have a desktop, or you don't have access to it's
> > desktop.
> > 
> > It's also unhelpful when you're trying to script something.  The last
> > thing you want a test harness script to do is prompt the user.
> 
> PolicyKit has no fundamental requirement for a graphical prompt. The
> default policy file for an app might require prompting, but is it
> easy to add a admin defined policy override. The separation of app
> logic from the authentication policy also makes it easy to provide
> different ways of prompting, whether graphical, or command line based,
> or totally disabled.

I'll question "easy".  PolicyKit remains an un-unixlike mystery to my
addled brain, which can't keep up with the high rate of new things and
high rate of obsoleting their own things which comes out of
freedesktop.org.

Can you override the policy for a specific instance with an
environment variable, without knowing anything about the original
policy (i.e. "just give me errors or succeed" :-) and without having
to learn all about PolicyKit, as you would want to when executing qemu
from some scripts?

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> On 11/05/2009 04:33 PM, Avi Kivity wrote:
>> and concerned that we're loosening security for qemu non-users.
>>
>
> I see you've addressed this via an acl system.  Still, this is IMO 
> should be outside qemu, esp. as security is now much more than 
> users/groups (i.e. selinux and friends).

Actually, I think this model is pretty close to what the latest crazes 
are in the security world.  The model you're advocating (privileged 
process handing over a fd) is not as secure because it requires that the 
management daemon runs as a privileged user.  There's nothing about this 
that prevents the use of a management framework.  In fact, had this 
existed when libvirt was first written, I'd hope libvirt would have used 
this mechanism instead of fd inheritance.

Management software is really just another user.  We really want 
management software to run unprivileged as much as possible.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 04:50 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>> On 11/05/2009 04:33 PM, Avi Kivity wrote:
>>> and concerned that we're loosening security for qemu non-users.
>>>
>>
>> I see you've addressed this via an acl system.  Still, this is IMO 
>> should be outside qemu, esp. as security is now much more than 
>> users/groups (i.e. selinux and friends).
>
> Actually, I think this model is pretty close to what the latest crazes 
> are in the security world.

I think we need to isolate qemu for these crazes.  First, we're hardly 
security experts here.  Second, anything we do will be bad for someone.  
Third, security models vary across distributions and time.

> The model you're advocating (privileged process handing over a fd) is 
> not as secure because it requires that the management daemon runs as a 
> privileged user.

Or it can acquire an fd from a privileged helper and pass it on or 
conjure it some other way.

>   There's nothing about this that prevents the use of a management 
> framework.  In fact, had this existed when libvirt was first written, 
> I'd hope libvirt would have used this mechanism instead of fd 
> inheritance.
>
> Management software is really just another user.  We really want 
> management software to run unprivileged as much as possible.

I'd like to see qemu confined to managing a single guest and not expand 
to system management.  We have enough to do without taking over 
management systems and security.

Bridged network configuration is painful now, but only for a handful of 
users (us developers).  For the vast majority it is handled behind their 
back by management, which has to deal with a bunch of privileged stuff 
anyway (assigned LVM volumes, assigned pci and usb devices, setting up 
the bridge, large pages, guest priorities).  Why are we adding code to 
benefit so few people, many of whom don't really use qemu as users?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
>> The model you're advocating (privileged process handing over a fd) is 
>> not as secure because it requires that the management daemon runs as 
>> a privileged user.
>
> Or it can acquire an fd from a privileged helper and pass it on or 
> conjure it some other way.

So qemu should not be allowed to interact with a privileged helper on 
it's own?  I can't tell what your objection here is.  Do you want people 
to just not use qemu directly from the command line?

>>   There's nothing about this that prevents the use of a management 
>> framework.  In fact, had this existed when libvirt was first written, 
>> I'd hope libvirt would have used this mechanism instead of fd 
>> inheritance.
>>
>> Management software is really just another user.  We really want 
>> management software to run unprivileged as much as possible.
>
> I'd like to see qemu confined to managing a single guest and not 
> expand to system management.  We have enough to do without taking over 
> management systems and security.
>
> Bridged network configuration is painful now, but only for a handful 
> of users (us developers).  For the vast majority it is handled behind 
> their back by management, which has to deal with a bunch of privileged 
> stuff anyway (assigned LVM volumes, assigned pci and usb devices, 
> setting up the bridge, large pages, guest priorities).  Why are we 
> adding code to benefit so few people, many of whom don't really use 
> qemu as users?

I strongly disagree with the way you separate users who use management 
software from people who invoke qemu directly.  libvirt and virt-manager 
are existence proofs that management software heavily relies on the 
defaults and mechanisms we establish within qemu.  We can say all we 
want about how management software should do things but the best way is 
to make it easy for them to do the right thing.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 05:50 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>>> The model you're advocating (privileged process handing over a fd) 
>>> is not as secure because it requires that the management daemon runs 
>>> as a privileged user.
>>
>> Or it can acquire an fd from a privileged helper and pass it on or 
>> conjure it some other way.
>
> So qemu should not be allowed to interact with a privileged helper on 
> it's own?  I can't tell what your objection here is.  Do you want 
> people to just not use qemu directly from the command line?

No, of course not, I use qemu from the command line and would benefit 
from -net bridge.  My badly-conveyed objection is that qemu should not 
take a system management role (and enforce system-wide policies) but 
leave that to system management tools.

Basically, qemu should keep itself to running a single guest and leave 
permissions, multiple guests, system configuration to the management stack.

Of course, -net bridge doesn't prevent the management stack from doing 
management itself (and using -net tap,fd=), but as can be seen from Dan 
Berrange's response, -net bridge might encourage them into laziness; 
then we're on the slippery slope of adding more features to -net bridge 
because libvirt depends on it.  Do you really want to add selinux 
labelling (whatever that is) to qemu?

>>>
>>> Management software is really just another user.  We really want 
>>> management software to run unprivileged as much as possible.
>>
>> I'd like to see qemu confined to managing a single guest and not 
>> expand to system management.  We have enough to do without taking 
>> over management systems and security.
>>
>> Bridged network configuration is painful now, but only for a handful 
>> of users (us developers).  For the vast majority it is handled behind 
>> their back by management, which has to deal with a bunch of 
>> privileged stuff anyway (assigned LVM volumes, assigned pci and usb 
>> devices, setting up the bridge, large pages, guest priorities).  Why 
>> are we adding code to benefit so few people, many of whom don't 
>> really use qemu as users?
>
> I strongly disagree with the way you separate users who use management 
> software from people who invoke qemu directly.  libvirt and 
> virt-manager are existence proofs that management software heavily 
> relies on the defaults and mechanisms we establish within qemu. 

So you say, if someone makes a wrong decision, we should fix it by 
making the decision ourselves?

-net bridge will only dig them deeper into qemu defaults.

> We can say all we want about how management software should do things 
> but the best way is to make it easy for them to do the right thing.

Except it's not the right thing, at least not completely.  Creating the 
tap and attaching it to a bridge is just a part of configuring 
networking.  You're making it easy to do that part and impossible to do 
the rest.

Perhaps the same patchset, but to libvirt-devel, would be more useful 
since they can then add any extra features without burdening qemu.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> No, of course not, I use qemu from the command line and would benefit 
> from -net bridge.  My badly-conveyed objection is that qemu should not 
> take a system management role (and enforce system-wide policies) but 
> leave that to system management tools.

I do not consider this system management functional no more than I see 
providing a global configuration file as system management functional.  
They are both mechanisms.  The ACL file is a mechanism just like VNC 
sasl ACLs are a mechanism.

A policy decides how group a bunch of mechanisms together into a higher 
level thing that's hopefully easier to understand/manage.

> Of course, -net bridge doesn't prevent the management stack from doing 
> management itself (and using -net tap,fd=), but as can be seen from 
> Dan Berrange's response, -net bridge might encourage them into 
> laziness; then we're on the slippery slope of adding more features to 
> -net bridge because libvirt depends on it.  Do you really want to add 
> selinux labelling (whatever that is) to qemu?

If you're real concern is that we're implementing a policy of 'ifconfig 
$tap 0.0.0.0 up && brctl addif $bridge $tap', then I certainly can 
understand where you're coming from.

However, I think you're wrong to think of that as a policy.  I've seen 
many exotic network configurations over the years and I've never seen 
anyone do anything other than that with a tap device.  It really doesn't 
make sense to do anything more than that.

The only other configuration I've seen with a tap device is to directly 
configure an ip address with it and not use a bridge at all.  That's 
covered by -net tap though and really is not all that useful except for 
benchmarking.

>> I strongly disagree with the way you separate users who use 
>> management software from people who invoke qemu directly.  libvirt 
>> and virt-manager are existence proofs that management software 
>> heavily relies on the defaults and mechanisms we establish within qemu. 
>
> So you say, if someone makes a wrong decision, we should fix it by 
> making the decision ourselves?
>
> -net bridge will only dig them deeper into qemu defaults.

I'm suggesting we should get off our ivory tower claiming that 
management tools should do a better job than they are today and 
proactively make it easier for them to do the right thing.  We've always 
touted the improvement of security that qemu/kvm bridges because it 
allows a guest to run as an unprivileged user.  But this is chart-ware 
because it's simply not the case today.

>> We can say all we want about how management software should do things 
>> but the best way is to make it easy for them to do the right thing.
>
> Except it's not the right thing, at least not completely.  Creating 
> the tap and attaching it to a bridge is just a part of configuring 
> networking.  You're making it easy to do that part and impossible to 
> do the rest.

What is impossible to do with -net bridge?  Certainly, you can still 
capture the network interface very easily.  You can also still program 
ebtables rules as it's trivial to discover the name of the network device.

>
> Perhaps the same patchset, but to libvirt-devel, would be more useful 
> since they can then add any extra features without burdening qemu.

Except why limit this functionality to libvirt when it's useful to all 
management tools?

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 06:19 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>> No, of course not, I use qemu from the command line and would benefit 
>> from -net bridge.  My badly-conveyed objection is that qemu should 
>> not take a system management role (and enforce system-wide policies) 
>> but leave that to system management tools.
>
> I do not consider this system management functional no more than I see 
> providing a global configuration file as system management 
> functional.  They are both mechanisms.  The ACL file is a mechanism 
> just like VNC sasl ACLs are a mechanism.

I meant system as in outside the scope of a single VM.  VNC 
authentication is for a single VM.  Determining who is allowed to bridge 
where is system-wide functionality.

>
> However, I think you're wrong to think of that as a policy.  I've seen 
> many exotic network configurations over the years and I've never seen 
> anyone do anything other than that with a tap device.  It really 
> doesn't make sense to do anything more than that.

guest-specific ebtables rules
traffic control / QoS
statistics on the tap interface
vlan encapsulation
selinux labelling (if that makes sense)

>
>>> I strongly disagree with the way you separate users who use 
>>> management software from people who invoke qemu directly.  libvirt 
>>> and virt-manager are existence proofs that management software 
>>> heavily relies on the defaults and mechanisms we establish within qemu. 
>>
>> So you say, if someone makes a wrong decision, we should fix it by 
>> making the decision ourselves?
>>
>> -net bridge will only dig them deeper into qemu defaults.
>
> I'm suggesting we should get off our ivory tower claiming that 
> management tools should do a better job than they are today and 
> proactively make it easier for them to do the right thing.  We've 
> always touted the improvement of security that qemu/kvm bridges 
> because it allows a guest to run as an unprivileged user.  But this is 
> chart-ware because it's simply not the case today.

Fine, but fix it where it's broken, not in qemu.  Configuring a tap is 
not rocket science, it's just 200 lines.

>>> We can say all we want about how management software should do 
>>> things but the best way is to make it easy for them to do the right 
>>> thing.
>>
>> Except it's not the right thing, at least not completely.  Creating 
>> the tap and attaching it to a bridge is just a part of configuring 
>> networking.  You're making it easy to do that part and impossible to 
>> do the rest.
>
> What is impossible to do with -net bridge?  Certainly, you can still 
> capture the network interface very easily.  You can also still program 
> ebtables rules as it's trivial to discover the name of the network 
> device.

How, through the qemu monitor?

>>
>> Perhaps the same patchset, but to libvirt-devel, would be more useful 
>> since they can then add any extra features without burdening qemu.
>
> Except why limit this functionality to libvirt when it's useful to all 
> management tools?
>

Because each will need to do something slightly different.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Anthony Liguori wrote:
> Avi Kivity wrote:
> >No, of course not, I use qemu from the command line and would benefit 
> >from -net bridge.  My badly-conveyed objection is that qemu should not 
> >take a system management role (and enforce system-wide policies) but 
> >leave that to system management tools.
> 
> I do not consider this system management functional no more than I see 
> providing a global configuration file as system management functional.  
> They are both mechanisms.  The ACL file is a mechanism just like VNC 
> sasl ACLs are a mechanism.

That's why I would like there to be options to either pass to the
helper program, or specify a different helper program.  (Sorry if
that's already in the patches - for some reason I received 0/4 but
didn't receive the 4 patch emails).

There's no need for QEMU to be cleverer than that, and that puts the
whole policy in the hands of the user - where it should be.

It'd still install the default helper you've provided and use it by
default, of course.

> The only other configuration I've seen with a tap device is to directly 
> configure an ip address with it and not use a bridge at all.  That's 
> covered by -net tap though and really is not all that useful except for 
> benchmarking.

Contrarily, it's incredibly useful!  Most of my server VMs uses the
tap device without a bridge.  They are on private subnets within the
host, and use iptables NAT to access the outside world, with NAT port
forwarding to offer specific services.  That isolates them securely
far more effectively than bridging, and the iptables is simpler too.

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Jamie Lokier wrote:
> Anthony Liguori wrote:
>   
>> Avi Kivity wrote:
>>     
>>> No, of course not, I use qemu from the command line and would benefit 
>>>       
>> >from -net bridge.  My badly-conveyed objection is that qemu should not 
>>     
>>> take a system management role (and enforce system-wide policies) but 
>>> leave that to system management tools.
>>>       
>> I do not consider this system management functional no more than I see 
>> providing a global configuration file as system management functional.  
>> They are both mechanisms.  The ACL file is a mechanism just like VNC 
>> sasl ACLs are a mechanism.
>>     
>
> That's why I would like there to be options to either pass to the
> helper program, or specify a different helper program.  (Sorry if
> that's already in the patches - for some reason I received 0/4 but
> didn't receive the 4 patch emails).
>
> There's no need for QEMU to be cleverer than that, and that puts the
> whole policy in the hands of the user - where it should be.
>
> It'd still install the default helper you've provided and use it by
> default, of course.
>   

That's already how it behaves.  You can say -net 
bridge,helper=/usr/local/bin/my-helper

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Arnd Bergmann]

On Thursday 05 November 2009, Anthony Liguori wrote:
> > It'd still install the default helper you've provided and use it by
> > default, of course.
> >   
> That's already how it behaves.  You can say -net 
> bridge,helper=/usr/local/bin/my-helper
> 

How about abstracting it further and not making the helper depend on
bridge code. If we put the helper into netcf, we could make that
a more generic '-net netcf,helper=/usr/bin/netcf-helper' target,
with netcf doing the correct thing for the system configuration,
whether that is tap+bridge, tap+route, macvtap or something else
coming up. The helper would essentially become a black box for
providing a tap-like file descriptor with external connectivity.

	Arnd <><

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Arnd Bergmann wrote:
> On Thursday 05 November 2009, Anthony Liguori wrote:
>   
>>> It'd still install the default helper you've provided and use it by
>>> default, of course.
>>>   
>>>       
>> That's already how it behaves.  You can say -net 
>> bridge,helper=/usr/local/bin/my-helper
>>
>>     
>
> How about abstracting it further and not making the helper depend on
> bridge code. If we put the helper into netcf, we could make that
> a more generic '-net netcf,helper=/usr/bin/netcf-helper' target,
>   

Your suggestion on irc of doing -net tap,helper="/usr/bin/netcf-helper 
--name=foo" would also do the trick.  For qemu-bridge-helper, it would 
be -net tap,helper="/usr/libexec/qemu-bridge-helper --bridge=qemubr0".

I'd still want to have -net bridge syntax but it's just a syntatic 
wrapper (like -hda).  It's not something that would be used by libvirt, 
for instance.

Whether qemu-bridge-helper should live in qemu is another point to think 
about.   I think I would like to start with it being in qemu but then 
hope someone else pulls it into a separate project along with tunctl.

I think one can really make the argument that qemu should never allocate 
tap devices directly.  It should either get a file descriptor or call 
out to a helper to allocate a tap device and configure however is 
needed.  Both cases allow an unprivileged qemu whereas qemu creating a 
device directly clearly doesn't.

Maybe we could even eventually get rid of all of the tap code and 
replace it with a generic helper.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Arnd Bergmann]

On Thursday 05 November 2009, Anthony Liguori wrote:
> > How about abstracting it further and not making the helper depend on
> > bridge code. If we put the helper into netcf, we could make that
> > a more generic '-net netcf,helper=/usr/bin/netcf-helper' target,
> ...
> 
> Whether qemu-bridge-helper should live in qemu is another point to think 
> about.   I think I would like to start with it being in qemu but then 
> hope someone else pulls it into a separate project along with tunctl.

So how about putting your qemu-bridge-helper into qemu for the -net bridge
wrapper, but makeing -net netcf default to a different wrapper that we
put into netcf?

The netcf version can then be extended with advanced features like routing
and macvlan modes, as well as more sophisticated integration into the
distro's access control infrastructure.

	Arnd <><

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Arnd Bergmann wrote:
> On Thursday 05 November 2009, Anthony Liguori wrote:
>   
>>> How about abstracting it further and not making the helper depend on
>>> bridge code. If we put the helper into netcf, we could make that
>>> a more generic '-net netcf,helper=/usr/bin/netcf-helper' target,
>>>       
>> ...
>>
>> Whether qemu-bridge-helper should live in qemu is another point to think 
>> about.   I think I would like to start with it being in qemu but then 
>> hope someone else pulls it into a separate project along with tunctl.
>>     
>
> So how about putting your qemu-bridge-helper into qemu for the -net bridge
> wrapper, but makeing -net netcf default to a different wrapper that we
> put into netcf?
>
> The netcf version can then be extended with advanced features like routing
> and macvlan modes, as well as more sophisticated integration into the
> distro's access control infrastructure.
>   

Yup.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 07:42 PM, Anthony Liguori wrote:
>
> Whether qemu-bridge-helper should live in qemu is another point to 
> think about.   I think I would like to start with it being in qemu but 
> then hope someone else pulls it into a separate project along with 
> tunctl.

Let's please contribute it to the tunctl package.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 07:20 PM, Arnd Bergmann wrote:
> On Thursday 05 November 2009, Anthony Liguori wrote:
>    
>>> It'd still install the default helper you've provided and use it by
>>> default, of course.
>>>
>>>        
>> That's already how it behaves.  You can say -net
>> bridge,helper=/usr/local/bin/my-helper
>>
>>      
> How about abstracting it further and not making the helper depend on
> bridge code. If we put the helper into netcf, we could make that
> a more generic '-net netcf,helper=/usr/bin/netcf-helper' target,
> with netcf doing the correct thing for the system configuration,
> whether that is tap+bridge, tap+route, macvtap or something else
> coming up. The helper would essentially become a black box for
> providing a tap-like file descriptor with external connectivity.
>    

Helpers are really bad.  On launch, I find the fragile and hard to do 
proper error handling with (but that's probably just me).  But the real 
problem is at runtime, if you have a 16GB guest then you have to 
write-protect 4M ptes and then kvm has to tear down or write protect 
(not sure which mmu notifier is called) 4M shadow ptes.  Once that's 
done, the guest will have to fault its way back; that's at least 4M 
exits, around 10 seconds worth of cpu time to execute a couple of syscalls.

I'd much prefer a small daemon serving taps on a unix-domain socket.  Of 
course, management should talk to that daemon, not qemu.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> Helpers are really bad.  On launch, I find the fragile and hard to do 
> proper error handling with (but that's probably just me).  But the 
> real problem is at runtime, if you have a 16GB guest then you have to 
> write-protect 4M ptes and then kvm has to tear down or write protect 
> (not sure which mmu notifier is called) 4M shadow ptes.  Once that's 
> done, the guest will have to fault its way back; that's at least 4M 
> exits, around 10 seconds worth of cpu time to execute a couple of 
> syscalls.

If this is such an issue, then it's something that ought to be fixed in 
the kernel.  It's only really applicable to hotplug anyway as you 
wouldn't have faulted in the memory when initially launching the guest.

I know this has been discussed before, but isn't this why there are 
things like vfork()?  Instead of doing silly things into qemu, if there 
is concern about this, then it should be fixed in Linux properly.

> I'd much prefer a small daemon serving taps on a unix-domain socket.  
> Of course, management should talk to that daemon, not qemu.

I'd rather not have a program running with elevated privileges when it 
not needed.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Anthony Liguori wrote:
> Avi Kivity wrote:
> >Helpers are really bad.  On launch, I find the fragile and hard to do 
> >proper error handling with (but that's probably just me).  But the 
> >real problem is at runtime, if you have a 16GB guest then you have to 
> >write-protect 4M ptes and then kvm has to tear down or write protect 
> >(not sure which mmu notifier is called) 4M shadow ptes.  Once that's 
> >done, the guest will have to fault its way back; that's at least 4M 
> >exits, around 10 seconds worth of cpu time to execute a couple of 
> >syscalls.
> 
> If this is such an issue, then it's something that ought to be fixed in 
> the kernel.  It's only really applicable to hotplug anyway as you 
> wouldn't have faulted in the memory when initially launching the guest.
> 
> I know this has been discussed before, but isn't this why there are 
> things like vfork()?  Instead of doing silly things into qemu, if there 
> is concern about this, then it should be fixed in Linux properly.

Well, I'm glad to hear KVM is designed to fork() correctly :-) I've
been thinking of forking some running VMs for a while; it could be a
great way to launch VMs in preloaded test states quickly.

Yes, vfork() should avoid all the page protection and mmu overhead.
If not, it could probably be changed to.

But these days, Glibc's system() is even smarter: it uses clone() to
spawn the child, then the child execs.  That's better than vfork()
because a slow exec (when the disk/NFS is slow) doesn't block the
parent.  But it doesn't duplicate the MM.

I haven't a clue if Glibc's clone method would trigger lots of KVM
exits.

> >I'd much prefer a small daemon serving taps on a unix-domain socket.  
> >Of course, management should talk to that daemon, not qemu.
> 
> I'd rather not have a program running with elevated privileges when it 
> not needed.

That sounds unnecessarily complicated.  How often do you expect to
request new taps while KVM is running?

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 09:58 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>> Helpers are really bad.  On launch, I find the fragile and hard to do 
>> proper error handling with (but that's probably just me).  But the 
>> real problem is at runtime, if you have a 16GB guest then you have to 
>> write-protect 4M ptes and then kvm has to tear down or write protect 
>> (not sure which mmu notifier is called) 4M shadow ptes.  Once that's 
>> done, the guest will have to fault its way back; that's at least 4M 
>> exits, around 10 seconds worth of cpu time to execute a couple of 
>> syscalls.
>
> If this is such an issue, then it's something that ought to be fixed 
> in the kernel. 

Now here you can say "tremendously difficult" without danger of 
exaggeration.  I looked at write-protecting the top 512 pgd entries 
instead of the 4M pte entries; that's difficult enough.  And then you 
fault write access back in, you have to figure out that no sharing is 
possibly going on underneath so you can grant write access at the 
pgd/pud/pmd level instead of the pte level.  There's currently nothing 
in Linux that can help with this as sharing is tracked at the page level.

For kvm you have to extend this to mmu notifiers; without npt/ept 
there's simply no hope (no correspondence between shadow and host page 
tables); with them things are a little easier, though still pretty bad, 
as memory won't be aligned the same way.

> It's only really applicable to hotplug anyway as you wouldn't have 
> faulted in the memory when initially launching the guest.

If we're doing something for management system we have to consider 
hotplug.  It would be pretty mean to offer libvirt an easy way to set up 
bridging only to have them track down bugs later where the guest freezes 
for tens of milliseconds and later slows down after a hotplug, then 
rewrite their code not to use the helper.

>
> I know this has been discussed before, but isn't this why there are 
> things like vfork()? 

vfork() doesn't work with threads - it requires that the calling process 
be halted until exec() is called.

> Instead of doing silly things into qemu, if there is concern about 
> this, then it should be fixed in Linux properly.

Of course there is concern about it, and you don't have to do anything 
silly to qemu to avoid it.  Just not call helpers while it's running.

>
>> I'd much prefer a small daemon serving taps on a unix-domain socket.  
>> Of course, management should talk to that daemon, not qemu.
>
> I'd rather not have a program running with elevated privileges when it 
> not needed.
>

suid helpers are dangerous whenever they are on disk; daemons are 
dangerous only when running.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> >I know this has been discussed before, but isn't this why there are 
> >things like vfork()? 
> 
> vfork() doesn't work with threads - it requires that the calling process 
> be halted until exec() is called.

On Linux (NPTL), vfork() is good for this.  It only halts the calling
thread.  Other threads continue to run, and when the vfork'd thread
called exec*(), it doesn't affect the other threads.  Of course this
behaviour isn't portable, but then again, neither is KVM.

(Forget what I said elsewhere about Glibc using clone() to spawn
processes.  It's just Glibc's very complicated way of implementing
fork(), and doesn't share the parent's memory).

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

Jamie Lokier wrote:
> Avi Kivity wrote:
>   
>>> I know this has been discussed before, but isn't this why there are 
>>> things like vfork()? 
>>>       
>> vfork() doesn't work with threads - it requires that the calling process 
>> be halted until exec() is called.
>>     
>
> On Linux (NPTL), vfork() is good for this.  It only halts the calling
> thread.  Other threads continue to run, and when the vfork'd thread
> called exec*(), it doesn't affect the other threads.  Of course this
> behaviour isn't portable, but then again, neither is KVM.
>   
Yup, see attached program.  Here's the output which confirms this is the 
case.

tick
sleeping
tick
tick
tick
tick
tick
tick
tick
tick
tick
done sleeping
vfork() returned
hello world

Regards,

Anthony Liguori

[-- Attachment #2: vfork.c --]
[-- Type: text/x-csrc, Size: 931 bytes --]

#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>
#include <pthread.h>

static void *tick_thread(void *unused)
{
    while (1) {
        pthread_testcancel();
        printf("tick\n");
        sleep(1);
    }
    return NULL;
}

int main(int argc, char **argv)
{
    pid_t pid;
    pthread_t tid;
    void *ret;

    if (pthread_create(&tid, NULL, tick_thread, NULL) < 0) {
        printf("pthread_create failed\n");
        return 1;
    }

    pid = vfork();
    if (pid == 0) {
        printf("sleeping\n");
        sleep(10);
        printf("done sleeping\n");
        execl("/bin/echo", "echo", "hello world", NULL);
        exit(1);
    } else if (pid > 0) {
        printf("vfork() returned\n");
        waitpid(pid, NULL, 0);
    } else {
        printf("vfork failed\n");
        return 1;
    }

    pthread_cancel(tid);
    pthread_join(tid, &ret);

    return 0;
}

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Anthony Liguori wrote:
> Jamie Lokier wrote:
> >Avi Kivity wrote:
> >  
> >>>I know this has been discussed before, but isn't this why there are 
> >>>things like vfork()? 
> >>>      
> >>vfork() doesn't work with threads - it requires that the calling process 
> >>be halted until exec() is called.
> >>    
> >
> >On Linux (NPTL), vfork() is good for this.  It only halts the calling
> >thread.  Other threads continue to run, and when the vfork'd thread
> >called exec*(), it doesn't affect the other threads.  Of course this
> >behaviour isn't portable, but then again, neither is KVM.
> >  
> Yup, see attached program.  Here's the output which confirms this is the 
> case.

Before assuming it, you might want to verify that it's the same on all
kernels which support KVM (including backports) and all versions of
Linux pthreads which might be used.

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
>> Instead of doing silly things into qemu, if there is concern about 
>> this, then it should be fixed in Linux properly.
>
> Of course there is concern about it, and you don't have to do anything 
> silly to qemu to avoid it.  Just not call helpers while it's running.

This is unacceptable.  We use helpers in multiple places today.  We use 
a helper to configure a tap device that we've allocated, we use it for 
the exec: protocol for live migration, etc.

Running qemu directly from the command line is absolutely an important 
use case.  A desktop user should not need things like libvirt and 
virt-manager.

If it cannot be fixed in the kernel, we'll have to work around it in 
userspace.  We can introduce our own spawn() function that works by 
fork()'ing very early and listening on a socketpair.  This will sit 
reading from the socket waiting for commands to exec.  Using a unix 
socket, we can pass fds that get inherited which we can't do with system().

>> I'd rather not have a program running with elevated privileges when 
>> it not needed.
>>
>
> suid helpers are dangerous whenever they are on disk; daemons are 
> dangerous only when running.

A suid helper equivalent to a root daemon from a security perspective.  
It's just long running vs. transient.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/06/2009 04:19 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>>> Instead of doing silly things into qemu, if there is concern about 
>>> this, then it should be fixed in Linux properly.
>>
>> Of course there is concern about it, and you don't have to do 
>> anything silly to qemu to avoid it.  Just not call helpers while it's 
>> running.
>
> This is unacceptable.  We use helpers in multiple places today.  We 
> use a helper to configure a tap device that we've allocated, we use it 
> for the exec: protocol for live migration, etc.

I hope none of these are ever used in production systems.  Taps should 
come pre-configured from management (no need for management to ask qemu 
to run a management helper when management could run that helper more 
easily itself).  exec: migration is also pointless when you can pass and 
fd and do any proxying required.

> Running qemu directly from the command line is absolutely an important 
> use case. 

Where does this requirement come from?

We probably have different ideas of what a desktop user means.  In my 
mind, a desktop user doesn't switch to the monitor and type 'eject', he 
clicks a button with an eject icon and a text caption underneath.  He 
doesn't run qemu-img create -f qcow2 /images/vm1.img, he clicks 'New 
Virtual Machine Wizard' and answers all the tedious questions.  And he 
absolutely doesn't type

   qemu-system-x86_64 -enable-kvm -m 1G -smp 2 -net 
nic,macaddr=aa:00:00:00:00:01 /images/vm1.img -cdrom 
/images/iso/en_us_....blah.iso,

instead he clicks My\ New\ Virtual\ Machine\ \(copy\).qemu on his desktop.

> A desktop user should not need things like libvirt and virt-manager.

virt-mananger is miles ahead of where you're aiming.

I'd like to a proper same-process graphical UI client.  But I don't 
think this list is the place to create it.  I don't think we have either 
the skills or the patience; also there's room for more than one.  We 
should focus on making it easy to write one; that involves exporting the 
display surface in an embeddable non-vnc way and making everything 
controllable via QObjects (perhaps through the monitor, perhaps through 
bindings for scripting languages.

> If it cannot be fixed in the kernel, we'll have to work around it in 
> userspace.  We can introduce our own spawn() function that works by 
> fork()'ing very early and listening on a socketpair.  This will sit 
> reading from the socket waiting for commands to exec.  Using a unix 
> socket, we can pass fds that get inherited which we can't do with 
> system().

Or we can admit to ourselves that qemu is too complex to be directly 
controlled by a user.  It's good to have an easy to use command line for 
developers and power users; I'd welcome -net bridge as one of them.  But 
we shouldn't try to invent access control systems or install suid 
helpers.  Mainstream use needs to involve some management agent which 
does authentication and privileged configuration (it was already 
established that the the hotplug equivalent of -net bridge is racy if 
any configuration is required).

>>> I'd rather not have a program running with elevated privileges when 
>>> it not needed.
>>>
>>
>> suid helpers are dangerous whenever they are on disk; daemons are 
>> dangerous only when running.
>
> A suid helper equivalent to a root daemon from a security 
> perspective.  It's just long running vs. transient.
>

They are not equivalent.  suid helpers are dangerous whenever they are 
on disk; daemons are dangerous only when running.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/07/2009 11:14 AM, Avi Kivity wrote:
> I'd welcome -net bridge as one of them.  But we shouldn't try to 
> invent access control systems or install suid helpers.

We can make the helper a script that does

   exec sudo /the/real/helper "$@"

so a user can add it to /etc/sudoers and get pre-authenticated 
configuration.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> On 11/07/2009 11:14 AM, Avi Kivity wrote:
>> I'd welcome -net bridge as one of them.  But we shouldn't try to 
>> invent access control systems or install suid helpers.
>
> We can make the helper a script that does
>
>   exec sudo /the/real/helper "$@"
>
> so a user can add it to /etc/sudoers and get pre-authenticated 
> configuration.

The key point of the helper here is that you pass an fd to a socketpair 
and you then receive an fd over that socket.  What the helper does is 
really less important.  Whether it's a script like you suggest or 
something like I proposed doesn't matter from a qemu perspective.

Whether the qemu-bridge-helper should live in qemu or somewhere else is 
a valid thing to discuss.  In my next posting, I'll have things 
restructured to separate out the two so that they two series can be 
considered independently.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Arnd Bergmann]

On Saturday 07 November 2009, Anthony Liguori wrote:
> Avi Kivity wrote:
> > On 11/07/2009 11:14 AM, Avi Kivity wrote:
> >> I'd welcome -net bridge as one of them.  But we shouldn't try to 
> >> invent access control systems or install suid helpers.
> >
> > We can make the helper a script that does
> >
> >   exec sudo /the/real/helper "$@"
> >
> > so a user can add it to /etc/sudoers and get pre-authenticated 
> > configuration.
> 
> The key point of the helper here is that you pass an fd to a socketpair 
> and you then receive an fd over that socket.  What the helper does is 
> really less important.  Whether it's a script like you suggest or 
> something like I proposed doesn't matter from a qemu perspective.

Well, the difference matters from a security perspective. The sudo
script that Avi suggested just means that you can guarantee you don't
introduce any security holes through a suid executable. Fortunately,
it does not impact the contents of your helper either, only the
installation. You could even be clever in qemu and use call the helper
using sudo if qemu is running as unpriviledged user and the helper is
not a suid file.

	Arnd <><

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Arnd Bergmann wrote:
> Well, the difference matters from a security perspective. The sudo
> script that Avi suggested just means that you can guarantee you don't
> introduce any security holes through a suid executable. Fortunately,
> it does not impact the contents of your helper either, only the
> installation. You could even be clever in qemu and use call the helper
> using sudo if qemu is running as unpriviledged user and the helper is
> not a suid file.
>   

Or just use fscaps and not even work about suid :-)  That's the 
preferred model.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/08/2009 12:12 AM, Anthony Liguori wrote:
> Arnd Bergmann wrote:
>> Well, the difference matters from a security perspective. The sudo
>> script that Avi suggested just means that you can guarantee you don't
>> introduce any security holes through a suid executable. Fortunately,
>> it does not impact the contents of your helper either, only the
>> installation. You could even be clever in qemu and use call the helper
>> using sudo if qemu is running as unpriviledged user and the helper is
>> not a suid file.
>
> Or just use fscaps and not even work about suid :-)  That's the 
> preferred model.

fscaps does not eliminate the security concern, just reduces it.  
CAP_NET_ADMIN is way to powerful to let loose.

If the sudo script execs your binary then we can install everything 
without special privileges.  All it takes then to enable bridging for 
non-privileged users is a line in /etc/sudoers allowing the script to be 
run without a password prompt (and of course, for someone to set up 
bridging and dhcp and to allocate MAC addresses).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
>> Running qemu directly from the command line is absolutely an 
>> important use case. 
>
> Where does this requirement come from?

For most of qemu's lifetime, this was the only option.  The current 
graphical front ends only support a subset of qemu's features and qemu's 
target architecture types.  qemu is more than just KVM accelerated x86 
guests.

I also disagree that only developers are interested in using qemu 
directly.  There are a lot of power users who also use qemu directly.

>> A desktop user should not need things like libvirt and virt-manager.
>
> virt-mananger is miles ahead of where you're aiming.
>
> I'd like to a proper same-process graphical UI client.  But I don't 
> think this list is the place to create it.  I don't think we have 
> either the skills or the patience; also there's room for more than 
> one.  We should focus on making it easy to write one; that involves 
> exporting the display surface in an embeddable non-vnc way and making 
> everything controllable via QObjects (perhaps through the monitor, 
> perhaps through bindings for scripting languages.
>
>> If it cannot be fixed in the kernel, we'll have to work around it in 
>> userspace.  We can introduce our own spawn() function that works by 
>> fork()'ing very early and listening on a socketpair.  This will sit 
>> reading from the socket waiting for commands to exec.  Using a unix 
>> socket, we can pass fds that get inherited which we can't do with 
>> system().
>
> Or we can admit to ourselves that qemu is too complex to be directly 
> controlled by a user.  It's good to have an easy to use command line 
> for developers and power users; I'd welcome -net bridge as one of 
> them.  But we shouldn't try to invent access control systems or 
> install suid helpers.  Mainstream use needs to involve some management 
> agent which does authentication and privileged configuration (it was 
> already established that the the hotplug equivalent of -net bridge is 
> racy if any configuration is required).

I disagree about the role a management app should play.  For a casual 
user, a management app really should not be needed.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> On 11/05/2009 07:20 PM, Arnd Bergmann wrote:
>> On Thursday 05 November 2009, Anthony Liguori wrote:
>>   
>>>> It'd still install the default helper you've provided and use it by
>>>> default, of course.
>>>>
>>>>        
>>> That's already how it behaves.  You can say -net
>>> bridge,helper=/usr/local/bin/my-helper
>>>
>>>      
>> How about abstracting it further and not making the helper depend on
>> bridge code. If we put the helper into netcf, we could make that
>> a more generic '-net netcf,helper=/usr/bin/netcf-helper' target,
>> with netcf doing the correct thing for the system configuration,
>> whether that is tap+bridge, tap+route, macvtap or something else
>> coming up. The helper would essentially become a black box for
>> providing a tap-like file descriptor with external connectivity.
>>    
>
> Helpers are really bad.  On launch, I find the fragile and hard to do 
> proper error handling with (but that's probably just me).  But the 
> real problem is at runtime, if you have a 16GB guest then you have to 
> write-protect 4M ptes and then kvm has to tear down or write protect 
> (not sure which mmu notifier is called) 4M shadow ptes.  Once that's 
> done, the guest will have to fault its way back; that's at least 4M 
> exits, around 10 seconds worth of cpu time to execute a couple of 
> syscalls.

FWIW, with large pages, it's only 8k exits which comes out to about 
16ms.  If nothing else, it's an argument for the important of 
transparent large pages.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/06/2009 02:29 AM, Anthony Liguori wrote:
>> Helpers are really bad.  On launch, I find the fragile and hard to do 
>> proper error handling with (but that's probably just me).  But the 
>> real problem is at runtime, if you have a 16GB guest then you have to 
>> write-protect 4M ptes and then kvm has to tear down or write protect 
>> (not sure which mmu notifier is called) 4M shadow ptes.  Once that's 
>> done, the guest will have to fault its way back; that's at least 4M 
>> exits, around 10 seconds worth of cpu time to execute a couple of 
>> syscalls.
>
>
> FWIW, with large pages, it's only 8k exits which comes out to about 
> 16ms.  If nothing else, it's an argument for the important of 
> transparent large pages.
>

No, it's an argument against fork() of large programs.  Transparent 
large pages are very important for performance, but speeding up fork() 
of large programs is not one of their reasons for existence.

In any case we don't have them yet.  Will you recommend people avoid 
hotplug until we have transparent large pages?  And check that the qemu 
is actually using them (they'll be best effort, not a guarantee) before 
hotplug?

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> On 11/06/2009 02:29 AM, Anthony Liguori wrote:
>>> Helpers are really bad.  On launch, I find the fragile and hard to 
>>> do proper error handling with (but that's probably just me).  But 
>>> the real problem is at runtime, if you have a 16GB guest then you 
>>> have to write-protect 4M ptes and then kvm has to tear down or write 
>>> protect (not sure which mmu notifier is called) 4M shadow ptes.  
>>> Once that's done, the guest will have to fault its way back; that's 
>>> at least 4M exits, around 10 seconds worth of cpu time to execute a 
>>> couple of syscalls.
>>
>>
>> FWIW, with large pages, it's only 8k exits which comes out to about 
>> 16ms.  If nothing else, it's an argument for the important of 
>> transparent large pages.
>>
>
> No, it's an argument against fork() of large programs.

After putting together a work around, I'm starting to have my doubts 
about how real of a problem this is.

You're only write protecting memory, correct?  So it's equivalent to 
enabling dirty tracking during live migration.  In my mind, if the cost 
associated with hot plug is a fraction of the cost of live migration, 
we're in good shape.

It's not likely that a 16GB guest is going to write-fault in it's 
entirely memory range immediately.  In fact, it's likely to be amortized 
over a very long period of time so I have a hard time believing this is 
really an issue in practice.

Arguably, it's a much bigger problem for live migration.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/06/2009 06:09 PM, Anthony Liguori wrote:
>> No, it's an argument against fork() of large programs.
>
>
> After putting together a work around, I'm starting to have my doubts 
> about how real of a problem this is.
>
> You're only write protecting memory, correct? 


The kernel write protects qemu memory, and kvm write protects shadow 
page table entries.  Live migration only does the second.

> So it's equivalent to enabling dirty tracking during live migration.  
> In my mind, if the cost associated with hot plug is a fraction of the 
> cost of live migration, we're in good shape.

I don't see why.  Live migration is pretty expensive, but that doesn't 
mean NIC hotplug should be.  Deployments where live migration isn't a 
concern (for example, performance critical guests that use device 
assignment) don't suffer the live migration penalty so they shouldn't 
expect to see a gratuitous NIC hotplug penalty that is a fraction of that.

Come to think of that, we probably have some fork() breakage with device 
assignment since we can't write protect pages assigned to the iommu.

> It's not likely that a 16GB guest is going to write-fault in it's 
> entirely memory range immediately.  In fact, it's likely to be 
> amortized over a very long period of time so I have a hard time 
> believing this is really an issue in practice.

It depends on the workload.  With large pages in both host and guest you 
can touch 10M pages/sec without difficulty.  Once you write protect them 
this drops to maybe 0.3M pages/sec.  The right workload will suffer 
pretty badly from this.

> Arguably, it's a much bigger problem for live migration.

It is.  I once considered switching live migration to shadow pte dirty 
bit tracking instead of write protection, but ept doesn't have dirty 
bits, so this will only help a minority of deployments by the time it 
reaches users.

So vfork() is required, or in light of its man page and glowing 
recommendations from the security people, we can mark guest memory as 
shared on fork and use plain fork(), like we do for pre mmu notifier 
kernels.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> So vfork() is required, or in light of its man page and glowing 
> recommendations from the security people,

vfork() on Linux is fine to use, as long as you respect it's unwritten
requirements :-)

In a multithreaded program: you can start by spawning a thread for the
sole purpose of spawning a process.  Call vfork() from the new thread.
Then a slow exec() call won't delay the rest of the program, because
it's only blocking the thread you created.

> we can mark guest memory as 
> shared on fork and use plain fork(), like we do for pre mmu notifier 
> kernels.

Aiee - what's the plan?  Can a running KVM be forked, as in into two
separate processes to run the forked guests in parallel, or not?

Thanks,
-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/07/2009 12:44 PM, Jamie Lokier wrote:
> Aiee - what's the plan?  Can a running KVM be forked, as in into two
> separate processes to run the forked guests in parallel, or not?
>    

kvm fds do not support fork().  Nothing prevents you from stopping the 
guest, reading guest state, fork()ing, and creating a new VM in the 
child with the same state, then restarting both instances.

qemu in general cannot be forked; the two instances would trample on 
each others disk images, host network connections (i.e. vnc, X, the 
monitor), guest network connections, etc.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> On 11/07/2009 12:44 PM, Jamie Lokier wrote:
> >Aiee - what's the plan?  Can a running KVM be forked, as in into two
> >separate processes to run the forked guests in parallel, or not?
> 
> kvm fds do not support fork().  Nothing prevents you from stopping the 
> guest, reading guest state, fork()ing, and creating a new VM in the 
> child with the same state, then restarting both instances.
> 
> qemu in general cannot be forked; the two instances would trample on 
> each others disk images, host network connections (i.e. vnc, X, the 
> monitor), guest network connections, etc.

Oh, I understand that qemu forking is fraught.  It would need to fork
at just the right moment and recreate all sorts of things.

And I'm not surprised that the kvm fd does not fork.  Why should it?
fds refer to device instances, and fork does not generally create new
instances.

What I'd like to know is, if all of QEMU's state is appropriately
recreated in the child instance, and KVM's device is reopened with a
copy of the kvm state (by using the recently introduced ioctls to get
and set it), will it fork the _guest RAM_ mapped into KVM in the way
that fork does?

Or is it necessary to copy all the guest RAM from original instance
into the new ones, so that there must be multiple copies of all the
guest RAM pages, even if they are the same?

What I have in mind is a "test state server", which is a KVM instance
halted in a state where loadvm has loaded up all of guest memory in a
state where the guest is ready to do something.

Incoming requests cause that server to fork off and start all the
device emulations, open network connections, create -snapshot qcow2
temporary fork files, etc., so the forked child is a running VM, doing
whatever it was poised to do, with no permanent effect.

(Naturally the guest would have to be in a usefully prepared state,
probably it's just about to run a network config script to get it's
per-instance settings and then fetch a test script to run.)

Basically, will KVM (the kernel bits) work using guest RAM which is
shared with other guest instances and a non-guest instance (the parent
process which hasn't yet started KVM) in that way?

Will KVM (the kernel bits) also work the other way, where it is paused
and all the devices closed (as if we're doing savevm and shutting
down), but instead of saving and unmapping guest memory, only the
device state is savevm'd, that's used to put it into the halted state
described above, from which it's able to reload device state and
continue?

In a nutshell, does KVM mind the guest's memory pages (including
device memory like framebuffers) being COW-shared in all the ways that
fork() can cause?

I know it's an arcane question, so thanks a lot if you answer! :-)
-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/09/2009 09:35 PM, Jamie Lokier wrote:
>
> What I'd like to know is, if all of QEMU's state is appropriately
> recreated in the child instance, and KVM's device is reopened with a
> copy of the kvm state (by using the recently introduced ioctls to get
> and set it), will it fork the _guest RAM_ mapped into KVM in the way
> that fork does?
>    

Guest RAM is just ordinary userspace memory.  If you told Linux to COW 
it, it will COW it.  If you told Linux not to COW it, it will not COW 
it.  kvm has nothing to say in the matter (at least with kernels 2.6.27 
and up).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> On 11/09/2009 09:35 PM, Jamie Lokier wrote:
> >
> >What I'd like to know is, if all of QEMU's state is appropriately
> >recreated in the child instance, and KVM's device is reopened with a
> >copy of the kvm state (by using the recently introduced ioctls to get
> >and set it), will it fork the _guest RAM_ mapped into KVM in the way
> >that fork does?
> >   
> 
> Guest RAM is just ordinary userspace memory.  If you told Linux to COW 
> it, it will COW it.  If you told Linux not to COW it, it will not COW 
> it.  kvm has nothing to say in the matter (at least with kernels 2.6.27 
> and up).

That's great, thanks.  I was wondering if KVM did something magical
with memory to make itself work, and it seems the answer is "it might
have done before", which is fine for my nefarious purpose.

Thanks again for your helpful explanation.

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
>> So it's equivalent to enabling dirty tracking during live migration.  
>> In my mind, if the cost associated with hot plug is a fraction of the 
>> cost of live migration, we're in good shape.
>
> I don't see why.  Live migration is pretty expensive, but that doesn't 
> mean NIC hotplug should be.  Deployments where live migration isn't a 
> concern (for example, performance critical guests that use device 
> assignment) don't suffer the live migration penalty so they shouldn't 
> expect to see a gratuitous NIC hotplug penalty that is a fraction of 
> that.

Live migration is a feature that I expect will often be used in an 
automated fashion.  IOW, there will be some background process that 
automatically migrates a VM to improve overall utilization in a cloud.

NIC hotplug, on the other hand, will almost always be initiated by a 
user.  If there's a cost associated with it, at least it's driven by a 
user action so it's something a user can plan for.

>> Arguably, it's a much bigger problem for live migration.
>
> It is.  I once considered switching live migration to shadow pte dirty 
> bit tracking instead of write protection, but ept doesn't have dirty 
> bits, so this will only help a minority of deployments by the time it 
> reaches users.
>
> So vfork() is required, or in light of its man page and glowing 
> recommendations from the security people, we can mark guest memory as 
> shared on fork and use plain fork(), like we do for pre mmu notifier 
> kernels.

Indeed.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> On 11/05/2009 04:50 PM, Anthony Liguori wrote:
> Bridged network configuration is painful now, but only for a handful of 
> users (us developers).

I have to disagree.  Although I'm a developer too, if I put my system
administrator hat on for a moment...

I found the bridged network configuration annoying.  I gave up trying
to be dynamic and just preallocated user-owned static tap devices in
the system interface list for every VM that my users want to run
(which means they have to ask me when they want more VMs on the
bridges).  I still had to configure the servers' bridges and
interfaces for all that manually, and bind all the tap devices to
bridges even when not in use.  The iptables wasn't pretty either.

I think this qemu mode to call out to an external program for the
interface is an simple, useful change which will make dynamic
interfaces easier.

You _can_ do the same thing with a management program or script.  At
first (like you) I was wondering why the helper is needed since you
can do it so easily with a wrapper around QEMU, and then I realised:

Because it's very hard to write a wrapper script which correctly
matches QEMU options and inserts an "fd=..." option in the right
place, for all different QEMU/KVM versions.

If you're using a management program with it's own configuration with
generates all the QEMU command options anyway, there's no problem.

But if you want useful networking, while specifying ordinary QEMU
options e.g. from a command line or simple script... then it's
difficult and fragile to make a script wrapper for that without
Anthony's change.

Btw, I'll probably replace the helper program with something which
does the permission checks and interface setup in a different way, but
it's still a good pattern to call one.

> For the vast majority it is handled behind their 
> back by management, which has to deal with a bunch of privileged stuff 
> anyway (assigned LVM volumes, assigned pci and usb devices, setting up 
> the bridge, large pages, guest priorities).  Why are we adding code to 
> benefit so few people, many of whom don't really use qemu as users?

Perhaps qemu should call out to helper programs to request access to
block devices, pci and usb devices, priorities and access to large
pages too?

It would at least be consistent if every "request for a system
resource which might need permission" could be replaced by
"user-configurable command for request for a system resouce".

Anyway, in my experience using QEMU and KVM, networking is the only
complicated bit of system configuration.  Everything else is just
usespace - I've never felt the need used LVM volumes, pass-through PCI
and USB devices, or guest priorities.  It's all VNC servers, emulated
USB mice, and disk images files, none of which needs system configuration.

Networking is unusual in being the only system service that virtually
every guest needs, so I guess my experience applies to most users who
are using QEMU/KVM directly.

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
>> At least with KVM support, this is probably the most common use case 
>> which means
>> that most of our users are running qemu as root.  That's terrible.
>>    
>
> Most of our users run managed systems.

I consider management software as a user.  Today, most management 
software launches qemu as root.  libvirt is just getting around to 
fixing this although they still are running it as a single user instead 
of as the user requesting the vm be launched.

The fundamental problem, is that to use qemu as a non-privileged user, 
you need to go from userA -> root -> userB.  For the lazy, it's easiest 
just to make userA == userB == root.  IMHO, the ideal thing is to always 
be userA.

If we make this easy for management software to do, they're more likely 
to do the right thing.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 04:57 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>>> At least with KVM support, this is probably the most common use case 
>>> which means
>>> that most of our users are running qemu as root.  That's terrible.
>>
>> Most of our users run managed systems.
>
> I consider management software as a user. 

It isn't.  A user is a person, and -net bridge helps people.

> Today, most management software launches qemu as root.  libvirt is 
> just getting around to fixing this although they still are running it 
> as a single user instead of as the user requesting the vm be launched.

That's a libvirt bug.  Maybe they should adopt your helper.

> The fundamental problem, is that to use qemu as a non-privileged user, 
> you need to go from userA -> root -> userB.  For the lazy, it's 
> easiest just to make userA == userB == root.  IMHO, the ideal thing is 
> to always be userA.

Agreed.

> If we make this easy for management software to do, they're more 
> likely to do the right thing.

But we're forcing our style of security management on them.  How to 
store permissions is the management system's job (and for a clu^Houd, it 
will typically be stored in a central database, not be scattered around 
/etc).

Again, IMO we should stick to making a guest work, and leave all the 
glue to management.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 05:11 PM, Avi Kivity wrote:
> But we're forcing our style of security management on them.  How to 
> store permissions is the management system's job (and for a clu^Houd, 
> it will typically be stored in a central database, not be scattered 
> around /etc).
>
> Again, IMO we should stick to making a guest work, and leave all the 
> glue to management.
>

As an example of why this is so, if the management stack wants to 
configure the tap interface further (say, add some ebtables rules 
guarding the new interface) it must push this into qemu or stop using 
-net bridge.

Having the tap accessible to management also allows it to run tcpdump or 
collect statistics on it at runtime.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> On 11/05/2009 05:11 PM, Avi Kivity wrote:
>> But we're forcing our style of security management on them.  How to 
>> store permissions is the management system's job (and for a clu^Houd, 
>> it will typically be stored in a central database, not be scattered 
>> around /etc).
>>
>> Again, IMO we should stick to making a guest work, and leave all the 
>> glue to management.
>>
>
> As an example of why this is so, if the management stack wants to 
> configure the tap interface further (say, add some ebtables rules 
> guarding the new interface) it must push this into qemu or stop using 
> -net bridge.

If you wanted to set rules based on the tap device itself, then yes.  
But I think the more common case (honestly, the only case I've seen so 
far) is where the rules are set on the bridge itself.

> Having the tap accessible to management also allows it to run tcpdump 
> or collect statistics on it at runtime.

I'm not advocating removing -net tap,fd=.  But -net bridge is obviously 
useful and makes writing management tools that do common things easier.  
Not doing something that helps management tools and command line users 
tremendously simply because it's possible to do it another way for 
management tools (but not for command line users) is almost user hostile.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 05:58 PM, Anthony Liguori wrote:
>> Having the tap accessible to management also allows it to run tcpdump 
>> or collect statistics on it at runtime.
>
> I'm not advocating removing -net tap,fd=.  But -net bridge is 
> obviously useful and makes writing management tools that do common 
> things easier.  Not doing something that helps management tools and 
> command line users tremendously simply because it's possible to do it 
> another way for management tools (but not for command line users) is 
> almost user hostile.
>

As your patchset shows, it's not "tremendously" difficult.  The basic 
helper (which is all libvirt would need; plus PolicyKit integration) is 
200 lines.  I don't see why you consider placing functionality in the 
management stack vs qemu user hostile, considering who our users are.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> I don't see why you consider placing functionality in the 
> management stack vs qemu user hostile, considering who our users are.

Do you mean that virtually all users use a management stack?

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
>> If we make this easy for management software to do, they're more 
>> likely to do the right thing.
>
> But we're forcing our style of security management on them.  How to 
> store permissions is the management system's job (and for a clu^Houd, 
> it will typically be stored in a central database, not be scattered 
> around /etc).
>
> Again, IMO we should stick to making a guest work, and leave all the 
> glue to management.

That's short sighted.  If we just focus on "making a guest work" we'll 
end up with crappy interfaces that cripple management tools.  If users 
are constantly struggling to do even the simplest things with qemu, then 
it doesn't matter how well our "guest works".  No one will use it.

I think we absolutely have to think about the full stack and how all the 
pieces interact.  There are definitely problems in the stack right now.  
Security is the one I'm trying to address in this series.  If you cannot 
launch a reasonable configured qemu from the command line as an 
unprivileged user, there's really no hope that we can expect a 
management tool to do that.

Again, there are no shortage of existence proofs of this (beyond 
libvirt).  I suspect there isn't a management tool out there that does 
the right thing today.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 06:06 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>>> If we make this easy for management software to do, they're more 
>>> likely to do the right thing.
>>
>> But we're forcing our style of security management on them.  How to 
>> store permissions is the management system's job (and for a clu^Houd, 
>> it will typically be stored in a central database, not be scattered 
>> around /etc).
>>
>> Again, IMO we should stick to making a guest work, and leave all the 
>> glue to management.
>
> That's short sighted.  If we just focus on "making a guest work" we'll 
> end up with crappy interfaces that cripple management tools. 

Only with management tools that cripple themselves.  It's pretty easy to 
get unprivileged bridging with -net tap; it's just that libvirt hadn't 
gotten around to it yet -- see Dan's comment.  Are you going to take on 
every libvirt deficiency and push it into qemu?

> If users are constantly struggling to do even the simplest things with 
> qemu, then it doesn't matter how well our "guest works".  No one will 
> use it.

That's not the case today, even with virt-manager.

> I think we absolutely have to think about the full stack and how all 
> the pieces interact.  There are definitely problems in the stack right 
> now.  Security is the one I'm trying to address in this series.  If 
> you cannot launch a reasonable configured qemu from the command line 
> as an unprivileged user, there's really no hope that we can expect a 
> management tool to do that.

I'm almost offended on Dan's behalf.

> Again, there are no shortage of existence proofs of this (beyond 
> libvirt).  I suspect there isn't a management tool out there that does 
> the right thing today.

RHEV-H launches guests as unprivileged users; the management daemon is 
also unprivileged.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
>> That's short sighted.  If we just focus on "making a guest work" 
>> we'll end up with crappy interfaces that cripple management tools. 
>
> Only with management tools that cripple themselves.  It's pretty easy 
> to get unprivileged bridging with -net tap; it's just that libvirt 
> hadn't gotten around to it yet -- see Dan's comment.  Are you going to 
> take on every libvirt deficiency and push it into qemu?

No, but I'd like avoid forcing management tools to introduce these 
deficiencies in the first place.

>> If users are constantly struggling to do even the simplest things 
>> with qemu, then it doesn't matter how well our "guest works".  No one 
>> will use it.
>
> That's not the case today, even with virt-manager.

I'm not going to pick on any particular tool, but kvm absolutely has a 
reputation of being difficult to use compared to other virtualization 
software out there.  Nothing else requires modifying configuration files 
just to get a guest with bridged networking.

>> I think we absolutely have to think about the full stack and how all 
>> the pieces interact.  There are definitely problems in the stack 
>> right now.  Security is the one I'm trying to address in this 
>> series.  If you cannot launch a reasonable configured qemu from the 
>> command line as an unprivileged user, there's really no hope that we 
>> can expect a management tool to do that.
>
> I'm almost offended on Dan's behalf.

We'll I guess it's about perspectives then.  I don't see the fact that 
libvirt runs qemu as root as a libvirt deficiency.  I see it as a qemu 
deficiency.

>> Again, there are no shortage of existence proofs of this (beyond 
>> libvirt).  I suspect there isn't a management tool out there that 
>> does the right thing today.
>
> RHEV-H launches guests as unprivileged users; the management daemon is 
> also unprivileged.

I suspected as much based on how strongly you were advocating this.  I 
think my point still stands though, there's an awful lot of management 
software out there that gets it wrong.  It's great that you guys got it 
right but so far, the majority of users are not using qemu through 
RHEV-M so I still think we have a problem.

Regards,

Anthony Liguori

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 06:25 PM, Anthony Liguori wrote:
>> That's not the case today, even with virt-manager.
>
>
> I'm not going to pick on any particular tool, but kvm absolutely has a 
> reputation of being difficult to use compared to other virtualization 
> software out there.  Nothing else requires modifying configuration 
> files just to get a guest with bridged networking.

The two management tools I'm familiar with (virt-manager and RHEV-M) 
don't need configuration files.  It's true that virt-manager does NAT, 
not full bridging, but -net bridge doesn't fix that.

>>> I think we absolutely have to think about the full stack and how all 
>>> the pieces interact.  There are definitely problems in the stack 
>>> right now.  Security is the one I'm trying to address in this 
>>> series.  If you cannot launch a reasonable configured qemu from the 
>>> command line as an unprivileged user, there's really no hope that we 
>>> can expect a management tool to do that.
>>
>> I'm almost offended on Dan's behalf.
>
> We'll I guess it's about perspectives then.  I don't see the fact that 
> libvirt runs qemu as root as a libvirt deficiency.  I see it as a qemu 
> deficiency.

I don't understand why.  It's pretty easy to run qemu as non-root.

>>> I suspect there isn't a management tool out there that does the 
>>> right thing today.
>>
> I suspected as much based on how strongly you were advocating this. 

You're a little too suspicious.  RHEV-H has nothing to do with my 
opposition to -net bridge.

> I think my point still stands though, there's an awful lot of 
> management software out there that gets it wrong.  It's great that you 
> guys got it right but so far, the majority of users are not using qemu 
> through RHEV-M so I still think we have a problem.

I'm worrying that we're transforming one problem into two different 
ones.  Expanding the scope of qemu, and making it more difficult to use 
advanced networking functionality.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
>>>> I suspect there isn't a management tool out there that does the 
>>>> right thing today.
>>>
>> I suspected as much based on how strongly you were advocating this. 
>
> You're a little too suspicious.  RHEV-H has nothing to do with my 
> opposition to -net bridge.
>
>> I think my point still stands though, there's an awful lot of 
>> management software out there that gets it wrong.  It's great that 
>> you guys got it right but so far, the majority of users are not using 
>> qemu through RHEV-M so I still think we have a problem.
>
> I'm worrying that we're transforming one problem into two different 
> ones.  Expanding the scope of qemu, and making it more difficult to 
> use advanced networking functionality.

Do you object to the idea of having qemu call to a helper program 
directly or to the idea of having the helper program in qemu at all?  
Splitting the helper into a separate project may be a more constructive 
discussion as was suggested by Arnd.

As a separate project, libvirt could also make use of it and use -net 
tap,fd= if it wanted to do crazy things.  From a distro perspective, 
it's just a matter of setting up dependencies to make it Just Work so I 
don't object to it that much.  It's just a bit annoying to create an 
entire new project for a few hundred line helper.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Scott Tsai]

On Fri, Nov 6, 2009 at 12:50 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> It's just a bit annoying to create an entire new
> project for a few hundred line helper.

This new project would also be a better place for 'tunctl' and other
projects such as user mode linux might want to use your new helper
program.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 07:16 PM, Scott Tsai wrote:
> On Fri, Nov 6, 2009 at 12:50 AM, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>    
>> It's just a bit annoying to create an entire new
>> project for a few hundred line helper.
>>      
> This new project would also be a better place for 'tunctl' and other
> projects such as user mode linux might want to use your new helper
> program.
>    

Seems like a library implemented as executables instead of a shared object.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> On 11/05/2009 07:16 PM, Scott Tsai wrote:
> >On Fri, Nov 6, 2009 at 12:50 AM, Anthony Liguori<aliguori@us.ibm.com>  
> >wrote:
> >   
> >>It's just a bit annoying to create an entire new
> >>project for a few hundred line helper.
> >>     
> >This new project would also be a better place for 'tunctl' and other
> >projects such as user mode linux might want to use your new helper
> >program.
> >   
> 
> Seems like a library implemented as executables instead of a shared object.

Yes.  It's an executable because it needs setuid to do privileged things...

It could be provided as a shared object (which calls an executable if
it needs setuiding), but then for some things you'd have to provide an
option like -net helper=/path/to/my/helper.so, and you might not want
the implied fixed ABI, and it'd be harder to use scripts.

After all we don't have -net
script=/path/to/my/upscript.so,downscript=/path/to/my/downscript.so do
we? ;-)

Spawning an executable is really fast in Linux.  For something which
is only called a few times at most when the program starts or is
rarely reconfigured, I don't see any reason not to call an executable.

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 06:50 PM, Anthony Liguori wrote:
>> I'm worrying that we're transforming one problem into two different 
>> ones.  Expanding the scope of qemu, and making it more difficult to 
>> use advanced networking functionality.
>
>
> Do you object to the idea of having qemu call to a helper program 
> directly or to the idea of having the helper program in qemu at all? 

Both, the former much less that the latter.

I dislike helper programs, I find them inefficient especially at runtime 
as the penalties of fork() on a memory hog (esp. one that uses mmu 
notifiers like qemu+kvm) are considerable.  They also make things like 
the oom-killer a necessity.  But I really want qemu out of the security 
and network management business and focusing on qemulation.  Security 
and network management are the job of the management stack, if they 
don't do the job well, send patches.

> Splitting the helper into a separate project may be a more 
> constructive discussion as was suggested by Arnd.

I agree.

> As a separate project, libvirt could also make use of it and use -net 
> tap,fd= if it wanted to do crazy things.  From a distro perspective, 
> it's just a matter of setting up dependencies to make it Just Work so 
> I don't object to it that much.  It's just a bit annoying to create an 
> entire new project for a few hundred line helper.

I'd much rather see libvirt have a small daemon doing this, but that's 
my standard dislike of helpers.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> On 11/05/2009 06:50 PM, Anthony Liguori wrote:
> >>I'm worrying that we're transforming one problem into two different 
> >>ones.  Expanding the scope of qemu, and making it more difficult to 
> >>use advanced networking functionality.
> >
> >
> >Do you object to the idea of having qemu call to a helper program 
> >directly or to the idea of having the helper program in qemu at all? 
> 
> Both, the former much less that the latter.
> 
> I dislike helper programs, I find them inefficient especially at runtime 
> as the penalties of fork() on a memory hog (esp. one that uses mmu 
> notifiers like qemu+kvm) are considerable.

Note that system() does not used fork() these days.  It should not be
affected by process size.

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Daniel P. Berrange]

On Thu, Nov 05, 2009 at 08:57:18AM -0600, Anthony Liguori wrote:
> Avi Kivity wrote:
> >>At least with KVM support, this is probably the most common use case 
> >>which means
> >>that most of our users are running qemu as root.  That's terrible.
> >>   
> >
> >Most of our users run managed systems.
> 
> I consider management software as a user.  Today, most management 
> software launches qemu as root.  libvirt is just getting around to 
> fixing this although they still are running it as a single user instead 
> of as the user requesting the vm be launched.

We have two modes of operating in libvirt.

 - The 'system' instance. The libvirtd daemon runs privileged in order
   to allow full management of all host services including network
   and storage. The QEMU vms run under a 'qemu' user.  This is intended
   for server virtualization use cases.

 - The 'session' instance. The libvirtd daemon runs unprivileged as the
   user account accessing it. The QEMU vms run under the users own
   account too. This is intended for desktop virtualization use cases.

The main problem is that we've never really used the 'session' instances,
since networking configs are rather limited to pretty much just SLIRP 
and people expect full bridging.  I think this patch series you've
done is invaluable and will let us finally make full use of the libvirt
'session' instances for desktop virt, running everything unprivileged.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 05:11 PM, Daniel P. Berrange wrote:
> The main problem is that we've never really used the 'session' instances,
> since networking configs are rather limited to pretty much just SLIRP
> and people expect full bridging.  I think this patch series you've
> done is invaluable and will let us finally make full use of the libvirt
> 'session' instances for desktop virt, running everything unprivileged.
>
>    

What's to stop you from using the same idea to get a tap fd for the 
unprivileged libvirtd instance?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Daniel P. Berrange]

On Thu, Nov 05, 2009 at 05:14:03PM +0200, Avi Kivity wrote:
> On 11/05/2009 05:11 PM, Daniel P. Berrange wrote:
> >The main problem is that we've never really used the 'session' instances,
> >since networking configs are rather limited to pretty much just SLIRP
> >and people expect full bridging.  I think this patch series you've
> >done is invaluable and will let us finally make full use of the libvirt
> >'session' instances for desktop virt, running everything unprivileged.
> >
> >   
> 
> What's to stop you from using the same idea to get a tap fd for the 
> unprivileged libvirtd instance?

Nothing in particular, but we've not done so thus far

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> On 11/05/2009 05:11 PM, Daniel P. Berrange wrote:
>> The main problem is that we've never really used the 'session' 
>> instances,
>> since networking configs are rather limited to pretty much just SLIRP
>> and people expect full bridging.  I think this patch series you've
>> done is invaluable and will let us finally make full use of the libvirt
>> 'session' instances for desktop virt, running everything unprivileged.
>>
>>    
>
> What's to stop you from using the same idea to get a tap fd for the 
> unprivileged libvirtd instance?

Why limit this to just libvirt based management tools?  The helper has 
to live somewhere, why not have it live in qemu?

libvirt can still call it and pass the fd to qemu if it really wants to.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 05:59 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>> On 11/05/2009 05:11 PM, Daniel P. Berrange wrote:
>>> The main problem is that we've never really used the 'session' 
>>> instances,
>>> since networking configs are rather limited to pretty much just SLIRP
>>> and people expect full bridging.  I think this patch series you've
>>> done is invaluable and will let us finally make full use of the libvirt
>>> 'session' instances for desktop virt, running everything unprivileged.
>>>
>>
>> What's to stop you from using the same idea to get a tap fd for the 
>> unprivileged libvirtd instance?
>
> Why limit this to just libvirt based management tools?  The helper has 
> to live somewhere, why not have it live in qemu?
>

Because anything special the management tools wants done (as simple as 
remembering the interface name so it can collect statistics and 
associate them with the guest) will render the helper unusable.  The 
helper is pure glue so it will be very hard to generalize.

> libvirt can still call it and pass the fd to qemu if it really wants to.

Until it needs a new feature, then it goes back to -net tap.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Avi Kivity wrote:
> On 11/05/2009 05:59 PM, Anthony Liguori wrote:
>> Avi Kivity wrote:
>>> On 11/05/2009 05:11 PM, Daniel P. Berrange wrote:
>>>> The main problem is that we've never really used the 'session' 
>>>> instances,
>>>> since networking configs are rather limited to pretty much just SLIRP
>>>> and people expect full bridging.  I think this patch series you've
>>>> done is invaluable and will let us finally make full use of the 
>>>> libvirt
>>>> 'session' instances for desktop virt, running everything unprivileged.
>>>>
>>>
>>> What's to stop you from using the same idea to get a tap fd for the 
>>> unprivileged libvirtd instance?
>>
>> Why limit this to just libvirt based management tools?  The helper 
>> has to live somewhere, why not have it live in qemu?
>>
>
> Because anything special the management tools wants done (as simple as 
> remembering the interface name so it can collect statistics and 
> associate them with the guest) will render the helper unusable.  The 
> helper is pure glue so it will be very hard to generalize.

It can get the interface names via info network.

But let's make this more concrete, what features cannot be implemented?

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Avi Kivity]

On 11/05/2009 06:28 PM, Anthony Liguori wrote:
>> Because anything special the management tools wants done (as simple 
>> as remembering the interface name so it can collect statistics and 
>> associate them with the guest) will render the helper unusable.  The 
>> helper is pure glue so it will be very hard to generalize.
>
>
> It can get the interface names via info network.
>
> But let's make this more concrete, what features cannot be implemented?
>

Well, with info network and running the guest stopped, everything's 
doable.  But it's a rather roundabout way compared to creating the 
interface, configuring the !@#$%^ out of it, and passing it on to the guest.

btw, for network hotplug, you'll need to create/config/pass the 
interface, since you can't stop the guest.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Daniel P. Berrange]

On Thu, Nov 05, 2009 at 06:35:50PM +0200, Avi Kivity wrote:
> On 11/05/2009 06:28 PM, Anthony Liguori wrote:
> >>Because anything special the management tools wants done (as simple 
> >>as remembering the interface name so it can collect statistics and 
> >>associate them with the guest) will render the helper unusable.  The 
> >>helper is pure glue so it will be very hard to generalize.
> >
> >
> >It can get the interface names via info network.
> >
> >But let's make this more concrete, what features cannot be implemented?
> >
> 
> Well, with info network and running the guest stopped, everything's 
> doable.  But it's a rather roundabout way compared to creating the 
> interface, configuring the !@#$%^ out of it, and passing it on to the guest.
> 
> btw, for network hotplug, you'll need to create/config/pass the 
> interface, since you can't stop the guest.

Indeed the hotplug  scenario is a bit of a problem in this model,
since libvirt needs to be able to setup iptables & ebtables rules
between creating the device & giving it to the guest.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Anthony Liguori]

Daniel P. Berrange wrote:
> Indeed the hotplug  scenario is a bit of a problem in this model,
> since libvirt needs to be able to setup iptables & ebtables rules
> between creating the device & giving it to the guest.
>   

But does libvirt every setup tap specific iptable or ebtable rules?

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Daniel P. Berrange]

On Thu, Nov 05, 2009 at 11:03:48AM -0600, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> >Indeed the hotplug  scenario is a bit of a problem in this model,
> >since libvirt needs to be able to setup iptables & ebtables rules
> >between creating the device & giving it to the guest.
> >  
> 
> But does libvirt every setup tap specific iptable or ebtable rules?

We have recently got a mode where we setup a rule against a specific TAP
device to filter non-assigned MAC, to prevent guests spoofing MAC addrs,
and will do similar for IP packets in the future.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Daniel P. Berrange wrote:
> On Thu, Nov 05, 2009 at 11:03:48AM -0600, Anthony Liguori wrote:
> > Daniel P. Berrange wrote:
> > >Indeed the hotplug  scenario is a bit of a problem in this model,
> > >since libvirt needs to be able to setup iptables & ebtables rules
> > >between creating the device & giving it to the guest.
> > >  
> > 
> > But does libvirt every setup tap specific iptable or ebtable rules?
> 
> We have recently got a mode where we setup a rule against a specific TAP
> device to filter non-assigned MAC, to prevent guests spoofing MAC addrs,
> and will do similar for IP packets in the future.

It's a good idea, but it can be difficult to update iptables rules on
a general system which has lots of other iptables rules as well.  How
do you handle that?

Btw, my approach to filtering & spoof avoidance, for some VMs which
don't need to be bridged, has been to avoid bridging, put the VMs on
their own private subnet inside the host, and used iptables NAT to
route them.

That blocks things like mDNS, Windows Network Neighbourhood discovery
and so on, but for some VMs that doesn't matter or is even preferable,
to provide better isolation.

-- Jamie

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Arnd Bergmann]

On Thursday 05 November 2009, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> > Indeed the hotplug  scenario is a bit of a problem in this model,
> > since libvirt needs to be able to setup iptables & ebtables rules
> > between creating the device & giving it to the guest.
> >   
> 
> But does libvirt every setup tap specific iptable or ebtable rules?

I believe that is part of what Gerhard is currently adding to libvirt.

	Arnd <><

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Gerhard Stenzel]

"Arnd Bergmann" <arnd@arndb.de> wrote on 11/05/2009 06:26:39 PM:

> "Arnd Bergmann" <arnd@arndb.de>
> 11/05/2009 06:26 PM
>
> To
>
> "Anthony Liguori" <aliguori@us.ibm.com>, Gerhard
Stenzel/Germany/IBM@IBMDE
>
> cc
>
> "Daniel P. Berrange" <berrange@redhat.com>, "Avi Kivity"
> <avi@redhat.com>, "Mark McLoughlin" <markmc@redhat.com>, "Arnd
> Bergmann" <arndbergmann@googlemail.com>, "Dustin Kirkland"
> <kirkland@canonical.com>, "Juan Quintela" <quintela@redhat.com>, ""
> <qemu-devel@nongnu.org>, "Michael Tsirkin" <mst@redhat.com>
>
> Subject
>
> Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu
>
> On Thursday 05 November 2009, Anthony Liguori wrote:
> > Daniel P. Berrange wrote:
> > > Indeed the hotplug  scenario is a bit of a problem in this model,
> > > since libvirt needs to be able to setup iptables & ebtables rules
> > > between creating the device & giving it to the guest.
> > >
> >
> > But does libvirt every setup tap specific iptable or ebtable rules?
>
> I believe that is part of what Gerhard is currently adding to libvirt.
>
>    Arnd <><

The current version
(https://www.redhat.com/archives/libvir-list/2009-October/msg00741.html)
only sets some some basic ebtables rules. This can certainly be augmented.
What other iptables and ebtables are we talking about here?

Best regards,

Gerhard Stenzel, Linux on Cell/Hybrid Technologies, LTC
-----------------------------------------------------------------------------------------------------------------------------------

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter | Geschäftsführung: Erich
Baier
Sitz der Gesellschaft: Böblingen | Registergericht: Amtsgericht Stuttgart,
HRB 243294

Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Jamie Lokier]

Avi Kivity wrote:
> On 11/05/2009 05:59 PM, Anthony Liguori wrote:
> >Avi Kivity wrote:
> >>On 11/05/2009 05:11 PM, Daniel P. Berrange wrote:
> >>>The main problem is that we've never really used the 'session' 
> >>>instances,
> >>>since networking configs are rather limited to pretty much just SLIRP
> >>>and people expect full bridging.  I think this patch series you've
> >>>done is invaluable and will let us finally make full use of the libvirt
> >>>'session' instances for desktop virt, running everything unprivileged.
> >>>
> >>
> >>What's to stop you from using the same idea to get a tap fd for the 
> >>unprivileged libvirtd instance?
> >
> >Why limit this to just libvirt based management tools?  The helper has 
> >to live somewhere, why not have it live in qemu?
> >
> 
> Because anything special the management tools wants done (as simple as 
> remembering the interface name so it can collect statistics and 
> associate them with the guest) will render the helper unusable.  The 
> helper is pure glue so it will be very hard to generalize.

The management tool can provide it's own helper program to QEMU, which
can communicate with the management tool... via a side channel, so the
management tool as a whole can do anything it wants when QEMU requests
the tap interface.  Getting the interface name and inserting
iptables/ebtables rules would be quite simple that way.

-- Jamie

[Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Mark McLoughlin]

On Tue, 2009-11-03 at 18:28 -0600, Anthony Liguori wrote:
> We address this problem by introducing a new network backend: -net bridge.  This
> backend is less flexible than -net tap because it relies on a helper with
> elevated privileges to do the heavy lifting of allocating and attaching a tap
> device to a bridge.  We use a special purpose helper because we don't want
> to elevate the privileges of more generic tools like brctl.

Just had a quick look through so far, but I like it.

I think it would make sense to move Fedora and libvirt to using this,
even for the system libvirtd.

Agree with danpb that we should hook in PolicyKit for the authorization
checking. It'd be nice to setup the PolicyKit auth on a per-bridge
basis, but we could try and figure that out later. A global auth would
be enough to begin with, falling back to the ACL files.

Also, I think the vnet_hdr and sndbuf arguments are valid for -net
bridge too

Cheers,
Mark.

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Daniel P. Berrange]

On Thu, Nov 05, 2009 at 10:00:05AM -0500, Mark McLoughlin wrote:
> On Tue, 2009-11-03 at 18:28 -0600, Anthony Liguori wrote:
> > We address this problem by introducing a new network backend: -net bridge.  This
> > backend is less flexible than -net tap because it relies on a helper with
> > elevated privileges to do the heavy lifting of allocating and attaching a tap
> > device to a bridge.  We use a special purpose helper because we don't want
> > to elevate the privileges of more generic tools like brctl.
> 
> Just had a quick look through so far, but I like it.
> 
> I think it would make sense to move Fedora and libvirt to using this,
> even for the system libvirtd.

Yes, if this functionality is present in QEMU, there's no reason we
shouldn't use it all the time. The other nice thing about this is 
that people will be able to take the command line generated by libvirt
and run it directly for troubleshooting, since it will no longer rely
on magic TAP file handles to be passed

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Dustin Kirkland]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

On Thu, 2009-11-05 at 15:14 +0000, Daniel P. Berrange wrote:
> On Thu, Nov 05, 2009 at 10:00:05AM -0500, Mark McLoughlin wrote:
> > On Tue, 2009-11-03 at 18:28 -0600, Anthony Liguori wrote:
> > > We address this problem by introducing a new network backend: -net bridge.  This
> > > backend is less flexible than -net tap because it relies on a helper with
> > > elevated privileges to do the heavy lifting of allocating and attaching a tap
> > > device to a bridge.  We use a special purpose helper because we don't want
> > > to elevate the privileges of more generic tools like brctl.
> > 
> > Just had a quick look through so far, but I like it.
> > 
> > I think it would make sense to move Fedora and libvirt to using this,
> > even for the system libvirtd.
> 
> Yes, if this functionality is present in QEMU, there's no reason we
> shouldn't use it all the time. The other nice thing about this is 
> that people will be able to take the command line generated by libvirt
> and run it directly for troubleshooting, since it will no longer rely
> on magic TAP file handles to be passed

That would be incredibly useful when trying to triage bugs.  Ubuntu
users often file virtualization bugs against virt-manager or virsh, in
that that's the interface they're using when they see a problem.  And
it's up to me to figure out where in the virt-manager -> libvirt ->
qemu-kvm -> qemu stack the bug actually exists.  The ability to
precisely run just the libvirt command line (without hand modification
like I have to do now) would be magical.

:-Dustin

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu

[Arnd Bergmann]

On Wednesday 04 November 2009, Anthony Liguori wrote:
> And assuming a bridge is defined named qemubr0 and the administrator has setup
> permissions accordingly, it will Just Work.  My hope is that distributions will
> do this work as part of the qemu packaging process such that for most users,
> the out-of-the-box experience will also Just Work.
> 
> More details are included in individual patches.  I broke up the helper into
> a series of patches to improve reviewabilty.

Looks all very nice. I'm not completely sure if qemu is the right place to
put the helper though. I can see that it helps you to have it there, but
basically anyone using tun/tap could use the same functionality, so packaging
it together with tunctl would be nice, either in the same binary or as
two binaries in the same package.

Then again, tunctl is (on some distros as least) part of the uml-utilities
package for similar reasons, which feels equally wrong now.

	Arnd <><