* [Qemu-devel] [PATCH v2 0/4] -net bridge: rootless bridge support for qemu
@ 2011-10-21 15:07 Corey Bryant
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper Corey Bryant
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Corey Bryant @ 2011-10-21 15:07 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, rmarwah
With qemu it is possible to run a guest from an unprivileged user but if
we wanted to communicate with the outside world we had to switch
to root.
We address this problem by introducing a new network backend and a new
network option for -net tap. This is less flexible when compared to
existing -net tap options 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.
Qemu can be run with the default network helper as follows (in these cases
attaching the tap device to the default br0 bridge):
qemu -hda linux.img -net bridge -net nic
or:
qemu -hda linux.img -net tap,helper=/usr/local/libexec/qemu-bridge-helper -net nic
The default helper uses it's own ACL mechanism for access control, but
future network helpers could be developed, for example, to support PolicyKit
for access control.
More details are included in individual patches. The helper is broken into
a series of patches to improve reviewabilty.
v2:
- Updated signed-off-by's
- Updated author's email
- Set default bridge to br0
- Added -net bridge
- Updated ACL example
- Moved from libcap to libcap-ng
- Fail helper when libcap-ng not configured
Corey Bryant (4):
Add basic version of bridge helper
Add access control support to qemu bridge helper
Add cap reduction support to enable use as SUID
Add support for net bridge
Makefile | 12 ++-
configure | 37 +++++
net.c | 29 ++++-
net.h | 3 +
net/tap.c | 190 ++++++++++++++++++++++++-
net/tap.h | 2 +
qemu-bridge-helper.c | 380 ++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-options.hx | 73 ++++++++--
8 files changed, 703 insertions(+), 23 deletions(-)
create mode 100644 qemu-bridge-helper.c
--
1.7.3.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper
2011-10-21 15:07 [Qemu-devel] [PATCH v2 0/4] -net bridge: rootless bridge support for qemu Corey Bryant
@ 2011-10-21 15:07 ` Corey Bryant
2011-10-23 12:56 ` Blue Swirl
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu " Corey Bryant
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Corey Bryant @ 2011-10-21 15:07 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, rmarwah
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>
Signed-off-by: Richa Marwaha <rmarwah@linux.vnet.ibm.com>
Signed-off-by: Corey Bryant <coreyb@linux.vnet.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 f63fc02..d9b447e 100644
--- a/Makefile
+++ b/Makefile
@@ -35,6 +35,8 @@ $(call set-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 QMP/qmp-commands.txt
else
@@ -75,7 +77,7 @@ defconfig:
-include config-all-devices.mak
-build-all: $(DOCS) $(TOOLS) recurse-all
+build-all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all
config-host.h: config-host.h-timestamp
config-host.h-timestamp: config-host.mak
@@ -153,6 +155,8 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y)
qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y)
qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-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)/scripts/hxtool -h < $< > $@," GEN $@")
@@ -221,7 +225,7 @@ 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 qemu-options.def
- rm -f *.o *.d *.a *.lo $(TOOLS) qemu-ga TAGS cscope.* *.pod *~ */*~
+ rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
rm -Rf .libs
rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o qga/*.d
rm -f qemu-img-cmds.h
@@ -289,6 +293,10 @@ install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
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 4f87e0a..6c8b659 100755
--- a/configure
+++ b/configure
@@ -2768,6 +2768,7 @@ echo "datadir=$datadir" >> $config_host_mak
echo "sysconfdir=$sysconfdir" >> $config_host_mak
echo "docdir=$docdir" >> $config_host_mak
echo "confdir=$confdir" >> $config_host_mak
+echo "libexecdir=\${prefix}/libexec" >> $config_host_mak
case "$cpu" in
i386|x86_64|alpha|cris|hppa|ia64|lm32|m68k|microblaze|mips|mips64|ppc|ppc64|s390|s390x|sparc|sparc64|unicore32)
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
new file mode 100644
index 0000000..2ce82fb
--- /dev/null
+++ b/qemu-bridge-helper.c
@@ -0,0 +1,205 @@
+/*
+ * QEMU Bridge Helper
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * 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 = 0;
+ 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.7.3.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu bridge helper
2011-10-21 15:07 [Qemu-devel] [PATCH v2 0/4] -net bridge: rootless bridge support for qemu Corey Bryant
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper Corey Bryant
@ 2011-10-21 15:07 ` Corey Bryant
2011-10-23 13:10 ` Blue Swirl
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID Corey Bryant
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 4/4] Add support for net bridge Corey Bryant
3 siblings, 1 reply; 20+ messages in thread
From: Corey Bryant @ 2011-10-21 15:07 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, rmarwah
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 an ACL mechanism that is enforced by qemu-bridge-helper.
The ACLs are fairly simple whitelist/blacklist mechanisms with a wildcard of
'all'. All users are blacklisted by default, and deny takes precedence over
allow.
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.
As an example:
/etc/qemu/bridge.conf root:qemu 0640
allow br0
include /etc/qemu/alice.conf
include /etc/qemu/bob.conf
include /etc/qemu/charlie.conf
/etc/qemu/alice.conf root:alice 0640
allow br1
/etc/qemu/bob.conf root:bob 0640
allow br2
/etc/qemu/charlie.conf root:charlie 0640
deny all
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.
Users in the charlie group cannot get a tap device connected to any bridge.
Under no circumstance can the bob group get access to br1 or can the alice
group get access to br2. And under no cicumstance can the charlie group
get access to any bridge.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Richa Marwaha <rmarwah@linux.vnet.ibm.com>
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
qemu-bridge-helper.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 141 insertions(+), 0 deletions(-)
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 2ce82fb..db257d5 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -33,6 +33,105 @@
#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 = 0;
@@ -95,6 +194,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, access_denied;
/* parse arguments */
if (argc < 3 || argc > 4) {
@@ -115,6 +217,45 @@ 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
+ * according acl policy if we have a deny and allow both
+ * then deny should always win over allow
+ */
+ access_allowed = 0;
+ access_denied = 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_denied = 1;
+ break;
+ case ACL_DENY:
+ if (strcmp(bridge, acls[i].iface) == 0) {
+ access_denied = 1;
+ }
+ break;
+ }
+ }
+
+ if ((access_allowed == 0) || (access_denied == 1)) {
+ 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.7.3.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-21 15:07 [Qemu-devel] [PATCH v2 0/4] -net bridge: rootless bridge support for qemu Corey Bryant
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper Corey Bryant
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu " Corey Bryant
@ 2011-10-21 15:07 ` Corey Bryant
2011-10-23 13:22 ` Blue Swirl
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 4/4] Add support for net bridge Corey Bryant
3 siblings, 1 reply; 20+ messages in thread
From: Corey Bryant @ 2011-10-21 15:07 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, rmarwah
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>
Signed-off-by: Richa Marwaha <rmarwah@linux.vnet.ibm.com>
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
configure | 34 ++++++++++++++++++++++++++++++++++
qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+), 0 deletions(-)
diff --git a/configure b/configure
index 6c8b659..fed66b0 100755
--- a/configure
+++ b/configure
@@ -128,6 +128,7 @@ vnc_thread="no"
xen=""
xen_ctrl_version=""
linux_aio=""
+cap=""
attr=""
xfs=""
@@ -653,6 +654,10 @@ for opt do
;;
--enable-kvm) kvm="yes"
;;
+ --disable-cap) cap="no"
+ ;;
+ --enable-cap) cap="yes"
+ ;;
--disable-spice) spice="no"
;;
--enable-spice) spice="yes"
@@ -1032,6 +1037,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-cap disable libcap-ng support"
+echo " --enable-cap enable libcap-ng support"
echo " --disable-attr disables attr and xattr support"
echo " --enable-attr enable attr and xattr support"
echo " --disable-blobs disable installing provided firmware blobs"
@@ -1638,6 +1645,29 @@ EOF
fi
##########################################
+# libcap-ng library probe
+if test "$cap" != "no" ; then
+ cap_libs="-lcap-ng"
+ cat > $TMPC << EOF
+#include <cap-ng.h>
+int main(void)
+{
+ capng_capability_to_name(CAPNG_EFFECTIVE);
+ 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()
@@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
echo "madvise $madvise"
echo "posix_madvise $posix_madvise"
echo "uuid support $uuid"
+echo "libcap-ng support $cap"
echo "vhost-net support $vhost_net"
echo "Trace backend $trace_backend"
echo "Trace output file $trace_file-<pid>"
@@ -2846,6 +2877,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 db257d5..b1562eb 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -33,6 +33,10 @@
#include "net/tap-linux.h"
+#ifdef CONFIG_LIBCAP
+#include <cap-ng.h>
+#endif
+
#define MAX_ACLS (128)
#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
@@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
return sendmsg(c, &msg, 0);
}
+#ifdef CONFIG_LIBCAP
+static int drop_privileges(void)
+{
+ /* clear all capabilities */
+ capng_clear(CAPNG_SELECT_BOTH);
+
+ if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
+ CAP_NET_ADMIN) < 0) {
+ return -1;
+ }
+
+ /* change to calling user's real uid and gid, retaining supplemental
+ * groups and CAP_NET_ADMIN */
+ if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
+ return -1;
+ }
+
+ return 0;
+}
+#endif
+
int main(int argc, char **argv)
{
struct ifreq ifr;
@@ -198,6 +223,20 @@ int main(int argc, char **argv)
int acl_count = 0;
int i, access_allowed, access_denied;
+ /* if we're run from an suid binary, immediately drop privileges preserving
+ * cap_net_admin -- exit immediately if libcap not configured */
+ if (geteuid() == 0 && getuid() != geteuid()) {
+#ifdef CONFIG_LIBCAP
+ if (drop_privileges() == -1) {
+ fprintf(stderr, "failed to drop privileges\n");
+ return 1;
+ }
+#else
+ 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.7.3.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 4/4] Add support for net bridge
2011-10-21 15:07 [Qemu-devel] [PATCH v2 0/4] -net bridge: rootless bridge support for qemu Corey Bryant
` (2 preceding siblings ...)
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID Corey Bryant
@ 2011-10-21 15:07 ` Corey Bryant
3 siblings, 0 replies; 20+ messages in thread
From: Corey Bryant @ 2011-10-21 15:07 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, rmarwah
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 existing -net tap options
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 would be:
qemu linux.img -net bridge -net nic,model=virtio
or:
qemu linux.img -net tap,helper=/usr/local/libexec/qemu-bridge-helper
-net nic,model=virtio
The default bridge that we attach to is br0. 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 linux.img -net bridge,br=qemubr0 -net nic,model=virtio
or:
qemu linux.img -net tap,helper=/usr/local/libexec/qemu-bridge-helper,br=qemubr0
-net nic,model=virtio
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Richa Marwaha <rmarwah@linux.vnet.ibm.com>
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
---
configure | 2 +
net.c | 29 ++++++++-
net.h | 3 +
net/tap.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
net/tap.h | 3 +
qemu-options.hx | 73 +++++++++++++++++----
6 files changed, 279 insertions(+), 21 deletions(-)
diff --git a/configure b/configure
index fed66b0..9493d60 100755
--- a/configure
+++ b/configure
@@ -2800,6 +2800,8 @@ echo "sysconfdir=$sysconfdir" >> $config_host_mak
echo "docdir=$docdir" >> $config_host_mak
echo "confdir=$confdir" >> $config_host_mak
echo "libexecdir=\${prefix}/libexec" >> $config_host_mak
+echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> $config_host_mak
+echo "CONFIG_QEMU_HELPERDIR=\"$prefix/libexec\"" >> $config_host_mak
case "$cpu" in
i386|x86_64|alpha|cris|hppa|ia64|lm32|m68k|microblaze|mips|mips64|ppc|ppc64|s390|s390x|sparc|sparc64|unicore32)
diff --git a/net.c b/net.c
index d05930c..2dcb2d4 100644
--- a/net.c
+++ b/net.c
@@ -956,6 +956,14 @@ static const struct {
.type = QEMU_OPT_STRING,
.help = "script to shut down the interface",
}, {
+ .name = "br",
+ .type = QEMU_OPT_STRING,
+ .help = "bridge name",
+ }, {
+ .name = "helper",
+ .type = QEMU_OPT_STRING,
+ .help = "command to execute to configure bridge",
+ }, {
.name = "sndbuf",
.type = QEMU_OPT_SIZE,
.help = "send buffer limit"
@@ -1053,6 +1061,23 @@ static const struct {
{ /* end of list */ }
},
},
+ [NET_CLIENT_TYPE_BRIDGE] = {
+ .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 */ }
+ },
+ },
};
int net_client_init(Monitor *mon, QemuOpts *opts, int is_netdev)
@@ -1075,7 +1100,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) {
qerror_report(QERR_INVALID_PARAMETER_VALUE, "type",
"a netdev backend type");
return -1;
@@ -1145,6 +1171,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.h b/net.h
index 9f633f8..d1340ad 100644
--- a/net.h
+++ b/net.h
@@ -36,6 +36,7 @@ typedef enum {
NET_CLIENT_TYPE_SOCKET,
NET_CLIENT_TYPE_VDE,
NET_CLIENT_TYPE_DUMP,
+ NET_CLIENT_TYPE_BRIDGE,
NET_CLIENT_TYPE_MAX
} net_client_type;
@@ -174,6 +175,8 @@ int do_netdev_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
#define DEFAULT_NETWORK_SCRIPT "/etc/qemu-ifup"
#define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
+#define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR "/qemu-bridge-helper"
+#define DEFAULT_BRIDGE_INTERFACE "br0"
void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
diff --git a/net/tap.c b/net/tap.c
index 1f26dc9..be9be27 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -388,6 +388,143 @@ static int launch_script(const char *setup_script, const char *ifname, int fd)
return -1;
}
+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 bridge helper */
+ 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"));
+ if (fd == -1) {
+ return -1;
+ }
+
+ 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->nc.info_str, sizeof(s->nc.info_str),
+ "br=%s", qemu_opt_get(opts, "br"));
+
+ return 0;
+}
+
static int net_tap_init(QemuOpts *opts, int *vnet_hdr)
{
int fd, vnet_hdr_required;
@@ -433,8 +570,11 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
if (qemu_opt_get(opts, "ifname") ||
qemu_opt_get(opts, "script") ||
qemu_opt_get(opts, "downscript") ||
- qemu_opt_get(opts, "vnet_hdr")) {
- error_report("ifname=, script=, downscript= and vnet_hdr= is invalid with fd=");
+ qemu_opt_get(opts, "vnet_hdr") ||
+ qemu_opt_get(opts, "br") ||
+ qemu_opt_get(opts, "helper")) {
+ error_report("ifname=, script=, downscript=, vnet_hdr=,"
+ "br= and helper= are invalid with fd=");
return -1;
}
@@ -446,7 +586,40 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
fcntl(fd, F_SETFL, O_NONBLOCK);
vnet_hdr = tap_probe_vnet_hdr(fd);
+ } else if (qemu_opt_get(opts, "helper")) {
+ if (qemu_opt_get(opts, "ifname") ||
+ qemu_opt_get(opts, "script") ||
+ qemu_opt_get(opts, "downscript")) {
+ error_report("ifname=, script= and downscript="
+ "are invalid with helper=");
+ return -1;
+ }
+
+ if (!qemu_opt_get(opts, "br")) {
+ qemu_opt_set(opts, "br", DEFAULT_BRIDGE_INTERFACE);
+ }
+
+ fd = net_bridge_run_helper(qemu_opt_get(opts, "helper"),
+ qemu_opt_get(opts, "br"));
+ if (fd == -1) {
+ return -1;
+ }
+
+ 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;
+ }
} else {
+ if (qemu_opt_get(opts, "br")) {
+ error_report("br= is invalid with script=");
+ return -1;
+ }
+
if (!qemu_opt_get(opts, "script")) {
qemu_opt_set(opts, "script", DEFAULT_NETWORK_SCRIPT);
}
@@ -459,12 +632,12 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
if (fd == -1) {
return -1;
}
- }
- s = net_tap_fd_init(vlan, "tap", name, fd, vnet_hdr);
- if (!s) {
- close(fd);
- return -1;
+ s = net_tap_fd_init(vlan, "tap", name, fd, vnet_hdr);
+ if (!s) {
+ close(fd);
+ return -1;
+ }
}
if (tap_set_sndbuf(s->fd, opts) < 0) {
@@ -473,6 +646,9 @@ int net_init_tap(QemuOpts *opts, Monitor *mon, const char *name, VLANState *vlan
if (qemu_opt_get(opts, "fd")) {
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "fd=%d", fd);
+ } else if (qemu_opt_get(opts, "helper")) {
+ snprintf(s->nc.info_str, sizeof(s->nc.info_str),
+ "br=%s", qemu_opt_get(opts, "br"));
} else {
const char *ifname, *script, *downscript;
diff --git a/net/tap.h b/net/tap.h
index e44bd2b..56c591f 100644
--- a/net/tap.h
+++ b/net/tap.h
@@ -57,4 +57,7 @@ int tap_get_fd(VLANClientState *vc);
struct vhost_net;
struct vhost_net *tap_get_vhost_net(VLANClientState *vc);
+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 5d2a776..3f92232 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1179,11 +1179,14 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
"-net tap[,vlan=n][,name=str],ifname=name\n"
" connect the host TAP network interface to VLAN 'n'\n"
#else
- "-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostforce=on|off]\n"
- " connect the host TAP network interface to VLAN 'n' and use the\n"
- " network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
- " and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
+ "-net tap[,vlan=n][,name=str][,fd=h][,ifname=name][,script=file][,downscript=dfile][,br=bridge][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off][,vhostfd=h][,vhostforce=on|off]\n"
+ " connect the host TAP network interface to VLAN 'n' \n"
+ " use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
+ " to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
+ " to deconfigure it\n"
" use '[down]script=no' to disable script execution\n"
+ " use network helper 'helper' (default=" DEFAULT_BRIDGE_HELPER ") and\n"
+ " bridge 'br' (default=" DEFAULT_BRIDGE_INTERFACE ") to configure it\n"
" use 'fd=h' to connect to an already opened TAP interface\n"
" use 'sndbuf=nbytes' to limit the size of the send buffer (the\n"
" default is disabled 'sndbuf=0' to enable flow control set 'sndbuf=1048576')\n"
@@ -1193,6 +1196,10 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
" (only has effect for virtio guests which use MSIX)\n"
" use vhostforce=on to force vhost on for non-MSIX virtio guests\n"
" use 'vhostfd=h' to connect to an already opened vhost net device\n"
+ "-net bridge[,vlan=n][,name=str][,br=bridge][,helper=helper]\n"
+ " connects a host TAP network interface to a host bridge device 'br'\n"
+ " (default=" DEFAULT_BRIDGE_INTERFACE ") using the program 'helper'\n"
+ " (default=" DEFAULT_BRIDGE_HELPER ")\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"
@@ -1352,26 +1359,66 @@ processed and applied to -net user. Mixing them with the new configuration
syntax gives undefined results. Their use for new applications is discouraged
as they will be removed from future versions.
-@item -net tap[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,ifname=@var{name}] [,script=@var{file}][,downscript=@var{dfile}]
-Connect the host TAP network interface @var{name} to VLAN @var{n}, use
-the network script @var{file} to configure it and the network script
+@item -net tap[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,br=@var{bridge}][,helper=@var{helper}]
+Connect the host TAP network interface @var{name} to VLAN @var{n}.
+
+Use the network script @var{file} to configure it and the network script
@var{dfile} to deconfigure it. If @var{name} is not provided, the OS
-automatically provides one. @option{fd}=@var{h} can be used to specify
-the handle of an already opened host TAP interface. The default network
-configure script is @file{/etc/qemu-ifup} and the default network
-deconfigure script is @file{/etc/qemu-ifdown}. Use @option{script=no}
-or @option{downscript=no} to disable script execution. Example:
+automatically provides one. The default network configure script is
+@file{/etc/qemu-ifup} and the default network deconfigure script is
+@file{/etc/qemu-ifdown}. Use @option{script=no} or @option{downscript=no}
+to disable script execution.
+
+If running QEMU as an unprivileged user, use the network helper
+@var{helper} to configure the TAP interface. The default network
+helper executable is @file{/usr/local/libexec/qemu-bridge-helper}
+and the default bridge device is @file{br0}.
+
+@option{fd}=@var{h} can be used to specify the handle of an already
+opened host TAP interface.
+
+Examples:
@example
+#launch a QEMU instance with the default network script
qemu linux.img -net nic -net tap
@end example
-More complicated example (two NICs, each one connected to a TAP device)
@example
+#launch a QEMU instance with two NICs, each one connected
+#to a TAP device
qemu linux.img -net nic,vlan=0 -net tap,vlan=0,ifname=tap0 \
-net nic,vlan=1 -net tap,vlan=1,ifname=tap1
@end example
+@example
+#launch a QEMU instance with the default network helper to
+#connect a TAP device to bridge br0
+qemu linux.img -net nic -net tap,helper=/usr/local/libexec/qemu-bridge-helper
+@end example
+
+@item -net bridge[,vlan=@var{n}][,name=@var{name}][,br=@var{bridge}][,helper=@var{helper}]
+Connect a host TAP network interface to a host bridge device.
+
+Use the network helper @var{helper} to configure the TAP interface and
+attach it to the bridge. The default network helper executable is
+@file{/usr/local/libexec/qemu-bridge-helper} and the default bridge
+device is @file{br0}.
+
+Examples:
+
+@example
+#launch a QEMU instance with the default network helper to
+#connect a TAP device to bridge br0
+qemu linux.img -net bridge -net nic,model=virtio
+@end example
+
+@example
+#launch a QEMU instance with the default network helper to
+#connect a TAP device to bridge qemubr0
+qemu linux.img -net bridge,br=qemubr0 -net nic,model=virtio
+@end example
+
@item -net socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}] [,listen=[@var{host}]:@var{port}][,connect=@var{host}:@var{port}]
Connect the VLAN @var{n} to a remote VLAN in another QEMU virtual
--
1.7.3.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper Corey Bryant
@ 2011-10-23 12:56 ` Blue Swirl
2011-10-24 13:12 ` Corey Bryant
0 siblings, 1 reply; 20+ messages in thread
From: Blue Swirl @ 2011-10-23 12:56 UTC (permalink / raw)
To: Corey Bryant; +Cc: rmarwah, aliguori, qemu-devel
On Fri, Oct 21, 2011 at 15:07, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
> 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>
> Signed-off-by: Richa Marwaha <rmarwah@linux.vnet.ibm.com>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.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 f63fc02..d9b447e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -35,6 +35,8 @@ $(call set-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 QMP/qmp-commands.txt
> else
> @@ -75,7 +77,7 @@ defconfig:
>
> -include config-all-devices.mak
>
> -build-all: $(DOCS) $(TOOLS) recurse-all
> +build-all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all
>
> config-host.h: config-host.h-timestamp
> config-host.h-timestamp: config-host.mak
> @@ -153,6 +155,8 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y)
> qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y)
> qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-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)/scripts/hxtool -h < $< > $@," GEN $@")
>
> @@ -221,7 +225,7 @@ 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 qemu-options.def
> - rm -f *.o *.d *.a *.lo $(TOOLS) qemu-ga TAGS cscope.* *.pod *~ */*~
> + rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
> rm -Rf .libs
> rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o qga/*.d
> rm -f qemu-img-cmds.h
> @@ -289,6 +293,10 @@ install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
> 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 4f87e0a..6c8b659 100755
> --- a/configure
> +++ b/configure
> @@ -2768,6 +2768,7 @@ echo "datadir=$datadir" >> $config_host_mak
> echo "sysconfdir=$sysconfdir" >> $config_host_mak
> echo "docdir=$docdir" >> $config_host_mak
> echo "confdir=$confdir" >> $config_host_mak
> +echo "libexecdir=\${prefix}/libexec" >> $config_host_mak
>
> case "$cpu" in
> i386|x86_64|alpha|cris|hppa|ia64|lm32|m68k|microblaze|mips|mips64|ppc|ppc64|s390|s390x|sparc|sparc64|unicore32)
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> new file mode 100644
> index 0000000..2ce82fb
> --- /dev/null
> +++ b/qemu-bridge-helper.c
> @@ -0,0 +1,205 @@
> +/*
> + * QEMU Bridge Helper
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * 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 = 0;
> + 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;
> + }
It looks like only the above series of ioctls is Linux specific. I'm
not familiar if other OS could support similar bridges, if so, it
would be better to contain the bridge setup in a separate function.
This can be done later though.
> +
> + /* 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.7.3.4
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu bridge helper
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu " Corey Bryant
@ 2011-10-23 13:10 ` Blue Swirl
2011-10-24 13:44 ` Corey Bryant
0 siblings, 1 reply; 20+ messages in thread
From: Blue Swirl @ 2011-10-23 13:10 UTC (permalink / raw)
To: Corey Bryant; +Cc: rmarwah, aliguori, qemu-devel
On Fri, Oct 21, 2011 at 15:07, Corey Bryant <coreyb@linux.vnet.ibm.com> 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 an ACL mechanism that is enforced by qemu-bridge-helper.
> The ACLs are fairly simple whitelist/blacklist mechanisms with a wildcard of
> 'all'. All users are blacklisted by default, and deny takes precedence over
> allow.
>
> 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
sophisticated
> policies via the file system.
>
> As an example:
>
> /etc/qemu/bridge.conf root:qemu 0640
>
> allow br0
> include /etc/qemu/alice.conf
> include /etc/qemu/bob.conf
> include /etc/qemu/charlie.conf
>
> /etc/qemu/alice.conf root:alice 0640
> allow br1
>
> /etc/qemu/bob.conf root:bob 0640
> allow br2
>
> /etc/qemu/charlie.conf root:charlie 0640
> deny all
I think syntax 'include /etc/qemu/user.d/*.conf' or 'includedir
/etc/qemu/user.d' could be also useful.
> 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.
>
> Users in the charlie group cannot get a tap device connected to any bridge.
>
> Under no circumstance can the bob group get access to br1 or can the alice
> group get access to br2. And under no cicumstance can the charlie group
> get access to any bridge.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Richa Marwaha <rmarwah@linux.vnet.ibm.com>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
> qemu-bridge-helper.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 141 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
> index 2ce82fb..db257d5 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -33,6 +33,105 @@
>
> #include "net/tap-linux.h"
>
> +#define MAX_ACLS (128)
If all users (or groups) in the system have an ACL, this number could
be way too low. Please use a list instead.
> +#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--;
> + }
These while loops to skip spaces are repeated, but the comment
skipping part is not, so it is not possible to have comments after
rules or split rules to several lines. I'd add a simple state variable
to track at which stage we are in parsing instead.
> + *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 = 0;
> @@ -95,6 +194,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, access_denied;
>
> /* parse arguments */
> if (argc < 3 || argc > 4) {
> @@ -115,6 +217,45 @@ 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
> + * according acl policy if we have a deny and allow both
> + * then deny should always win over allow
> + */
> + access_allowed = 0;
> + access_denied = 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_denied = 1;
> + break;
> + case ACL_DENY:
> + if (strcmp(bridge, acls[i].iface) == 0) {
> + access_denied = 1;
> + }
> + break;
> + }
> + }
> +
> + if ((access_allowed == 0) || (access_denied == 1)) {
> + 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.7.3.4
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID Corey Bryant
@ 2011-10-23 13:22 ` Blue Swirl
2011-10-24 14:13 ` Corey Bryant
0 siblings, 1 reply; 20+ messages in thread
From: Blue Swirl @ 2011-10-23 13:22 UTC (permalink / raw)
To: Corey Bryant; +Cc: rmarwah, aliguori, qemu-devel
On Fri, Oct 21, 2011 at 15:07, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
> 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>
> Signed-off-by: Richa Marwaha <rmarwah@linux.vnet.ibm.com>
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
> configure | 34 ++++++++++++++++++++++++++++++++++
> qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 73 insertions(+), 0 deletions(-)
>
> diff --git a/configure b/configure
> index 6c8b659..fed66b0 100755
> --- a/configure
> +++ b/configure
> @@ -128,6 +128,7 @@ vnc_thread="no"
> xen=""
> xen_ctrl_version=""
> linux_aio=""
> +cap=""
> attr=""
> xfs=""
>
> @@ -653,6 +654,10 @@ for opt do
> ;;
> --enable-kvm) kvm="yes"
> ;;
> + --disable-cap) cap="no"
> + ;;
> + --enable-cap) cap="yes"
> + ;;
> --disable-spice) spice="no"
> ;;
> --enable-spice) spice="yes"
> @@ -1032,6 +1037,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-cap disable libcap-ng support"
> +echo " --enable-cap enable libcap-ng support"
> echo " --disable-attr disables attr and xattr support"
> echo " --enable-attr enable attr and xattr support"
> echo " --disable-blobs disable installing provided firmware blobs"
> @@ -1638,6 +1645,29 @@ EOF
> fi
>
> ##########################################
> +# libcap-ng library probe
> +if test "$cap" != "no" ; then
> + cap_libs="-lcap-ng"
> + cat > $TMPC << EOF
> +#include <cap-ng.h>
> +int main(void)
> +{
> + capng_capability_to_name(CAPNG_EFFECTIVE);
> + 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()
> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
> echo "madvise $madvise"
> echo "posix_madvise $posix_madvise"
> echo "uuid support $uuid"
> +echo "libcap-ng support $cap"
> echo "vhost-net support $vhost_net"
> echo "Trace backend $trace_backend"
> echo "Trace output file $trace_file-<pid>"
> @@ -2846,6 +2877,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 db257d5..b1562eb 100644
> --- a/qemu-bridge-helper.c
> +++ b/qemu-bridge-helper.c
> @@ -33,6 +33,10 @@
>
> #include "net/tap-linux.h"
>
> +#ifdef CONFIG_LIBCAP
> +#include <cap-ng.h>
> +#endif
> +
> #define MAX_ACLS (128)
> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>
> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
> return sendmsg(c, &msg, 0);
> }
>
> +#ifdef CONFIG_LIBCAP
> +static int drop_privileges(void)
> +{
> + /* clear all capabilities */
> + capng_clear(CAPNG_SELECT_BOTH);
> +
> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
> + CAP_NET_ADMIN) < 0) {
> + return -1;
> + }
> +
> + /* change to calling user's real uid and gid, retaining supplemental
> + * groups and CAP_NET_ADMIN */
> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> int main(int argc, char **argv)
> {
> struct ifreq ifr;
> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
> int acl_count = 0;
> int i, access_allowed, access_denied;
>
> + /* if we're run from an suid binary, immediately drop privileges preserving
> + * cap_net_admin -- exit immediately if libcap not configured */
> + if (geteuid() == 0 && getuid() != geteuid()) {
> +#ifdef CONFIG_LIBCAP
> + if (drop_privileges() == -1) {
> + fprintf(stderr, "failed to drop privileges\n");
> + return 1;
> + }
> +#else
> + fprintf(stderr, "failed to drop privileges\n");
This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
possible to use setfsuid() instead for Linux?
Some fork+setuid helper could be used for other Unix and for the lame
OSes without any file system DAC capabilities, a different syntax that
does not rely on underlying FS may need to be introduced. Again, I
don't know if the tool is even interesting for non-Linux.
> + return 1;
> +#endif
> + }
> +
> /* parse arguments */
> if (argc < 3 || argc > 4) {
> fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
> --
> 1.7.3.4
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper
2011-10-23 12:56 ` Blue Swirl
@ 2011-10-24 13:12 ` Corey Bryant
0 siblings, 0 replies; 20+ messages in thread
From: Corey Bryant @ 2011-10-24 13:12 UTC (permalink / raw)
To: Blue Swirl; +Cc: rmarwah, aliguori, qemu-devel
On 10/23/2011 08:56 AM, Blue Swirl wrote:
> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com> wrote:
>> 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>
>> Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.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 f63fc02..d9b447e 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -35,6 +35,8 @@ $(call set-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 QMP/qmp-commands.txt
>> else
>> @@ -75,7 +77,7 @@ defconfig:
>>
>> -include config-all-devices.mak
>>
>> -build-all: $(DOCS) $(TOOLS) recurse-all
>> +build-all: $(DOCS) $(TOOLS) $(HELPERS-y) recurse-all
>>
>> config-host.h: config-host.h-timestamp
>> config-host.h-timestamp: config-host.mak
>> @@ -153,6 +155,8 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y)
>> qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y)
>> qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-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)/scripts/hxtool -h< $< > $@," GEN $@")
>>
>> @@ -221,7 +225,7 @@ 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 qemu-options.def
>> - rm -f *.o *.d *.a *.lo $(TOOLS) qemu-ga TAGS cscope.* *.pod *~ */*~
>> + rm -f *.o *.d *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~
>> rm -Rf .libs
>> rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o qga/*.d
>> rm -f qemu-img-cmds.h
>> @@ -289,6 +293,10 @@ install: all $(if $(BUILD_DOCS),install-doc) install-sysconfig
>> 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 4f87e0a..6c8b659 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2768,6 +2768,7 @@ echo "datadir=$datadir">> $config_host_mak
>> echo "sysconfdir=$sysconfdir">> $config_host_mak
>> echo "docdir=$docdir">> $config_host_mak
>> echo "confdir=$confdir">> $config_host_mak
>> +echo "libexecdir=\${prefix}/libexec">> $config_host_mak
>>
>> case "$cpu" in
>> i386|x86_64|alpha|cris|hppa|ia64|lm32|m68k|microblaze|mips|mips64|ppc|ppc64|s390|s390x|sparc|sparc64|unicore32)
>> diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
>> new file mode 100644
>> index 0000000..2ce82fb
>> --- /dev/null
>> +++ b/qemu-bridge-helper.c
>> @@ -0,0 +1,205 @@
>> +/*
>> + * QEMU Bridge Helper
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * 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 = 0;
>> + 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;
>> + }
>
> It looks like only the above series of ioctls is Linux specific. I'm
> not familiar if other OS could support similar bridges, if so, it
> would be better to contain the bridge setup in a separate function.
> This can be done later though.
>
I agree.
>> +
>> + /* 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.7.3.4
>>
>>
>>
>
--
Regards,
Corey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu bridge helper
2011-10-23 13:10 ` Blue Swirl
@ 2011-10-24 13:44 ` Corey Bryant
2011-10-24 16:58 ` Blue Swirl
0 siblings, 1 reply; 20+ messages in thread
From: Corey Bryant @ 2011-10-24 13:44 UTC (permalink / raw)
To: Blue Swirl; +Cc: rmarwah, aliguori, qemu-devel
On 10/23/2011 09:10 AM, Blue Swirl wrote:
> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com> 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 an ACL mechanism that is enforced by qemu-bridge-helper.
>> > The ACLs are fairly simple whitelist/blacklist mechanisms with a wildcard of
>> > 'all'. All users are blacklisted by default, and deny takes precedence over
>> > allow.
>> >
>> > 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
> sophisticated
>
Yep, thanks.
>> > policies via the file system.
>> >
>> > As an example:
>> >
>> > /etc/qemu/bridge.conf root:qemu 0640
>> >
>> > allow br0
>> > include /etc/qemu/alice.conf
>> > include /etc/qemu/bob.conf
>> > include /etc/qemu/charlie.conf
>> >
>> > /etc/qemu/alice.conf root:alice 0640
>> > allow br1
>> >
>> > /etc/qemu/bob.conf root:bob 0640
>> > allow br2
>> >
>> > /etc/qemu/charlie.conf root:charlie 0640
>> > deny all
> I think syntax 'include/etc/qemu/user.d/*.conf' or 'includedir
> /etc/qemu/user.d' could be also useful.
>
That could be useful, though I'm not sure it's necessary right now.
>> > 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.
>> >
>> > Users in the charlie group cannot get a tap device connected to any bridge.
>> >
>> > Under no circumstance can the bob group get access to br1 or can the alice
>> > group get access to br2. And under no cicumstance can the charlie group
>> > get access to any bridge.
>> >
>> > Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> > Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>> > Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>> > ---
>> > qemu-bridge-helper.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> > 1 files changed, 141 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
>> > index 2ce82fb..db257d5 100644
>> > --- a/qemu-bridge-helper.c
>> > +++ b/qemu-bridge-helper.c
>> > @@ -33,6 +33,105 @@
>> >
>> > #include "net/tap-linux.h"
>> >
>> > +#define MAX_ACLS (128)
> If all users (or groups) in the system have an ACL, this number could
> be way too low. Please use a list instead.
>
I agree, we shouldn't be hard-coding the limit here. I'll update this.
>> > +#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--;
>> > + }
> These while loops to skip spaces are repeated, but the comment
> skipping part is not, so it is not possible to have comments after
> rules or split rules to several lines. I'd add a simple state variable
> to track at which stage we are in parsing instead.
>
That could be useful too, but again not sure it's necessary right now.
I really like the simplicity we have with the existing approach.
>> > + *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 = 0;
>> > @@ -95,6 +194,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, access_denied;
>> >
>> > /* parse arguments */
>> > if (argc< 3 || argc> 4) {
>> > @@ -115,6 +217,45 @@ 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
>> > + * according acl policy if we have a deny and allow both
>> > + * then deny should always win over allow
>> > + */
>> > + access_allowed = 0;
>> > + access_denied = 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_denied = 1;
>> > + break;
>> > + case ACL_DENY:
>> > + if (strcmp(bridge, acls[i].iface) == 0) {
>> > + access_denied = 1;
>> > + }
>> > + break;
>> > + }
>> > + }
>> > +
>> > + if ((access_allowed == 0) || (access_denied == 1)) {
>> > + 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.7.3.4
>> >
>> >
>> >
--
Regards,
Corey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-23 13:22 ` Blue Swirl
@ 2011-10-24 14:13 ` Corey Bryant
2011-10-24 17:10 ` Blue Swirl
2011-10-24 19:19 ` Anthony Liguori
0 siblings, 2 replies; 20+ messages in thread
From: Corey Bryant @ 2011-10-24 14:13 UTC (permalink / raw)
To: Blue Swirl; +Cc: rmarwah, aliguori, qemu-devel
On 10/23/2011 09:22 AM, Blue Swirl wrote:
> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com> wrote:
>> 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>
>> Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>> ---
>> configure | 34 ++++++++++++++++++++++++++++++++++
>> qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 73 insertions(+), 0 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 6c8b659..fed66b0 100755
>> --- a/configure
>> +++ b/configure
>> @@ -128,6 +128,7 @@ vnc_thread="no"
>> xen=""
>> xen_ctrl_version=""
>> linux_aio=""
>> +cap=""
>> attr=""
>> xfs=""
>>
>> @@ -653,6 +654,10 @@ for opt do
>> ;;
>> --enable-kvm) kvm="yes"
>> ;;
>> + --disable-cap) cap="no"
>> + ;;
>> + --enable-cap) cap="yes"
>> + ;;
>> --disable-spice) spice="no"
>> ;;
>> --enable-spice) spice="yes"
>> @@ -1032,6 +1037,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-cap disable libcap-ng support"
>> +echo " --enable-cap enable libcap-ng support"
>> echo " --disable-attr disables attr and xattr support"
>> echo " --enable-attr enable attr and xattr support"
>> echo " --disable-blobs disable installing provided firmware blobs"
>> @@ -1638,6 +1645,29 @@ EOF
>> fi
>>
>> ##########################################
>> +# libcap-ng library probe
>> +if test "$cap" != "no" ; then
>> + cap_libs="-lcap-ng"
>> + cat> $TMPC<< EOF
>> +#include<cap-ng.h>
>> +int main(void)
>> +{
>> + capng_capability_to_name(CAPNG_EFFECTIVE);
>> + 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()
>> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
>> echo "madvise $madvise"
>> echo "posix_madvise $posix_madvise"
>> echo "uuid support $uuid"
>> +echo "libcap-ng support $cap"
>> echo "vhost-net support $vhost_net"
>> echo "Trace backend $trace_backend"
>> echo "Trace output file $trace_file-<pid>"
>> @@ -2846,6 +2877,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 db257d5..b1562eb 100644
>> --- a/qemu-bridge-helper.c
>> +++ b/qemu-bridge-helper.c
>> @@ -33,6 +33,10 @@
>>
>> #include "net/tap-linux.h"
>>
>> +#ifdef CONFIG_LIBCAP
>> +#include<cap-ng.h>
>> +#endif
>> +
>> #define MAX_ACLS (128)
>> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>>
>> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
>> return sendmsg(c,&msg, 0);
>> }
>>
>> +#ifdef CONFIG_LIBCAP
>> +static int drop_privileges(void)
>> +{
>> + /* clear all capabilities */
>> + capng_clear(CAPNG_SELECT_BOTH);
>> +
>> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
>> + CAP_NET_ADMIN)< 0) {
>> + return -1;
>> + }
>> +
>> + /* change to calling user's real uid and gid, retaining supplemental
>> + * groups and CAP_NET_ADMIN */
>> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +#endif
>> +
>> int main(int argc, char **argv)
>> {
>> struct ifreq ifr;
>> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
>> int acl_count = 0;
>> int i, access_allowed, access_denied;
>>
>> + /* if we're run from an suid binary, immediately drop privileges preserving
>> + * cap_net_admin -- exit immediately if libcap not configured */
>> + if (geteuid() == 0&& getuid() != geteuid()) {
>> +#ifdef CONFIG_LIBCAP
>> + if (drop_privileges() == -1) {
>> + fprintf(stderr, "failed to drop privileges\n");
>> + return 1;
>> + }
>> +#else
>> + fprintf(stderr, "failed to drop privileges\n");
>
> This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
> possible to use setfsuid() instead for Linux?
>
> Some fork+setuid helper could be used for other Unix and for the lame
> OSes without any file system DAC capabilities, a different syntax that
> does not rely on underlying FS may need to be introduced. Again, I
> don't know if the tool is even interesting for non-Linux.
>
I just want to make sure that there is no chance that the helper is run
as root beyond this point. Are you saying to seteuid(getuid) and
setfsuid(root)? I'm not sure that would drop the privileges enough.
>> + return 1;
>> +#endif
>> + }
>> +
>> /* parse arguments */
>> if (argc< 3 || argc> 4) {
>> fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
>> --
>> 1.7.3.4
>>
>>
>>
>
--
Regards,
Corey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu bridge helper
2011-10-24 13:44 ` Corey Bryant
@ 2011-10-24 16:58 ` Blue Swirl
0 siblings, 0 replies; 20+ messages in thread
From: Blue Swirl @ 2011-10-24 16:58 UTC (permalink / raw)
To: Corey Bryant; +Cc: rmarwah, aliguori, qemu-devel
On Mon, Oct 24, 2011 at 13:44, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>
> On 10/23/2011 09:10 AM, Blue Swirl wrote:
>>
>> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com>
>> 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 an ACL mechanism that is enforced by
>>> > qemu-bridge-helper.
>>> > The ACLs are fairly simple whitelist/blacklist mechanisms with a
>>> > wildcard of
>>> > 'all'. All users are blacklisted by default, and deny takes
>>> > precedence over
>>> > allow.
>>> >
>>> > 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
>>
>> sophisticated
>>
>
> Yep, thanks.
>
>>> > policies via the file system.
>>> >
>>> > As an example:
>>> >
>>> > /etc/qemu/bridge.conf root:qemu 0640
>>> >
>>> > allow br0
>>> > include /etc/qemu/alice.conf
>>> > include /etc/qemu/bob.conf
>>> > include /etc/qemu/charlie.conf
>>> >
>>> > /etc/qemu/alice.conf root:alice 0640
>>> > allow br1
>>> >
>>> > /etc/qemu/bob.conf root:bob 0640
>>> > allow br2
>>> >
>>> > /etc/qemu/charlie.conf root:charlie 0640
>>> > deny all
>>
>> I think syntax 'include/etc/qemu/user.d/*.conf' or 'includedir
>> /etc/qemu/user.d' could be also useful.
>>
>
> That could be useful, though I'm not sure it's necessary right now.
It can be added later.
>>> > 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.
>>> >
>>> > Users in the charlie group cannot get a tap device connected to any
>>> > bridge.
>>> >
>>> > Under no circumstance can the bob group get access to br1 or can the
>>> > alice
>>> > group get access to br2. And under no cicumstance can the charlie
>>> > group
>>> > get access to any bridge.
>>> >
>>> > Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>> > Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>>> > Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>>> > ---
>>> > qemu-bridge-helper.c | 141
>>> > ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> > 1 files changed, 141 insertions(+), 0 deletions(-)
>>> >
>>> > diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
>>> > index 2ce82fb..db257d5 100644
>>> > --- a/qemu-bridge-helper.c
>>> > +++ b/qemu-bridge-helper.c
>>> > @@ -33,6 +33,105 @@
>>> >
>>> > #include "net/tap-linux.h"
>>> >
>>> > +#define MAX_ACLS (128)
>>
>> If all users (or groups) in the system have an ACL, this number could
>> be way too low. Please use a list instead.
>>
>
> I agree, we shouldn't be hard-coding the limit here. I'll update this.
>
>>> > +#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--;
>>> > + }
>>
>> These while loops to skip spaces are repeated, but the comment
>> skipping part is not, so it is not possible to have comments after
>> rules or split rules to several lines. I'd add a simple state variable
>> to track at which stage we are in parsing instead.
>>
>
> That could be useful too, but again not sure it's necessary right now. I
> really like the simplicity we have with the existing approach.
It's not necessary, more like cleanup _if_ it turns out to be even simpler.
>>> > + *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 = 0;
>>> > @@ -95,6 +194,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, access_denied;
>>> >
>>> > /* parse arguments */
>>> > if (argc< 3 || argc> 4) {
>>> > @@ -115,6 +217,45 @@ 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
>>> > + * according acl policy if we have a deny and allow both
>>> > + * then deny should always win over allow
>>> > + */
>>> > + access_allowed = 0;
>>> > + access_denied = 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_denied = 1;
>>> > + break;
>>> > + case ACL_DENY:
>>> > + if (strcmp(bridge, acls[i].iface) == 0) {
>>> > + access_denied = 1;
>>> > + }
>>> > + break;
>>> > + }
>>> > + }
>>> > +
>>> > + if ((access_allowed == 0) || (access_denied == 1)) {
>>> > + 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.7.3.4
>>> >
>>> >
>>> >
>
> --
> Regards,
> Corey
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-24 14:13 ` Corey Bryant
@ 2011-10-24 17:10 ` Blue Swirl
2011-10-24 18:38 ` Corey Bryant
2011-10-24 19:19 ` Anthony Liguori
1 sibling, 1 reply; 20+ messages in thread
From: Blue Swirl @ 2011-10-24 17:10 UTC (permalink / raw)
To: Corey Bryant; +Cc: rmarwah, aliguori, qemu-devel
On Mon, Oct 24, 2011 at 14:13, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>
> On 10/23/2011 09:22 AM, Blue Swirl wrote:
>>
>> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com>
>> wrote:
>>>
>>> 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>
>>> Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>>> ---
>>> configure | 34 ++++++++++++++++++++++++++++++++++
>>> qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 73 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 6c8b659..fed66b0 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -128,6 +128,7 @@ vnc_thread="no"
>>> xen=""
>>> xen_ctrl_version=""
>>> linux_aio=""
>>> +cap=""
>>> attr=""
>>> xfs=""
>>>
>>> @@ -653,6 +654,10 @@ for opt do
>>> ;;
>>> --enable-kvm) kvm="yes"
>>> ;;
>>> + --disable-cap) cap="no"
>>> + ;;
>>> + --enable-cap) cap="yes"
>>> + ;;
>>> --disable-spice) spice="no"
>>> ;;
>>> --enable-spice) spice="yes"
>>> @@ -1032,6 +1037,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-cap disable libcap-ng support"
>>> +echo " --enable-cap enable libcap-ng support"
>>> echo " --disable-attr disables attr and xattr support"
>>> echo " --enable-attr enable attr and xattr support"
>>> echo " --disable-blobs disable installing provided firmware
>>> blobs"
>>> @@ -1638,6 +1645,29 @@ EOF
>>> fi
>>>
>>> ##########################################
>>> +# libcap-ng library probe
>>> +if test "$cap" != "no" ; then
>>> + cap_libs="-lcap-ng"
>>> + cat> $TMPC<< EOF
>>> +#include<cap-ng.h>
>>> +int main(void)
>>> +{
>>> + capng_capability_to_name(CAPNG_EFFECTIVE);
>>> + 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()
>>> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
>>> echo "madvise $madvise"
>>> echo "posix_madvise $posix_madvise"
>>> echo "uuid support $uuid"
>>> +echo "libcap-ng support $cap"
>>> echo "vhost-net support $vhost_net"
>>> echo "Trace backend $trace_backend"
>>> echo "Trace output file $trace_file-<pid>"
>>> @@ -2846,6 +2877,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 db257d5..b1562eb 100644
>>> --- a/qemu-bridge-helper.c
>>> +++ b/qemu-bridge-helper.c
>>> @@ -33,6 +33,10 @@
>>>
>>> #include "net/tap-linux.h"
>>>
>>> +#ifdef CONFIG_LIBCAP
>>> +#include<cap-ng.h>
>>> +#endif
>>> +
>>> #define MAX_ACLS (128)
>>> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>>>
>>> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
>>> return sendmsg(c,&msg, 0);
>>> }
>>>
>>> +#ifdef CONFIG_LIBCAP
>>> +static int drop_privileges(void)
>>> +{
>>> + /* clear all capabilities */
>>> + capng_clear(CAPNG_SELECT_BOTH);
>>> +
>>> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
>>> + CAP_NET_ADMIN)< 0) {
>>> + return -1;
>>> + }
>>> +
>>> + /* change to calling user's real uid and gid, retaining supplemental
>>> + * groups and CAP_NET_ADMIN */
>>> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
>>> + return -1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> int main(int argc, char **argv)
>>> {
>>> struct ifreq ifr;
>>> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
>>> int acl_count = 0;
>>> int i, access_allowed, access_denied;
>>>
>>> + /* if we're run from an suid binary, immediately drop privileges
>>> preserving
>>> + * cap_net_admin -- exit immediately if libcap not configured */
>>> + if (geteuid() == 0&& getuid() != geteuid()) {
>>> +#ifdef CONFIG_LIBCAP
>>> + if (drop_privileges() == -1) {
>>> + fprintf(stderr, "failed to drop privileges\n");
>>> + return 1;
>>> + }
>>> +#else
>>> + fprintf(stderr, "failed to drop privileges\n");
>>
>> This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
>> possible to use setfsuid() instead for Linux?
>>
>> Some fork+setuid helper could be used for other Unix and for the lame
>> OSes without any file system DAC capabilities, a different syntax that
>> does not rely on underlying FS may need to be introduced. Again, I
>> don't know if the tool is even interesting for non-Linux.
>>
>
> I just want to make sure that there is no chance that the helper is run as
> root beyond this point. Are you saying to seteuid(getuid) and
> setfsuid(root)? I'm not sure that would drop the privileges enough.
Without capabilities, we can't drop root privileges because bridge
setup would fail otherwise, but we could use setfsuid(getuid()) and
setfsgid(getgid()) during file access so permission checks work.
Perhaps non-Linux could use seteuid() etc. instead.
>>> + return 1;
>>> +#endif
>>> + }
>>> +
>>> /* parse arguments */
>>> if (argc< 3 || argc> 4) {
>>> fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
>>> --
>>> 1.7.3.4
>>>
>>>
>>>
>>
>
> --
> Regards,
> Corey
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-24 17:10 ` Blue Swirl
@ 2011-10-24 18:38 ` Corey Bryant
2011-10-24 18:58 ` Blue Swirl
0 siblings, 1 reply; 20+ messages in thread
From: Corey Bryant @ 2011-10-24 18:38 UTC (permalink / raw)
To: Blue Swirl; +Cc: rmarwah, aliguori, qemu-devel
On 10/24/2011 01:10 PM, Blue Swirl wrote:
> On Mon, Oct 24, 2011 at 14:13, Corey Bryant<coreyb@linux.vnet.ibm.com> wrote:
>>
>>
>> On 10/23/2011 09:22 AM, Blue Swirl wrote:
>>>
>>> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com>
>>> wrote:
>>>>
>>>> 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>
>>>> Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>>>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>>>> ---
>>>> configure | 34 ++++++++++++++++++++++++++++++++++
>>>> qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 73 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 6c8b659..fed66b0 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -128,6 +128,7 @@ vnc_thread="no"
>>>> xen=""
>>>> xen_ctrl_version=""
>>>> linux_aio=""
>>>> +cap=""
>>>> attr=""
>>>> xfs=""
>>>>
>>>> @@ -653,6 +654,10 @@ for opt do
>>>> ;;
>>>> --enable-kvm) kvm="yes"
>>>> ;;
>>>> + --disable-cap) cap="no"
>>>> + ;;
>>>> + --enable-cap) cap="yes"
>>>> + ;;
>>>> --disable-spice) spice="no"
>>>> ;;
>>>> --enable-spice) spice="yes"
>>>> @@ -1032,6 +1037,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-cap disable libcap-ng support"
>>>> +echo " --enable-cap enable libcap-ng support"
>>>> echo " --disable-attr disables attr and xattr support"
>>>> echo " --enable-attr enable attr and xattr support"
>>>> echo " --disable-blobs disable installing provided firmware
>>>> blobs"
>>>> @@ -1638,6 +1645,29 @@ EOF
>>>> fi
>>>>
>>>> ##########################################
>>>> +# libcap-ng library probe
>>>> +if test "$cap" != "no" ; then
>>>> + cap_libs="-lcap-ng"
>>>> + cat> $TMPC<< EOF
>>>> +#include<cap-ng.h>
>>>> +int main(void)
>>>> +{
>>>> + capng_capability_to_name(CAPNG_EFFECTIVE);
>>>> + 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()
>>>> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
>>>> echo "madvise $madvise"
>>>> echo "posix_madvise $posix_madvise"
>>>> echo "uuid support $uuid"
>>>> +echo "libcap-ng support $cap"
>>>> echo "vhost-net support $vhost_net"
>>>> echo "Trace backend $trace_backend"
>>>> echo "Trace output file $trace_file-<pid>"
>>>> @@ -2846,6 +2877,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 db257d5..b1562eb 100644
>>>> --- a/qemu-bridge-helper.c
>>>> +++ b/qemu-bridge-helper.c
>>>> @@ -33,6 +33,10 @@
>>>>
>>>> #include "net/tap-linux.h"
>>>>
>>>> +#ifdef CONFIG_LIBCAP
>>>> +#include<cap-ng.h>
>>>> +#endif
>>>> +
>>>> #define MAX_ACLS (128)
>>>> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>>>>
>>>> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
>>>> return sendmsg(c,&msg, 0);
>>>> }
>>>>
>>>> +#ifdef CONFIG_LIBCAP
>>>> +static int drop_privileges(void)
>>>> +{
>>>> + /* clear all capabilities */
>>>> + capng_clear(CAPNG_SELECT_BOTH);
>>>> +
>>>> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
>>>> + CAP_NET_ADMIN)< 0) {
>>>> + return -1;
>>>> + }
>>>> +
>>>> + /* change to calling user's real uid and gid, retaining supplemental
>>>> + * groups and CAP_NET_ADMIN */
>>>> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
>>>> + return -1;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +#endif
>>>> +
>>>> int main(int argc, char **argv)
>>>> {
>>>> struct ifreq ifr;
>>>> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
>>>> int acl_count = 0;
>>>> int i, access_allowed, access_denied;
>>>>
>>>> + /* if we're run from an suid binary, immediately drop privileges
>>>> preserving
>>>> + * cap_net_admin -- exit immediately if libcap not configured */
>>>> + if (geteuid() == 0&& getuid() != geteuid()) {
>>>> +#ifdef CONFIG_LIBCAP
>>>> + if (drop_privileges() == -1) {
>>>> + fprintf(stderr, "failed to drop privileges\n");
>>>> + return 1;
>>>> + }
>>>> +#else
>>>> + fprintf(stderr, "failed to drop privileges\n");
>>>
>>> This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
>>> possible to use setfsuid() instead for Linux?
>>>
>>> Some fork+setuid helper could be used for other Unix and for the lame
>>> OSes without any file system DAC capabilities, a different syntax that
>>> does not rely on underlying FS may need to be introduced. Again, I
>>> don't know if the tool is even interesting for non-Linux.
>>>
>>
>> I just want to make sure that there is no chance that the helper is run as
>> root beyond this point. Are you saying to seteuid(getuid) and
>> setfsuid(root)? I'm not sure that would drop the privileges enough.
>
> Without capabilities, we can't drop root privileges because bridge
> setup would fail otherwise, but we could use setfsuid(getuid()) and
> setfsgid(getgid()) during file access so permission checks work.
> Perhaps non-Linux could use seteuid() etc. instead.
>
This would reduce file system access from effective UID/GID (root/root)
to real UID/GID (non-root/non-root). Other than file system access, the
helper would still run under root/root, right? I don't think we want
that from a security aspect.
--
Regards,
Corey
>>>> + return 1;
>>>> +#endif
>>>> + }
>>>> +
>>>> /* parse arguments */
>>>> if (argc< 3 || argc> 4) {
>>>> fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
>>>> --
>>>> 1.7.3.4
>>>>
>>>>
>>>>
>>>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-24 18:38 ` Corey Bryant
@ 2011-10-24 18:58 ` Blue Swirl
2011-10-24 19:13 ` Corey Bryant
0 siblings, 1 reply; 20+ messages in thread
From: Blue Swirl @ 2011-10-24 18:58 UTC (permalink / raw)
To: Corey Bryant; +Cc: rmarwah, aliguori, qemu-devel
On Mon, Oct 24, 2011 at 18:38, Corey Bryant <coreyb@linux.vnet.ibm.com> wrote:
>
>
> On 10/24/2011 01:10 PM, Blue Swirl wrote:
>>
>> On Mon, Oct 24, 2011 at 14:13, Corey Bryant<coreyb@linux.vnet.ibm.com>
>> wrote:
>>>
>>>
>>> On 10/23/2011 09:22 AM, Blue Swirl wrote:
>>>>
>>>> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com>
>>>> wrote:
>>>>>
>>>>> 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>
>>>>> Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>>>>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>>>>> ---
>>>>> configure | 34 ++++++++++++++++++++++++++++++++++
>>>>> qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 73 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/configure b/configure
>>>>> index 6c8b659..fed66b0 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -128,6 +128,7 @@ vnc_thread="no"
>>>>> xen=""
>>>>> xen_ctrl_version=""
>>>>> linux_aio=""
>>>>> +cap=""
>>>>> attr=""
>>>>> xfs=""
>>>>>
>>>>> @@ -653,6 +654,10 @@ for opt do
>>>>> ;;
>>>>> --enable-kvm) kvm="yes"
>>>>> ;;
>>>>> + --disable-cap) cap="no"
>>>>> + ;;
>>>>> + --enable-cap) cap="yes"
>>>>> + ;;
>>>>> --disable-spice) spice="no"
>>>>> ;;
>>>>> --enable-spice) spice="yes"
>>>>> @@ -1032,6 +1037,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-cap disable libcap-ng support"
>>>>> +echo " --enable-cap enable libcap-ng support"
>>>>> echo " --disable-attr disables attr and xattr support"
>>>>> echo " --enable-attr enable attr and xattr support"
>>>>> echo " --disable-blobs disable installing provided firmware
>>>>> blobs"
>>>>> @@ -1638,6 +1645,29 @@ EOF
>>>>> fi
>>>>>
>>>>> ##########################################
>>>>> +# libcap-ng library probe
>>>>> +if test "$cap" != "no" ; then
>>>>> + cap_libs="-lcap-ng"
>>>>> + cat> $TMPC<< EOF
>>>>> +#include<cap-ng.h>
>>>>> +int main(void)
>>>>> +{
>>>>> + capng_capability_to_name(CAPNG_EFFECTIVE);
>>>>> + 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()
>>>>> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
>>>>> echo "madvise $madvise"
>>>>> echo "posix_madvise $posix_madvise"
>>>>> echo "uuid support $uuid"
>>>>> +echo "libcap-ng support $cap"
>>>>> echo "vhost-net support $vhost_net"
>>>>> echo "Trace backend $trace_backend"
>>>>> echo "Trace output file $trace_file-<pid>"
>>>>> @@ -2846,6 +2877,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 db257d5..b1562eb 100644
>>>>> --- a/qemu-bridge-helper.c
>>>>> +++ b/qemu-bridge-helper.c
>>>>> @@ -33,6 +33,10 @@
>>>>>
>>>>> #include "net/tap-linux.h"
>>>>>
>>>>> +#ifdef CONFIG_LIBCAP
>>>>> +#include<cap-ng.h>
>>>>> +#endif
>>>>> +
>>>>> #define MAX_ACLS (128)
>>>>> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>>>>>
>>>>> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
>>>>> return sendmsg(c,&msg, 0);
>>>>> }
>>>>>
>>>>> +#ifdef CONFIG_LIBCAP
>>>>> +static int drop_privileges(void)
>>>>> +{
>>>>> + /* clear all capabilities */
>>>>> + capng_clear(CAPNG_SELECT_BOTH);
>>>>> +
>>>>> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
>>>>> + CAP_NET_ADMIN)< 0) {
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + /* change to calling user's real uid and gid, retaining
>>>>> supplemental
>>>>> + * groups and CAP_NET_ADMIN */
>>>>> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> int main(int argc, char **argv)
>>>>> {
>>>>> struct ifreq ifr;
>>>>> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
>>>>> int acl_count = 0;
>>>>> int i, access_allowed, access_denied;
>>>>>
>>>>> + /* if we're run from an suid binary, immediately drop privileges
>>>>> preserving
>>>>> + * cap_net_admin -- exit immediately if libcap not configured */
>>>>> + if (geteuid() == 0&& getuid() != geteuid()) {
>>>>> +#ifdef CONFIG_LIBCAP
>>>>> + if (drop_privileges() == -1) {
>>>>> + fprintf(stderr, "failed to drop privileges\n");
>>>>> + return 1;
>>>>> + }
>>>>> +#else
>>>>> + fprintf(stderr, "failed to drop privileges\n");
>>>>
>>>> This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
>>>> possible to use setfsuid() instead for Linux?
>>>>
>>>> Some fork+setuid helper could be used for other Unix and for the lame
>>>> OSes without any file system DAC capabilities, a different syntax that
>>>> does not rely on underlying FS may need to be introduced. Again, I
>>>> don't know if the tool is even interesting for non-Linux.
>>>>
>>>
>>> I just want to make sure that there is no chance that the helper is run
>>> as
>>> root beyond this point. Are you saying to seteuid(getuid) and
>>> setfsuid(root)? I'm not sure that would drop the privileges enough.
>>
>> Without capabilities, we can't drop root privileges because bridge
>> setup would fail otherwise, but we could use setfsuid(getuid()) and
>> setfsgid(getgid()) during file access so permission checks work.
>> Perhaps non-Linux could use seteuid() etc. instead.
>>
>
> This would reduce file system access from effective UID/GID (root/root) to
> real UID/GID (non-root/non-root). Other than file system access, the helper
> would still run under root/root, right? I don't think we want that from a
> security aspect.
Right, it's not desirable, but isn't that the best we can do without
libcap or FS capabilities?
> --
> Regards,
> Corey
>
>>>>> + return 1;
>>>>> +#endif
>>>>> + }
>>>>> +
>>>>> /* parse arguments */
>>>>> if (argc< 3 || argc> 4) {
>>>>> fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
>>>>> --
>>>>> 1.7.3.4
>>>>>
>>>>>
>>>>>
>>>>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-24 18:58 ` Blue Swirl
@ 2011-10-24 19:13 ` Corey Bryant
2011-10-24 19:21 ` Anthony Liguori
0 siblings, 1 reply; 20+ messages in thread
From: Corey Bryant @ 2011-10-24 19:13 UTC (permalink / raw)
To: Blue Swirl; +Cc: rmarwah, aliguori, qemu-devel
On 10/24/2011 02:58 PM, Blue Swirl wrote:
> On Mon, Oct 24, 2011 at 18:38, Corey Bryant<coreyb@linux.vnet.ibm.com> wrote:
>>
>>
>> On 10/24/2011 01:10 PM, Blue Swirl wrote:
>>>
>>> On Mon, Oct 24, 2011 at 14:13, Corey Bryant<coreyb@linux.vnet.ibm.com>
>>> wrote:
>>>>
>>>>
>>>> On 10/23/2011 09:22 AM, Blue Swirl wrote:
>>>>>
>>>>> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com>
>>>>> wrote:
>>>>>>
>>>>>> 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>
>>>>>> Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>>>>>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> configure | 34 ++++++++++++++++++++++++++++++++++
>>>>>> qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 73 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/configure b/configure
>>>>>> index 6c8b659..fed66b0 100755
>>>>>> --- a/configure
>>>>>> +++ b/configure
>>>>>> @@ -128,6 +128,7 @@ vnc_thread="no"
>>>>>> xen=""
>>>>>> xen_ctrl_version=""
>>>>>> linux_aio=""
>>>>>> +cap=""
>>>>>> attr=""
>>>>>> xfs=""
>>>>>>
>>>>>> @@ -653,6 +654,10 @@ for opt do
>>>>>> ;;
>>>>>> --enable-kvm) kvm="yes"
>>>>>> ;;
>>>>>> + --disable-cap) cap="no"
>>>>>> + ;;
>>>>>> + --enable-cap) cap="yes"
>>>>>> + ;;
>>>>>> --disable-spice) spice="no"
>>>>>> ;;
>>>>>> --enable-spice) spice="yes"
>>>>>> @@ -1032,6 +1037,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-cap disable libcap-ng support"
>>>>>> +echo " --enable-cap enable libcap-ng support"
>>>>>> echo " --disable-attr disables attr and xattr support"
>>>>>> echo " --enable-attr enable attr and xattr support"
>>>>>> echo " --disable-blobs disable installing provided firmware
>>>>>> blobs"
>>>>>> @@ -1638,6 +1645,29 @@ EOF
>>>>>> fi
>>>>>>
>>>>>> ##########################################
>>>>>> +# libcap-ng library probe
>>>>>> +if test "$cap" != "no" ; then
>>>>>> + cap_libs="-lcap-ng"
>>>>>> + cat> $TMPC<< EOF
>>>>>> +#include<cap-ng.h>
>>>>>> +int main(void)
>>>>>> +{
>>>>>> + capng_capability_to_name(CAPNG_EFFECTIVE);
>>>>>> + 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()
>>>>>> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
>>>>>> echo "madvise $madvise"
>>>>>> echo "posix_madvise $posix_madvise"
>>>>>> echo "uuid support $uuid"
>>>>>> +echo "libcap-ng support $cap"
>>>>>> echo "vhost-net support $vhost_net"
>>>>>> echo "Trace backend $trace_backend"
>>>>>> echo "Trace output file $trace_file-<pid>"
>>>>>> @@ -2846,6 +2877,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 db257d5..b1562eb 100644
>>>>>> --- a/qemu-bridge-helper.c
>>>>>> +++ b/qemu-bridge-helper.c
>>>>>> @@ -33,6 +33,10 @@
>>>>>>
>>>>>> #include "net/tap-linux.h"
>>>>>>
>>>>>> +#ifdef CONFIG_LIBCAP
>>>>>> +#include<cap-ng.h>
>>>>>> +#endif
>>>>>> +
>>>>>> #define MAX_ACLS (128)
>>>>>> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>>>>>>
>>>>>> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
>>>>>> return sendmsg(c,&msg, 0);
>>>>>> }
>>>>>>
>>>>>> +#ifdef CONFIG_LIBCAP
>>>>>> +static int drop_privileges(void)
>>>>>> +{
>>>>>> + /* clear all capabilities */
>>>>>> + capng_clear(CAPNG_SELECT_BOTH);
>>>>>> +
>>>>>> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
>>>>>> + CAP_NET_ADMIN)< 0) {
>>>>>> + return -1;
>>>>>> + }
>>>>>> +
>>>>>> + /* change to calling user's real uid and gid, retaining
>>>>>> supplemental
>>>>>> + * groups and CAP_NET_ADMIN */
>>>>>> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
>>>>>> + return -1;
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +#endif
>>>>>> +
>>>>>> int main(int argc, char **argv)
>>>>>> {
>>>>>> struct ifreq ifr;
>>>>>> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
>>>>>> int acl_count = 0;
>>>>>> int i, access_allowed, access_denied;
>>>>>>
>>>>>> + /* if we're run from an suid binary, immediately drop privileges
>>>>>> preserving
>>>>>> + * cap_net_admin -- exit immediately if libcap not configured */
>>>>>> + if (geteuid() == 0&& getuid() != geteuid()) {
>>>>>> +#ifdef CONFIG_LIBCAP
>>>>>> + if (drop_privileges() == -1) {
>>>>>> + fprintf(stderr, "failed to drop privileges\n");
>>>>>> + return 1;
>>>>>> + }
>>>>>> +#else
>>>>>> + fprintf(stderr, "failed to drop privileges\n");
>>>>>
>>>>> This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
>>>>> possible to use setfsuid() instead for Linux?
>>>>>
>>>>> Some fork+setuid helper could be used for other Unix and for the lame
>>>>> OSes without any file system DAC capabilities, a different syntax that
>>>>> does not rely on underlying FS may need to be introduced. Again, I
>>>>> don't know if the tool is even interesting for non-Linux.
>>>>>
>>>>
>>>> I just want to make sure that there is no chance that the helper is run
>>>> as
>>>> root beyond this point. Are you saying to seteuid(getuid) and
>>>> setfsuid(root)? I'm not sure that would drop the privileges enough.
>>>
>>> Without capabilities, we can't drop root privileges because bridge
>>> setup would fail otherwise, but we could use setfsuid(getuid()) and
>>> setfsgid(getgid()) during file access so permission checks work.
>>> Perhaps non-Linux could use seteuid() etc. instead.
>>>
>>
>> This would reduce file system access from effective UID/GID (root/root) to
>> real UID/GID (non-root/non-root). Other than file system access, the helper
>> would still run under root/root, right? I don't think we want that from a
>> security aspect.
>
> Right, it's not desirable, but isn't that the best we can do without
> libcap or FS capabilities?
>
I think the best we can do is not let it run in those cases. :) I'd
like see if others in the community have an opinion on this though.
--
Regards,
Corey
>> --
>> Regards,
>> Corey
>>
>>>>>> + return 1;
>>>>>> +#endif
>>>>>> + }
>>>>>> +
>>>>>> /* parse arguments */
>>>>>> if (argc< 3 || argc> 4) {
>>>>>> fprintf(stderr, "Usage: %s [--use-vnet] BRIDGE FD\n", argv[0]);
>>>>>> --
>>>>>> 1.7.3.4
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>
>>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-24 14:13 ` Corey Bryant
2011-10-24 17:10 ` Blue Swirl
@ 2011-10-24 19:19 ` Anthony Liguori
1 sibling, 0 replies; 20+ messages in thread
From: Anthony Liguori @ 2011-10-24 19:19 UTC (permalink / raw)
To: Corey Bryant; +Cc: Blue Swirl, rmarwah, qemu-devel
On 10/24/2011 09:13 AM, Corey Bryant wrote:
>
>
> On 10/23/2011 09:22 AM, Blue Swirl wrote:
>> On Fri, Oct 21, 2011 at 15:07, Corey Bryant<coreyb@linux.vnet.ibm.com> wrote:
>>> 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>
>>> Signed-off-by: Richa Marwaha<rmarwah@linux.vnet.ibm.com>
>>> Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
>>> ---
>>> configure | 34 ++++++++++++++++++++++++++++++++++
>>> qemu-bridge-helper.c | 39 +++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 73 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 6c8b659..fed66b0 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -128,6 +128,7 @@ vnc_thread="no"
>>> xen=""
>>> xen_ctrl_version=""
>>> linux_aio=""
>>> +cap=""
>>> attr=""
>>> xfs=""
>>>
>>> @@ -653,6 +654,10 @@ for opt do
>>> ;;
>>> --enable-kvm) kvm="yes"
>>> ;;
>>> + --disable-cap) cap="no"
>>> + ;;
>>> + --enable-cap) cap="yes"
>>> + ;;
>>> --disable-spice) spice="no"
>>> ;;
>>> --enable-spice) spice="yes"
>>> @@ -1032,6 +1037,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-cap disable libcap-ng support"
>>> +echo " --enable-cap enable libcap-ng support"
>>> echo " --disable-attr disables attr and xattr support"
>>> echo " --enable-attr enable attr and xattr support"
>>> echo " --disable-blobs disable installing provided firmware blobs"
>>> @@ -1638,6 +1645,29 @@ EOF
>>> fi
>>>
>>> ##########################################
>>> +# libcap-ng library probe
>>> +if test "$cap" != "no" ; then
>>> + cap_libs="-lcap-ng"
>>> + cat> $TMPC<< EOF
>>> +#include<cap-ng.h>
>>> +int main(void)
>>> +{
>>> + capng_capability_to_name(CAPNG_EFFECTIVE);
>>> + 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()
>>> @@ -2735,6 +2765,7 @@ echo "fdatasync $fdatasync"
>>> echo "madvise $madvise"
>>> echo "posix_madvise $posix_madvise"
>>> echo "uuid support $uuid"
>>> +echo "libcap-ng support $cap"
>>> echo "vhost-net support $vhost_net"
>>> echo "Trace backend $trace_backend"
>>> echo "Trace output file $trace_file-<pid>"
>>> @@ -2846,6 +2877,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 db257d5..b1562eb 100644
>>> --- a/qemu-bridge-helper.c
>>> +++ b/qemu-bridge-helper.c
>>> @@ -33,6 +33,10 @@
>>>
>>> #include "net/tap-linux.h"
>>>
>>> +#ifdef CONFIG_LIBCAP
>>> +#include<cap-ng.h>
>>> +#endif
>>> +
>>> #define MAX_ACLS (128)
>>> #define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf"
>>>
>>> @@ -185,6 +189,27 @@ static int send_fd(int c, int fd)
>>> return sendmsg(c,&msg, 0);
>>> }
>>>
>>> +#ifdef CONFIG_LIBCAP
>>> +static int drop_privileges(void)
>>> +{
>>> + /* clear all capabilities */
>>> + capng_clear(CAPNG_SELECT_BOTH);
>>> +
>>> + if (capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED,
>>> + CAP_NET_ADMIN)< 0) {
>>> + return -1;
>>> + }
>>> +
>>> + /* change to calling user's real uid and gid, retaining supplemental
>>> + * groups and CAP_NET_ADMIN */
>>> + if (capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING)) {
>>> + return -1;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> int main(int argc, char **argv)
>>> {
>>> struct ifreq ifr;
>>> @@ -198,6 +223,20 @@ int main(int argc, char **argv)
>>> int acl_count = 0;
>>> int i, access_allowed, access_denied;
>>>
>>> + /* if we're run from an suid binary, immediately drop privileges preserving
>>> + * cap_net_admin -- exit immediately if libcap not configured */
>>> + if (geteuid() == 0&& getuid() != geteuid()) {
>>> +#ifdef CONFIG_LIBCAP
>>> + if (drop_privileges() == -1) {
>>> + fprintf(stderr, "failed to drop privileges\n");
>>> + return 1;
>>> + }
>>> +#else
>>> + fprintf(stderr, "failed to drop privileges\n");
>>
>> This makes the tool useless without CONFIG_LIBCAP. Wouldn't it be
>> possible to use setfsuid() instead for Linux?
>>
>> Some fork+setuid helper could be used for other Unix and for the lame
>> OSes without any file system DAC capabilities, a different syntax that
>> does not rely on underlying FS may need to be introduced. Again, I
>> don't know if the tool is even interesting for non-Linux.
>>
>
> I just want to make sure that there is no chance that the helper is run as root
> beyond this point.
But the whole pointer of the helper is to run as root. It's a small trusted
piece of code.
Obviously, it's better to drop unneeded privileges when that's possible but in
the event that is isn't, we shouldn't bail out completely.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-24 19:13 ` Corey Bryant
@ 2011-10-24 19:21 ` Anthony Liguori
2011-10-24 20:20 ` Corey Bryant
0 siblings, 1 reply; 20+ messages in thread
From: Anthony Liguori @ 2011-10-24 19:21 UTC (permalink / raw)
To: Corey Bryant; +Cc: Blue Swirl, rmarwah, qemu-devel
On 10/24/2011 02:13 PM, Corey Bryant wrote:
>> Right, it's not desirable, but isn't that the best we can do without
>> libcap or FS capabilities?
>>
>
> I think the best we can do is not let it run in those cases. :) I'd like see if
> others in the community have an opinion on this though.
IMHO, it should work as an setuid binary maintaining root privileges. As long
as it's a small binary (which it is) and is easy to audit, it should be safe.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-24 19:21 ` Anthony Liguori
@ 2011-10-24 20:20 ` Corey Bryant
2011-10-24 22:15 ` Anthony Liguori
0 siblings, 1 reply; 20+ messages in thread
From: Corey Bryant @ 2011-10-24 20:20 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Blue Swirl, rmarwah, qemu-devel
On 10/24/2011 03:21 PM, Anthony Liguori wrote:
> On 10/24/2011 02:13 PM, Corey Bryant wrote:
>>> Right, it's not desirable, but isn't that the best we can do without
>>> libcap or FS capabilities?
>>>
>>
>> I think the best we can do is not let it run in those cases. :) I'd
>> like see if
>> others in the community have an opinion on this though.
>
> IMHO, it should work as an setuid binary maintaining root privileges. As
> long as it's a small binary (which it is) and is easy to audit, it
> should be safe.
>
> Regards,
>
> Anthony Liguori
>
>
Alright, I'll concede on this. I'll run a static analyzer on the code
and let it run as root if libcap-ng is not configured.
It would be nice to also cut an audit record, but I'm not seeing a
precedence for doing that in QEMU. Any thoughts?
--
Regards,
Corey
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID
2011-10-24 20:20 ` Corey Bryant
@ 2011-10-24 22:15 ` Anthony Liguori
0 siblings, 0 replies; 20+ messages in thread
From: Anthony Liguori @ 2011-10-24 22:15 UTC (permalink / raw)
To: Corey Bryant; +Cc: Blue Swirl, rmarwah, qemu-devel
On 10/24/2011 03:20 PM, Corey Bryant wrote:
> On 10/24/2011 03:21 PM, Anthony Liguori wrote:
>> On 10/24/2011 02:13 PM, Corey Bryant wrote:
>>>> Right, it's not desirable, but isn't that the best we can do without
>>>> libcap or FS capabilities?
>>>>
>>>
>>> I think the best we can do is not let it run in those cases. :) I'd
>>> like see if
>>> others in the community have an opinion on this though.
>>
>> IMHO, it should work as an setuid binary maintaining root privileges. As
>> long as it's a small binary (which it is) and is easy to audit, it
>> should be safe.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>
>
> Alright, I'll concede on this. I'll run a static analyzer on the code and let it
> run as root if libcap-ng is not configured.
>
> It would be nice to also cut an audit record, but I'm not seeing a precedence
> for doing that in QEMU. Any thoughts?
I'd be happy with just a hand full of Reviewed-by's from regular contributors.
Regards,
Anthony Liguori
>
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-10-24 22:15 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-21 15:07 [Qemu-devel] [PATCH v2 0/4] -net bridge: rootless bridge support for qemu Corey Bryant
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 1/4] Add basic version of bridge helper Corey Bryant
2011-10-23 12:56 ` Blue Swirl
2011-10-24 13:12 ` Corey Bryant
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 2/4] Add access control support to qemu " Corey Bryant
2011-10-23 13:10 ` Blue Swirl
2011-10-24 13:44 ` Corey Bryant
2011-10-24 16:58 ` Blue Swirl
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 3/4] Add cap reduction support to enable use as SUID Corey Bryant
2011-10-23 13:22 ` Blue Swirl
2011-10-24 14:13 ` Corey Bryant
2011-10-24 17:10 ` Blue Swirl
2011-10-24 18:38 ` Corey Bryant
2011-10-24 18:58 ` Blue Swirl
2011-10-24 19:13 ` Corey Bryant
2011-10-24 19:21 ` Anthony Liguori
2011-10-24 20:20 ` Corey Bryant
2011-10-24 22:15 ` Anthony Liguori
2011-10-24 19:19 ` Anthony Liguori
2011-10-21 15:07 ` [Qemu-devel] [PATCH v2 4/4] Add support for net bridge Corey Bryant
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).