1:08 p.m.
From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
QEMU has a new feature which allows QEMU to execute under an unprivileged user ID and
still be able to
add a tap device to a Linux network bridge. Below is the link to the QEMU patches for the
bridge helper
feature:
http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03562.html
The existing libvirt tap network device support for adding a tap device to a bridge
(-netdev tap) works
only when connected to a libvirtd instance running as the privileged system account
'root'.
When connected to a libvirtd instance running as an unprivileged user (ie. using the
session URI) creation of
the tap device fails as follows:
error: Failed to start domain F14_64 error: Unable to create tap device vnet%d: Operation
not permitted
With this support, creating a tap device in the above scenario will be possible.
Additionally, hot attaching
a tap device to a bridge while running when connected to a libvirtd instance running as an
unprivileged user
will be possible.
Richa Marwaha (3):
Add -netdev bridge capabilities
Add -netdev bridge support
apparmor: QEMU bridge helper policy updates
examples/apparmor/libvirt-qemu | 21 +++++++++++++-
src/qemu/qemu_capabilities.c | 13 ++++++--
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 61 ++++++++++++++++++++++++++++-----------
src/qemu/qemu_command.h | 2 +
src/qemu/qemu_hotplug.c | 31 ++++++++++++++------
6 files changed, 97 insertions(+), 32 deletions(-)
1:08 p.m.
New subject: [libvirt] [Patch v2 1/3] Add -netdev bridge capabilities
From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
This patch adds the capability in libvirt to check if
-netdev bridge option is supported or not.
Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
---
v2
-This is a new patch that helps libvirt to check if -netdev bridge
option is supported in the qemu version that is being used to run
the guest. In v1 we didnot have the capability to check if qemu
version we are using supported -netdev bridge or not.
src/qemu/qemu_capabilities.c | 13 +++++++++----
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1e12a39..b2d7be2 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -166,6 +166,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"hda-micro", /* 95 */
"dump-guest-memory",
"nec-usb-xhci",
+ "bridge",
);
@@ -1005,7 +1006,7 @@ qemuCapsComputeCmdFlags(const char *help,
bool check_yajl ATTRIBUTE_UNUSED)
{
const char *p;
- const char *fsdev;
+ const char *fsdev, *netdev;
if (strstr(help, "-no-kqemu"))
qemuCapsSet(flags, QEMU_CAPS_KQEMU);
@@ -1118,13 +1119,17 @@ qemuCapsComputeCmdFlags(const char *help,
if (strstr(help, "-smbios type"))
qemuCapsSet(flags, QEMU_CAPS_SMBIOS_TYPE);
- if (strstr(help, "-netdev")) {
+ if ((netdev = strstr(help, "-netdev"))) {
/* Disable -netdev on 0.12 since although it exists,
* the corresponding netdev_add/remove monitor commands
* do not, and we need them to be able to do hotplug.
* But see below about RHEL build. */
- if (version >= 13000)
- qemuCapsSet(flags, QEMU_CAPS_NETDEV);
+ if (version >= 13000) {
+ if (strstr (netdev, "bridge"))
+ qemuCapsSet(flags, QEMU_CAPS_NETDEV_BRIDGE);
+ else
+ qemuCapsSet(flags, QEMU_CAPS_NETDEV);
+ }
}
if (strstr(help, "-sdl"))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 83c135b..458a73e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -133,6 +133,7 @@ enum qemuCapsFlags {
QEMU_CAPS_HDA_MICRO = 95, /* -device hda-micro */
QEMU_CAPS_DUMP_GUEST_MEMORY = 96, /* dump-guest-memory command */
QEMU_CAPS_NEC_USB_XHCI = 97, /* -device nec-usb-xhci */
+ QEMU_CAPS_NETDEV_BRIDGE = 98, /* bridge helper support */
QEMU_CAPS_LAST, /* this must always be the last item */
};
--
1.7.1
10:37 a.m.
New subject: [libvirt] [Patch v2 1/3] Add -netdev bridge capabilities
On 29.06.2012 20:08, rmarwah(a)linux.vnet.ibm.com wrote:
From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
This patch adds the capability in libvirt to check if
-netdev bridge option is supported or not.
Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
---
v2
-This is a new patch that helps libvirt to check if -netdev bridge
option is supported in the qemu version that is being used to run
the guest. In v1 we didnot have the capability to check if qemu
version we are using supported -netdev bridge or not.
src/qemu/qemu_capabilities.c | 13 +++++++++----
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1e12a39..b2d7be2 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -166,6 +166,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"hda-micro", /* 95 */
"dump-guest-memory",
"nec-usb-xhci",
+ "bridge",
);
@@ -1005,7 +1006,7 @@ qemuCapsComputeCmdFlags(const char *help,
bool check_yajl ATTRIBUTE_UNUSED)
{
const char *p;
- const char *fsdev;
+ const char *fsdev, *netdev;
if (strstr(help, "-no-kqemu"))
qemuCapsSet(flags, QEMU_CAPS_KQEMU);
@@ -1118,13 +1119,17 @@ qemuCapsComputeCmdFlags(const char *help,
if (strstr(help, "-smbios type"))
qemuCapsSet(flags, QEMU_CAPS_SMBIOS_TYPE);
- if (strstr(help, "-netdev")) {
+ if ((netdev = strstr(help, "-netdev"))) {
/* Disable -netdev on 0.12 since although it exists,
* the corresponding netdev_add/remove monitor commands
* do not, and we need them to be able to do hotplug.
* But see below about RHEL build. */
- if (version >= 13000)
- qemuCapsSet(flags, QEMU_CAPS_NETDEV);
+ if (version >= 13000) {
+ if (strstr (netdev, "bridge"))
+ qemuCapsSet(flags, QEMU_CAPS_NETDEV_BRIDGE);
+ else
+ qemuCapsSet(flags, QEMU_CAPS_NETDEV);
+ }
}
I think we want both capabilities set as QEMU_CAPS_NETDEV is used later
in the code. Moreover, QEMU_CAPS_NETDEV_BRIDGE is just refined version
of QEMU_CAPS_NETDEV. That is, if QEMU_CAPS_NETDEV_BRIDGE is set,
QEMU_CAPS_NETDEV must be supported as well.
if (strstr(help, "-sdl"))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 83c135b..458a73e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -133,6 +133,7 @@ enum qemuCapsFlags {
QEMU_CAPS_HDA_MICRO = 95, /* -device hda-micro */
QEMU_CAPS_DUMP_GUEST_MEMORY = 96, /* dump-guest-memory command */
QEMU_CAPS_NEC_USB_XHCI = 97, /* -device nec-usb-xhci */
+ QEMU_CAPS_NETDEV_BRIDGE = 98, /* bridge helper support */
QEMU_CAPS_LAST, /* this must always be the last item */
};
You forgot to add yourself to AUTHORS:
diff --git a/AUTHORS b/AUTHORS
index 0e179a9..f59550e 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -247,6 +247,7 @@ Patches have also been contributed by:
Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Thang Pham <thang.pham(a)us.ibm.com>
Eiichi Tsukata <eiichi.tsukata.xh(a)hitachi.com>
+ Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
[....send patches to get your name here....]
And don't forget to update tests/qemuhelptest.c as well.
Michal
1:08 p.m.
New subject: [libvirt] [Patch v2 2/3] Add -netdev bridge support
From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
This patch adds the support to run the QEMU network helper
under unprivileged user. It also adds the support for
attach-interface option in virsh to run under unprivileged
user.
Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
---
v2
- This patch attach-interface option is tested on
commit cd15303fd123146b0ba53e387d08ef22b707223
src/qemu/qemu_command.c | 61 +++++++++++++++++++++++++++++++++-------------
src/qemu/qemu_command.h | 2 +
src/qemu/qemu_hotplug.c | 31 ++++++++++++++++-------
3 files changed, 67 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6549f57..4eb8cd5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2851,6 +2851,8 @@ error:
char *
qemuBuildHostNetStr(virDomainNetDefPtr net,
+ struct qemud_driver *driver,
+ virBitmapPtr qemuCaps,
char type_sep,
int vlan,
const char *tapfd,
@@ -2859,6 +2861,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
bool is_tap = false;
virBuffer buf = VIR_BUFFER_INITIALIZER;
enum virDomainNetType netType = virDomainNetGetActualType(net);
+ const char *brname = NULL;
if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2868,8 +2871,21 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
}
switch (netType) {
- case VIR_DOMAIN_NET_TYPE_NETWORK:
+ /*
+ * If type='bridge', and we're running as privileged user
+ * or -netdev bridge is not supported then it will fall
+ * through, -net tap,fd
+ */
case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ if (!driver->privileged &&
+ qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE)) {
+ brname = virDomainNetGetActualBridgeName(net);
+ virBufferAsprintf(&buf, "bridge%cbr=%s", type_sep, brname);
+ type_sep = ',';
+ is_tap = true;
+ break;
+ }
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_DIRECT:
virBufferAsprintf(&buf, "tap%cfd=%s", type_sep, tapfd);
type_sep = ',';
@@ -4997,7 +5013,7 @@ qemuBuildCommandLine(virConnectPtr conn,
for (i = 0 ; i < def->nnets ; i++) {
virDomainNetDefPtr net = def->nets[i];
char *nic, *host;
- char tapfd_name[50];
+ char tapfd_name[50] = "";
char vhostfd_name[50] = "";
int vlan;
int bootindex = bootNet;
@@ -5034,17 +5050,26 @@ qemuBuildCommandLine(virConnectPtr conn,
if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net,
- qemuCaps);
- if (tapfd < 0)
- goto error;
-
- last_good_net = i;
- virCommandTransferFD(cmd, tapfd);
-
- if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
- tapfd) >= sizeof(tapfd_name))
- goto no_memory;
+ /*
+ * If type='bridge' then we attempt to allocate the tap fd here
only if
+ * running under a privilged user or -netdev bridge option is not
+ * supported.
+ */
+ if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ driver->privileged ||
+ (!qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) {
+ int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net,
+ qemuCaps);
+ if (tapfd < 0)
+ goto error;
+
+ last_good_net = i;
+ virCommandTransferFD(cmd, tapfd);
+
+ if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
+ tapfd) >= sizeof(tapfd_name))
+ goto no_memory;
+ }
} else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
int tapfd = qemuPhysIfaceConnect(def, driver, net,
qemuCaps, vmop);
@@ -5087,8 +5112,9 @@ qemuBuildCommandLine(virConnectPtr conn,
if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
virCommandAddArg(cmd, "-netdev");
- if (!(host = qemuBuildHostNetStr(net, ',', vlan,
- tapfd_name, vhostfd_name)))
+ if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps,
+ ',', vlan, tapfd_name,
+ vhostfd_name)))
goto error;
virCommandAddArg(cmd, host);
VIR_FREE(host);
@@ -5110,8 +5136,9 @@ qemuBuildCommandLine(virConnectPtr conn,
if (!(qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) {
virCommandAddArg(cmd, "-net");
- if (!(host = qemuBuildHostNetStr(net, ',', vlan,
- tapfd_name, vhostfd_name)))
+ if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps,
+ ',', vlan, tapfd_name,
+ vhostfd_name)))
goto error;
virCommandAddArg(cmd, host);
VIR_FREE(host);
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 1eafeb3..ebf7ad0 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -62,6 +62,8 @@ qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
/* With vlan == -1, use netdev syntax, else old hostnet */
char * qemuBuildHostNetStr(virDomainNetDefPtr net,
+ struct qemud_driver *driver,
+ virBitmapPtr qemuCaps,
char type_sep,
int vlan,
const char *tapfd,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c2fa75b..deca4cb 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -699,12 +699,21 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net,
- priv->qemuCaps)) < 0)
- goto cleanup;
- iface_connected = true;
- if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
- goto cleanup;
+ /*
+ * If type=bridge then we attempt to allocate the tap fd here only if
+ * running under a privilged user or -netdev bridge option is not
+ * supported.
+ */
+ if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ driver->privileged ||
+ (!qemuCapsGet (priv->qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) {
+ if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net,
+ priv->qemuCaps)) < 0)
+ goto cleanup;
+ iface_connected = true;
+ if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) <
0)
+ goto cleanup;
+ }
} else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
if ((tapfd = qemuPhysIfaceConnect(vm->def, driver, net,
priv->qemuCaps,
@@ -752,12 +761,14 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) &&
qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
- if (!(netstr = qemuBuildHostNetStr(net, ',',
- -1, tapfd_name, vhostfd_name)))
+ if (!(netstr = qemuBuildHostNetStr(net, driver, priv->qemuCaps,
+ ',', -1, tapfd_name,
+ vhostfd_name)))
goto cleanup;
} else {
- if (!(netstr = qemuBuildHostNetStr(net, ' ',
- vlan, tapfd_name, vhostfd_name)))
+ if (!(netstr = qemuBuildHostNetStr(net, driver, priv->qemuCaps,
+ ' ', vlan, tapfd_name,
+ vhostfd_name)))
goto cleanup;
}
--
1.7.1
10:37 a.m.
New subject: [libvirt] [Patch v2 2/3] Add -netdev bridge support
On 29.06.2012 20:08, rmarwah(a)linux.vnet.ibm.com wrote:
From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
This patch adds the support to run the QEMU network helper
under unprivileged user. It also adds the support for
attach-interface option in virsh to run under unprivileged
user.
Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
---
v2
- This patch attach-interface option is tested on
commit cd15303fd123146b0ba53e387d08ef22b707223
src/qemu/qemu_command.c | 61 +++++++++++++++++++++++++++++++++-------------
src/qemu/qemu_command.h | 2 +
src/qemu/qemu_hotplug.c | 31 ++++++++++++++++-------
3 files changed, 67 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6549f57..4eb8cd5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2851,6 +2851,8 @@ error:
char *
qemuBuildHostNetStr(virDomainNetDefPtr net,
+ struct qemud_driver *driver,
+ virBitmapPtr qemuCaps,
char type_sep,
int vlan,
const char *tapfd,
@@ -2859,6 +2861,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
bool is_tap = false;
virBuffer buf = VIR_BUFFER_INITIALIZER;
enum virDomainNetType netType = virDomainNetGetActualType(net);
+ const char *brname = NULL;
if (net->script && netType != VIR_DOMAIN_NET_TYPE_ETHERNET) {
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -2868,8 +2871,21 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
}
switch (netType) {
- case VIR_DOMAIN_NET_TYPE_NETWORK:
+ /*
+ * If type='bridge', and we're running as privileged user
+ * or -netdev bridge is not supported then it will fall
+ * through, -net tap,fd
+ */
case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ if (!driver->privileged &&
+ qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE)) {
+ brname = virDomainNetGetActualBridgeName(net);
+ virBufferAsprintf(&buf, "bridge%cbr=%s", type_sep, brname);
+ type_sep = ',';
+ is_tap = true;
+ break;
+ }
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_DIRECT:
virBufferAsprintf(&buf, "tap%cfd=%s", type_sep, tapfd);
type_sep = ',';
@@ -4997,7 +5013,7 @@ qemuBuildCommandLine(virConnectPtr conn,
for (i = 0 ; i < def->nnets ; i++) {
virDomainNetDefPtr net = def->nets[i];
char *nic, *host;
- char tapfd_name[50];
+ char tapfd_name[50] = "";
char vhostfd_name[50] = "";
int vlan;
int bootindex = bootNet;
@@ -5034,17 +5050,26 @@ qemuBuildCommandLine(virConnectPtr conn,
if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net,
- qemuCaps);
- if (tapfd < 0)
- goto error;
-
- last_good_net = i;
- virCommandTransferFD(cmd, tapfd);
-
- if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
- tapfd) >= sizeof(tapfd_name))
- goto no_memory;
+ /*
+ * If type='bridge' then we attempt to allocate the tap fd here
only if
+ * running under a privilged user or -netdev bridge option is not
+ * supported.
+ */
+ if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ driver->privileged ||
+ (!qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) {
+ int tapfd = qemuNetworkIfaceConnect(def, conn, driver, net,
+ qemuCaps);
+ if (tapfd < 0)
+ goto error;
+
+ last_good_net = i;
+ virCommandTransferFD(cmd, tapfd);
+
+ if (snprintf(tapfd_name, sizeof(tapfd_name), "%d",
+ tapfd) >= sizeof(tapfd_name))
+ goto no_memory;
+ }
} else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
int tapfd = qemuPhysIfaceConnect(def, driver, net,
qemuCaps, vmop);
@@ -5087,8 +5112,9 @@ qemuBuildCommandLine(virConnectPtr conn,
if (qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
virCommandAddArg(cmd, "-netdev");
- if (!(host = qemuBuildHostNetStr(net, ',', vlan,
- tapfd_name, vhostfd_name)))
+ if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps,
+ ',', vlan, tapfd_name,
+ vhostfd_name)))
goto error;
virCommandAddArg(cmd, host);
VIR_FREE(host);
@@ -5110,8 +5136,9 @@ qemuBuildCommandLine(virConnectPtr conn,
if (!(qemuCapsGet(qemuCaps, QEMU_CAPS_NETDEV) &&
qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) {
virCommandAddArg(cmd, "-net");
- if (!(host = qemuBuildHostNetStr(net, ',', vlan,
- tapfd_name, vhostfd_name)))
+ if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps,
+ ',', vlan, tapfd_name,
+ vhostfd_name)))
goto error;
virCommandAddArg(cmd, host);
VIR_FREE(host);
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 1eafeb3..ebf7ad0 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -62,6 +62,8 @@ qemuBuildChrDeviceStr (virDomainChrDefPtr serial,
/* With vlan == -1, use netdev syntax, else old hostnet */
char * qemuBuildHostNetStr(virDomainNetDefPtr net,
+ struct qemud_driver *driver,
+ virBitmapPtr qemuCaps,
char type_sep,
int vlan,
const char *tapfd,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c2fa75b..deca4cb 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -699,12 +699,21 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net,
- priv->qemuCaps)) < 0)
- goto cleanup;
- iface_connected = true;
- if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) < 0)
- goto cleanup;
+ /*
+ * If type=bridge then we attempt to allocate the tap fd here only if
+ * running under a privilged user or -netdev bridge option is not
+ * supported.
+ */
+ if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
+ driver->privileged ||
+ (!qemuCapsGet (priv->qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) {
+ if ((tapfd = qemuNetworkIfaceConnect(vm->def, conn, driver, net,
+ priv->qemuCaps)) < 0)
+ goto cleanup;
+ iface_connected = true;
+ if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, &vhostfd) <
0)
+ goto cleanup;
+ }
} else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
if ((tapfd = qemuPhysIfaceConnect(vm->def, driver, net,
priv->qemuCaps,
@@ -752,12 +761,14 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
if (qemuCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV) &&
qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
- if (!(netstr = qemuBuildHostNetStr(net, ',',
- -1, tapfd_name, vhostfd_name)))
+ if (!(netstr = qemuBuildHostNetStr(net, driver, priv->qemuCaps,
+ ',', -1, tapfd_name,
+ vhostfd_name)))
goto cleanup;
} else {
- if (!(netstr = qemuBuildHostNetStr(net, ' ',
- vlan, tapfd_name, vhostfd_name)))
+ if (!(netstr = qemuBuildHostNetStr(net, driver, priv->qemuCaps,
+ ' ', vlan, tapfd_name,
+ vhostfd_name)))
goto cleanup;
}
ACK
I am not going to review next apparmor patch which has already been
reviewed.
Michal
1:08 p.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
This patch provides AppArmor policy updates for the QEMU bridge helper.
The QEMU bridge helper is a SUID executable exec'd by QEMU that drops
capabilities to CAP_NET_ADMIN and adds a tap device to a network bridge.
Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
---
examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
1 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 10cdd36..766a334 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -1,4 +1,4 @@
-# Last Modified: Mon Apr 5 15:11:27 2010
+# Last Modified: Fri Mar 9 14:43:22 2012
#include <abstractions/base>
#include <abstractions/consoles>
@@ -108,3 +108,22 @@
/bin/dash rmix,
/bin/dd rmix,
/bin/cat rmix,
+
+ /usr/libexec/qemu-bridge-helper Cx,
+ # child profile for bridge helper process
+ profile /usr/libexec/qemu-bridge-helper {
+ #include <abstractions/base>
+
+ capability setuid,
+ capability setgid,
+ capability setpcap,
+ capability net_admin,
+
+ network inet stream,
+
+ /dev/net/tun rw,
+ /etc/qemu/** r,
+ owner @{PROC}/*/status r,
+
+ /usr/libexec/qemu-bridge-helper rmix,
+ }
--
1.7.1
7:16 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
This patch provides AppArmor policy updates for the QEMU bridge helper.
The QEMU bridge helper is a SUID executable exec'd by QEMU that drops
capabilities to CAP_NET_ADMIN and adds a tap device to a network bridge.
Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
---
examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
1 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 10cdd36..766a334 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -1,4 +1,4 @@
-# Last Modified: Mon Apr 5 15:11:27 2010
+# Last Modified: Fri Mar 9 14:43:22 2012
#include <abstractions/base>
#include <abstractions/consoles>
@@ -108,3 +108,22 @@
/bin/dash rmix,
/bin/dd rmix,
/bin/cat rmix,
+
+ /usr/libexec/qemu-bridge-helper Cx,
+ # child profile for bridge helper process
+ profile /usr/libexec/qemu-bridge-helper {
+ #include <abstractions/base>
+
+ capability setuid,
+ capability setgid,
+ capability setpcap,
+ capability net_admin,
+
+ network inet stream,
+
+ /dev/net/tun rw,
+ /etc/qemu/** r,
+ owner @{PROC}/*/status r,
+
+ /usr/libexec/qemu-bridge-helper rmix,
+ }
Looking at the child profile itself, this seems fine.
However, the Cx transition makes it so that all libvirt-managed qemu
processes are allowed to execute this setuid helper and I wonder if that
is too much? Ie, a guest running under libvirt's NAT wouldn't need
access to this helper at all. I wonder if it would be better to adjust
virt-aa-helper to add this transition and child profile instead (the
child profile could theoretically still be in apparmor/libvirt-qemu, but
this is a bit messy)? Can we determine from the domain XML the
circumstances when /usr/libexec/qemu-bridge-helper will be used? If so,
virt-aa-helper could add the access only then. As a side-benefit,
handling this in virt-aa-helper allows us at compile-time to adjust the
path to qemu-bridge-helper to use the configured path to the binary (ie,
some distros may not use /usr/libexec).
--
Jamie Strandboge | http://www.canonical.com
11:05 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
Quoting Jamie Strandboge <jamie(a)canonical.com>:
On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>
> This patch provides AppArmor policy updates for the QEMU bridge helper.
> The QEMU bridge helper is a SUID executable exec'd by QEMU that drops
> capabilities to CAP_NET_ADMIN and adds a tap device to a network bridge.
>
> Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
> Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
> ---
> examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
> 1 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
> index 10cdd36..766a334 100644
> --- a/examples/apparmor/libvirt-qemu
> +++ b/examples/apparmor/libvirt-qemu
> @@ -1,4 +1,4 @@
> -# Last Modified: Mon Apr 5 15:11:27 2010
> +# Last Modified: Fri Mar 9 14:43:22 2012
>
> #include <abstractions/base>
> #include <abstractions/consoles>
> @@ -108,3 +108,22 @@
> /bin/dash rmix,
> /bin/dd rmix,
> /bin/cat rmix,
> +
> + /usr/libexec/qemu-bridge-helper Cx,
> + # child profile for bridge helper process
> + profile /usr/libexec/qemu-bridge-helper {
> + #include <abstractions/base>
> +
> + capability setuid,
> + capability setgid,
> + capability setpcap,
> + capability net_admin,
> +
> + network inet stream,
> +
> + /dev/net/tun rw,
> + /etc/qemu/** r,
> + owner @{PROC}/*/status r,
> +
> + /usr/libexec/qemu-bridge-helper rmix,
> + }
Looking at the child profile itself, this seems fine.
However, the Cx transition makes it so that all libvirt-managed qemu
processes are allowed to execute this setuid helper and I wonder if that
is too much? Ie, a guest running under libvirt's NAT wouldn't need
access to this helper at all. I wonder if it would be better to adjust
virt-aa-helper to add this transition and child profile instead (the
child profile could theoretically still be in apparmor/libvirt-qemu, but
this is a bit messy)? Can we determine from the domain XML the
circumstances when /usr/libexec/qemu-bridge-helper will be used? If so,
virt-aa-helper could add the access only then. As a side-benefit,
handling this in virt-aa-helper allows us at compile-time to adjust the
path to qemu-bridge-helper to use the configured path to the binary (ie,
some distros may not use /usr/libexec).
Thanks a lot reviewing the apparmor patch. We cannot detemine from the
domain XML whether /usr/libexec/qemu-bridge-helper will be used or not
because we cannot determine whether we are running as privileged user
or not. Is there a way we can specify the configured path to the
binary in the current policy we have?
Regards
Richa Marwaha
--
Jamie Strandboge | http://www.canonical.com
3:09 p.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
Quoting Jamie Strandboge <jamie(a)canonical.com>:
> On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>>
>> This patch provides AppArmor policy updates for the QEMU bridge helper.
>> The QEMU bridge helper is a SUID executable exec'd by QEMU that drops
>> capabilities to CAP_NET_ADMIN and adds a tap device to a network bridge.
>>
>> Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>> Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
>> ---
>> examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
>> 1 files changed, 20 insertions(+), 1 deletions(-)
>>
>> diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
>> index 10cdd36..766a334 100644
>> --- a/examples/apparmor/libvirt-qemu
>> +++ b/examples/apparmor/libvirt-qemu
>> @@ -1,4 +1,4 @@
>> -# Last Modified: Mon Apr 5 15:11:27 2010
>> +# Last Modified: Fri Mar 9 14:43:22 2012
>>
>> #include <abstractions/base>
>> #include <abstractions/consoles>
>> @@ -108,3 +108,22 @@
>> /bin/dash rmix,
>> /bin/dd rmix,
>> /bin/cat rmix,
>> +
>> + /usr/libexec/qemu-bridge-helper Cx,
>> + # child profile for bridge helper process
>> + profile /usr/libexec/qemu-bridge-helper {
>> + #include <abstractions/base>
>> +
>> + capability setuid,
>> + capability setgid,
>> + capability setpcap,
>> + capability net_admin,
>> +
>> + network inet stream,
>> +
>> + /dev/net/tun rw,
>> + /etc/qemu/** r,
>> + owner @{PROC}/*/status r,
>> +
>> + /usr/libexec/qemu-bridge-helper rmix,
>> + }
>
> Looking at the child profile itself, this seems fine.
>
> However, the Cx transition makes it so that all libvirt-managed qemu
> processes are allowed to execute this setuid helper and I wonder if that
> is too much? Ie, a guest running under libvirt's NAT wouldn't need
> access to this helper at all. I wonder if it would be better to adjust
> virt-aa-helper to add this transition and child profile instead (the
> child profile could theoretically still be in apparmor/libvirt-qemu, but
> this is a bit messy)? Can we determine from the domain XML the
> circumstances when /usr/libexec/qemu-bridge-helper will be used? If so,
> virt-aa-helper could add the access only then. As a side-benefit,
> handling this in virt-aa-helper allows us at compile-time to adjust the
> path to qemu-bridge-helper to use the configured path to the binary (ie,
> some distros may not use /usr/libexec).
Thanks a lot reviewing the apparmor patch. We cannot detemine from the
domain XML whether /usr/libexec/qemu-bridge-helper will be used or not
because we cannot determine whether we are running as privileged user
or not.
Hmmm, that's too bad.
Is there a way we can specify the configured path to the
binary in the current policy we have?
Not in a convenient way, no. The policy is intended as example policy
anyway, and distributions are expected to modify this, so I don't think
this alone is a blocker.
--
Jamie Strandboge | http://www.canonical.com
9:22 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
Quoting Jamie Strandboge <jamie(a)canonical.com>:
On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>
> > On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> >> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
> >>
> >> This patch provides AppArmor policy updates for the QEMU bridge helper.
> >> The QEMU bridge helper is a SUID executable exec'd by QEMU that drops
> >> capabilities to CAP_NET_ADMIN and adds a tap device to a network bridge.
> >>
> >> Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
> >> Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
> >> ---
> >> examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
> >> 1 files changed, 20 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/examples/apparmor/libvirt-qemu
> b/examples/apparmor/libvirt-qemu
> >> index 10cdd36..766a334 100644
> >> --- a/examples/apparmor/libvirt-qemu
> >> +++ b/examples/apparmor/libvirt-qemu
> >> @@ -1,4 +1,4 @@
> >> -# Last Modified: Mon Apr 5 15:11:27 2010
> >> +# Last Modified: Fri Mar 9 14:43:22 2012
> >>
> >> #include <abstractions/base>
> >> #include <abstractions/consoles>
> >> @@ -108,3 +108,22 @@
> >> /bin/dash rmix,
> >> /bin/dd rmix,
> >> /bin/cat rmix,
> >> +
> >> + /usr/libexec/qemu-bridge-helper Cx,
> >> + # child profile for bridge helper process
> >> + profile /usr/libexec/qemu-bridge-helper {
> >> + #include <abstractions/base>
> >> +
> >> + capability setuid,
> >> + capability setgid,
> >> + capability setpcap,
> >> + capability net_admin,
> >> +
> >> + network inet stream,
> >> +
> >> + /dev/net/tun rw,
> >> + /etc/qemu/** r,
> >> + owner @{PROC}/*/status r,
> >> +
> >> + /usr/libexec/qemu-bridge-helper rmix,
> >> + }
> >
> > Looking at the child profile itself, this seems fine.
> >
> > However, the Cx transition makes it so that all libvirt-managed qemu
> > processes are allowed to execute this setuid helper and I wonder if that
> > is too much? Ie, a guest running under libvirt's NAT wouldn't need
> > access to this helper at all. I wonder if it would be better to adjust
> > virt-aa-helper to add this transition and child profile instead (the
> > child profile could theoretically still be in apparmor/libvirt-qemu, but
> > this is a bit messy)? Can we determine from the domain XML the
> > circumstances when /usr/libexec/qemu-bridge-helper will be used? If so,
> > virt-aa-helper could add the access only then. As a side-benefit,
> > handling this in virt-aa-helper allows us at compile-time to adjust the
> > path to qemu-bridge-helper to use the configured path to the binary (ie,
> > some distros may not use /usr/libexec).
>
> Thanks a lot reviewing the apparmor patch. We cannot detemine from the
> domain XML whether /usr/libexec/qemu-bridge-helper will be used or not
> because we cannot determine whether we are running as privileged user
> or not.
Hmmm, that's too bad.
> Is there a way we can specify the configured path to the
> binary in the current policy we have?
Not in a convenient way, no. The policy is intended as example policy
anyway, and distributions are expected to modify this, so I don't think
this alone is a blocker.
Right now the only way we can think of is that whenever in domain XML
we see interface=bridge
we set the policy for the qemu-bridge-helper even though we don't know
if the qemu-bridge-helper
is going to be used or not.
Thanks
Regards
Richa Marwaha
--
Jamie Strandboge | http://www.canonical.com
9:46 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On Mon, 2012-07-09 at 10:22 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
Quoting Jamie Strandboge <jamie(a)canonical.com>:
> On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>
>> > On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>> >> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>> >>
>> >> This patch provides AppArmor policy updates for the QEMU bridge
helper.
>> >> The QEMU bridge helper is a SUID executable exec'd by QEMU that
drops
>> >> capabilities to CAP_NET_ADMIN and adds a tap device to a network
bridge.
>> >>
>> >> Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>> >> Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
>> >> ---
>> >> examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
>> >> 1 files changed, 20 insertions(+), 1 deletions(-)
>> >>
>> >> diff --git a/examples/apparmor/libvirt-qemu
>> b/examples/apparmor/libvirt-qemu
>> >> index 10cdd36..766a334 100644
>> >> --- a/examples/apparmor/libvirt-qemu
>> >> +++ b/examples/apparmor/libvirt-qemu
>> >> @@ -1,4 +1,4 @@
>> >> -# Last Modified: Mon Apr 5 15:11:27 2010
>> >> +# Last Modified: Fri Mar 9 14:43:22 2012
>> >>
>> >> #include <abstractions/base>
>> >> #include <abstractions/consoles>
>> >> @@ -108,3 +108,22 @@
>> >> /bin/dash rmix,
>> >> /bin/dd rmix,
>> >> /bin/cat rmix,
>> >> +
>> >> + /usr/libexec/qemu-bridge-helper Cx,
>> >> + # child profile for bridge helper process
>> >> + profile /usr/libexec/qemu-bridge-helper {
>> >> + #include <abstractions/base>
>> >> +
>> >> + capability setuid,
>> >> + capability setgid,
>> >> + capability setpcap,
>> >> + capability net_admin,
>> >> +
>> >> + network inet stream,
>> >> +
>> >> + /dev/net/tun rw,
>> >> + /etc/qemu/** r,
>> >> + owner @{PROC}/*/status r,
>> >> +
>> >> + /usr/libexec/qemu-bridge-helper rmix,
>> >> + }
>> >
>> > Looking at the child profile itself, this seems fine.
>> >
>> > However, the Cx transition makes it so that all libvirt-managed qemu
>> > processes are allowed to execute this setuid helper and I wonder if that
>> > is too much? Ie, a guest running under libvirt's NAT wouldn't need
>> > access to this helper at all. I wonder if it would be better to adjust
>> > virt-aa-helper to add this transition and child profile instead (the
>> > child profile could theoretically still be in apparmor/libvirt-qemu, but
>> > this is a bit messy)? Can we determine from the domain XML the
>> > circumstances when /usr/libexec/qemu-bridge-helper will be used? If so,
>> > virt-aa-helper could add the access only then. As a side-benefit,
>> > handling this in virt-aa-helper allows us at compile-time to adjust the
>> > path to qemu-bridge-helper to use the configured path to the binary (ie,
>> > some distros may not use /usr/libexec).
>>
>> Thanks a lot reviewing the apparmor patch. We cannot detemine from the
>> domain XML whether /usr/libexec/qemu-bridge-helper will be used or not
>> because we cannot determine whether we are running as privileged user
>> or not.
> Hmmm, that's too bad.
>
>> Is there a way we can specify the configured path to the
>> binary in the current policy we have?
>
> Not in a convenient way, no. The policy is intended as example policy
> anyway, and distributions are expected to modify this, so I don't think
> this alone is a blocker.
Right now the only way we can think of is that whenever in domain XML
we see interface=bridge
we set the policy for the qemu-bridge-helper even though we don't know
if the qemu-bridge-helper
is going to be used or not.
Ah, well, that would at least be somewhat better and IMHO, worthwhile.
--
Jamie Strandboge | http://www.canonical.com
9:30 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
Quoting Jamie Strandboge <jamie(a)canonical.com>:
On Mon, 2012-07-09 at 10:22 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>
> > On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> >> Quoting Jamie Strandboge <jamie(a)canonical.com>:
> >>
> >> > On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> >> >> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
> >> >>
> >> >> This patch provides AppArmor policy updates for the QEMU
> bridge helper.
> >> >> The QEMU bridge helper is a SUID executable exec'd by QEMU that
drops
> >> >> capabilities to CAP_NET_ADMIN and adds a tap device to a
> network bridge.
> >> >>
> >> >> Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
> >> >> Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
> >> >> ---
> >> >> examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
> >> >> 1 files changed, 20 insertions(+), 1 deletions(-)
> >> >>
> >> >> diff --git a/examples/apparmor/libvirt-qemu
> >> b/examples/apparmor/libvirt-qemu
> >> >> index 10cdd36..766a334 100644
> >> >> --- a/examples/apparmor/libvirt-qemu
> >> >> +++ b/examples/apparmor/libvirt-qemu
> >> >> @@ -1,4 +1,4 @@
> >> >> -# Last Modified: Mon Apr 5 15:11:27 2010
> >> >> +# Last Modified: Fri Mar 9 14:43:22 2012
> >> >>
> >> >> #include <abstractions/base>
> >> >> #include <abstractions/consoles>
> >> >> @@ -108,3 +108,22 @@
> >> >> /bin/dash rmix,
> >> >> /bin/dd rmix,
> >> >> /bin/cat rmix,
> >> >> +
> >> >> + /usr/libexec/qemu-bridge-helper Cx,
> >> >> + # child profile for bridge helper process
> >> >> + profile /usr/libexec/qemu-bridge-helper {
> >> >> + #include <abstractions/base>
> >> >> +
> >> >> + capability setuid,
> >> >> + capability setgid,
> >> >> + capability setpcap,
> >> >> + capability net_admin,
> >> >> +
> >> >> + network inet stream,
> >> >> +
> >> >> + /dev/net/tun rw,
> >> >> + /etc/qemu/** r,
> >> >> + owner @{PROC}/*/status r,
> >> >> +
> >> >> + /usr/libexec/qemu-bridge-helper rmix,
> >> >> + }
> >> >
> >> > Looking at the child profile itself, this seems fine.
> >> >
> >> > However, the Cx transition makes it so that all libvirt-managed qemu
> >> > processes are allowed to execute this setuid helper and I
> wonder if that
> >> > is too much? Ie, a guest running under libvirt's NAT wouldn't
need
> >> > access to this helper at all. I wonder if it would be better to adjust
> >> > virt-aa-helper to add this transition and child profile instead (the
> >> > child profile could theoretically still be in
> apparmor/libvirt-qemu, but
> >> > this is a bit messy)? Can we determine from the domain XML the
> >> > circumstances when /usr/libexec/qemu-bridge-helper will be used? If
so,
> >> > virt-aa-helper could add the access only then. As a side-benefit,
> >> > handling this in virt-aa-helper allows us at compile-time to adjust
the
> >> > path to qemu-bridge-helper to use the configured path to the
> binary (ie,
> >> > some distros may not use /usr/libexec).
> >>
> >> Thanks a lot reviewing the apparmor patch. We cannot detemine from the
> >> domain XML whether /usr/libexec/qemu-bridge-helper will be used or not
> >> because we cannot determine whether we are running as privileged user
> >> or not.
> > Hmmm, that's too bad.
> >
> >> Is there a way we can specify the configured path to the
> >> binary in the current policy we have?
> >
> > Not in a convenient way, no. The policy is intended as example policy
> > anyway, and distributions are expected to modify this, so I don't think
> > this alone is a blocker.
>
> Right now the only way we can think of is that whenever in domain XML
> we see interface=bridge
> we set the policy for the qemu-bridge-helper even though we don't know
> if the qemu-bridge-helper
> is going to be used or not.
Ah, well, that would at least be somewhat better and IMHO, worthwhile.
Hi Jamie
I started testing out the policy generation with the approach that if
it checks inteface=bridge then only we generate
the qemu-bridge-helper policies. I found 2 issues while trying to do that
1) There are a lot of ways to start bridge helper and the way libvirt
is starting it is using -netdev bridge br=br0 and the executable
is started by the qemu and not libvirt. So the way I can think of
changing the path at compile time is to search for the executable in
/usr/. Not being a big fan of this approach for searching the executable.
2) The second issue that I face is the generation of the child profile
as virt-aa-helper creates a profile with -uuid and the virt-aa-helper
appends the libvirt-uuid.files. Is there a way in virt-aa-helper that
we could do the generation of child profile in libvirt-uuid profile?
Regards
Richa
--
Jamie Strandboge | http://www.canonical.com
10:54 p.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On 07/26/2012 10:30 AM, rmarwah(a)linux.vnet.ibm.com wrote:
Quoting Jamie Strandboge <jamie(a)canonical.com>:
> On Mon, 2012-07-09 at 10:22 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>
>> > On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>> >> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>> >>
>> >> > On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com
>> wrote:
>> >> >> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>> >> >>
>> >> >> This patch provides AppArmor policy updates for the QEMU
bridge
>> helper.
>> >> >> The QEMU bridge helper is a SUID executable exec'd by QEMU
that
>> drops
>> >> >> capabilities to CAP_NET_ADMIN and adds a tap device to a
>> network bridge.
>> >> >>
>> >> >> Signed-off-by: Richa Marwaha
<rmarwah(a)linux.vnet.ibm.com>
>> >> >> Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
>> >> >> ---
>> >> >> examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
>> >> >> 1 files changed, 20 insertions(+), 1 deletions(-)
>> >> >>
>> >> >> diff --git a/examples/apparmor/libvirt-qemu
>> >> b/examples/apparmor/libvirt-qemu
>> >> >> index 10cdd36..766a334 100644
>> >> >> --- a/examples/apparmor/libvirt-qemu
>> >> >> +++ b/examples/apparmor/libvirt-qemu
>> >> >> @@ -1,4 +1,4 @@
>> >> >> -# Last Modified: Mon Apr 5 15:11:27 2010
>> >> >> +# Last Modified: Fri Mar 9 14:43:22 2012
>> >> >>
>> >> >> #include <abstractions/base>
>> >> >> #include <abstractions/consoles>
>> >> >> @@ -108,3 +108,22 @@
>> >> >> /bin/dash rmix,
>> >> >> /bin/dd rmix,
>> >> >> /bin/cat rmix,
>> >> >> +
>> >> >> + /usr/libexec/qemu-bridge-helper Cx,
>> >> >> + # child profile for bridge helper process
>> >> >> + profile /usr/libexec/qemu-bridge-helper {
>> >> >> + #include <abstractions/base>
>> >> >> +
>> >> >> + capability setuid,
>> >> >> + capability setgid,
>> >> >> + capability setpcap,
>> >> >> + capability net_admin,
>> >> >> +
>> >> >> + network inet stream,
>> >> >> +
>> >> >> + /dev/net/tun rw,
>> >> >> + /etc/qemu/** r,
>> >> >> + owner @{PROC}/*/status r,
>> >> >> +
>> >> >> + /usr/libexec/qemu-bridge-helper rmix,
>> >> >> + }
>> >> >
>> >> > Looking at the child profile itself, this seems fine.
>> >> >
>> >> > However, the Cx transition makes it so that all libvirt-managed
>> qemu
>> >> > processes are allowed to execute this setuid helper and I wonder
>> if that
>> >> > is too much? Ie, a guest running under libvirt's NAT
wouldn't need
>> >> > access to this helper at all. I wonder if it would be better to
>> adjust
>> >> > virt-aa-helper to add this transition and child profile instead
>> (the
>> >> > child profile could theoretically still be in
>> apparmor/libvirt-qemu, but
>> >> > this is a bit messy)? Can we determine from the domain XML the
>> >> > circumstances when /usr/libexec/qemu-bridge-helper will be used?
>> If so,
>> >> > virt-aa-helper could add the access only then. As a side-benefit,
>> >> > handling this in virt-aa-helper allows us at compile-time to
>> adjust the
>> >> > path to qemu-bridge-helper to use the configured path to the
>> binary (ie,
>> >> > some distros may not use /usr/libexec).
>> >>
>> >> Thanks a lot reviewing the apparmor patch. We cannot detemine from
>> the
>> >> domain XML whether /usr/libexec/qemu-bridge-helper will be used or
>> not
>> >> because we cannot determine whether we are running as privileged user
>> >> or not.
>> > Hmmm, that's too bad.
>> >
>> >> Is there a way we can specify the configured path to the
>> >> binary in the current policy we have?
>> >
>> > Not in a convenient way, no. The policy is intended as example policy
>> > anyway, and distributions are expected to modify this, so I don't
>> think
>> > this alone is a blocker.
>>
>> Right now the only way we can think of is that whenever in domain XML
>> we see interface=bridge
>> we set the policy for the qemu-bridge-helper even though we don't know
>> if the qemu-bridge-helper
>> is going to be used or not.
>
> Ah, well, that would at least be somewhat better and IMHO, worthwhile.
Hi Jamie
I started testing out the policy generation with the approach that if it
checks inteface=bridge then only we generate
the qemu-bridge-helper policies. I found 2 issues while trying to do that
1) There are a lot of ways to start bridge helper and the way libvirt is
starting it is using -netdev bridge br=br0 and the executable
is started by the qemu and not libvirt. So the way I can think of
changing the path at compile time is to search for the executable in
/usr/. Not being a big fan of this approach for searching the executable.
Can you add a new tag to the domain XML that allows specification of the
helper executable path? For example, adding <helper>:
<interface type='bridge'>
<source bridge='br0'/>
<helper='/usr/libexec/qemu-bridge-helper' />
</interface>
That would translate to something like:
qemu-kvm -netdev
bridge,helper=/usr/libexec/qemu-bridge-helper,br=br0,id=xyz -device ...
--
Regards,
Corey
10:07 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
Quoting Corey Bryant <coreyb(a)linux.vnet.ibm.com>:
On 07/26/2012 10:30 AM, rmarwah(a)linux.vnet.ibm.com wrote:
>
> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>
>> On Mon, 2012-07-09 at 10:22 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>>> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>>
>>>> On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>>>>> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>>>>
>>>>> > On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com
>>> wrote:
>>>>> >> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>>>>> >>
>>>>> >> This patch provides AppArmor policy updates for the QEMU
bridge
>>> helper.
>>>>> >> The QEMU bridge helper is a SUID executable exec'd by
QEMU that
>>> drops
>>>>> >> capabilities to CAP_NET_ADMIN and adds a tap device to a
>>> network bridge.
>>>>> >>
>>>>> >> Signed-off-by: Richa Marwaha
<rmarwah(a)linux.vnet.ibm.com>
>>>>> >> Signed-off-by: Corey
Bryant<coreyb(a)linux.vnet.ibm.com>
>>>>> >> ---
>>>>> >> examples/apparmor/libvirt-qemu | 21
++++++++++++++++++++-
>>>>> >> 1 files changed, 20 insertions(+), 1 deletions(-)
>>>>> >>
>>>>> >> diff --git a/examples/apparmor/libvirt-qemu
>>>>> b/examples/apparmor/libvirt-qemu
>>>>> >> index 10cdd36..766a334 100644
>>>>> >> --- a/examples/apparmor/libvirt-qemu
>>>>> >> +++ b/examples/apparmor/libvirt-qemu
>>>>> >> @@ -1,4 +1,4 @@
>>>>> >> -# Last Modified: Mon Apr 5 15:11:27 2010
>>>>> >> +# Last Modified: Fri Mar 9 14:43:22 2012
>>>>> >>
>>>>> >> #include <abstractions/base>
>>>>> >> #include <abstractions/consoles>
>>>>> >> @@ -108,3 +108,22 @@
>>>>> >> /bin/dash rmix,
>>>>> >> /bin/dd rmix,
>>>>> >> /bin/cat rmix,
>>>>> >> +
>>>>> >> + /usr/libexec/qemu-bridge-helper Cx,
>>>>> >> + # child profile for bridge helper process
>>>>> >> + profile /usr/libexec/qemu-bridge-helper {
>>>>> >> + #include <abstractions/base>
>>>>> >> +
>>>>> >> + capability setuid,
>>>>> >> + capability setgid,
>>>>> >> + capability setpcap,
>>>>> >> + capability net_admin,
>>>>> >> +
>>>>> >> + network inet stream,
>>>>> >> +
>>>>> >> + /dev/net/tun rw,
>>>>> >> + /etc/qemu/** r,
>>>>> >> + owner @{PROC}/*/status r,
>>>>> >> +
>>>>> >> + /usr/libexec/qemu-bridge-helper rmix,
>>>>> >> + }
>>>>> >
>>>>> > Looking at the child profile itself, this seems fine.
>>>>> >
>>>>> > However, the Cx transition makes it so that all libvirt-managed
>>> qemu
>>>>> > processes are allowed to execute this setuid helper and I
wonder
>>> if that
>>>>> > is too much? Ie, a guest running under libvirt's NAT
wouldn't need
>>>>> > access to this helper at all. I wonder if it would be better to
>>> adjust
>>>>> > virt-aa-helper to add this transition and child profile instead
>>> (the
>>>>> > child profile could theoretically still be in
>>> apparmor/libvirt-qemu, but
>>>>> > this is a bit messy)? Can we determine from the domain XML the
>>>>> > circumstances when /usr/libexec/qemu-bridge-helper will be
used?
>>> If so,
>>>>> > virt-aa-helper could add the access only then. As a
side-benefit,
>>>>> > handling this in virt-aa-helper allows us at compile-time to
>>> adjust the
>>>>> > path to qemu-bridge-helper to use the configured path to the
>>> binary (ie,
>>>>> > some distros may not use /usr/libexec).
>>>>>
>>>>> Thanks a lot reviewing the apparmor patch. We cannot detemine from
>>> the
>>>>> domain XML whether /usr/libexec/qemu-bridge-helper will be used or
>>> not
>>>>> because we cannot determine whether we are running as privileged
user
>>>>> or not.
>>>> Hmmm, that's too bad.
>>>>
>>>>> Is there a way we can specify the configured path to the
>>>>> binary in the current policy we have?
>>>>
>>>> Not in a convenient way, no. The policy is intended as example policy
>>>> anyway, and distributions are expected to modify this, so I don't
>>> think
>>>> this alone is a blocker.
>>>
>>> Right now the only way we can think of is that whenever in domain XML
>>> we see interface=bridge
>>> we set the policy for the qemu-bridge-helper even though we don't know
>>> if the qemu-bridge-helper
>>> is going to be used or not.
>>
>> Ah, well, that would at least be somewhat better and IMHO, worthwhile.
>
> Hi Jamie
> I started testing out the policy generation with the approach that if it
> checks inteface=bridge then only we generate
> the qemu-bridge-helper policies. I found 2 issues while trying to do that
> 1) There are a lot of ways to start bridge helper and the way libvirt is
> starting it is using -netdev bridge br=br0 and the executable
> is started by the qemu and not libvirt. So the way I can think of
> changing the path at compile time is to search for the executable in
> /usr/. Not being a big fan of this approach for searching the executable.
Can you add a new tag to the domain XML that allows specification of
the helper executable path? For example, adding <helper>:
<interface type='bridge'>
<source bridge='br0'/>
<helper='/usr/libexec/qemu-bridge-helper' />
</interface>
That would translate to something like:
qemu-kvm -netdev
bridge,helper=/usr/libexec/qemu-bridge-helper,br=br0,id=xyz -device
...
Corey I can add the tag but the domain xml gets the information from
the installation of the guest (correct me if I am wrong in this
thought).
I did think of adding a new member in the bridge definition currently
it has 3 members brname, ipaddr and virtPortProfile. But I would still
have to do the search for the qemu-bridge-helper.
Regards
Richa
--
Regards,
Corey
3 p.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On 07/26/2012 11:54 PM, Corey Bryant wrote:
On 07/26/2012 10:30 AM, rmarwah(a)linux.vnet.ibm.com wrote:
> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>> On Mon, 2012-07-09 at 10:22 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>>> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>>
>>> > On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>>> >> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>> >>
>>> >> > On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com
>>> wrote:
>>> >> >> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>>> >> >>
>>> >> >> This patch provides AppArmor policy updates for the QEMU
bridge
>>> helper.
>>> >> >> The QEMU bridge helper is a SUID executable exec'd by
QEMU that
>>> drops
>>> >> >> capabilities to CAP_NET_ADMIN and adds a tap device to a
>>> network bridge.
>>> >> >>
>>> >> >> Signed-off-by: Richa Marwaha
<rmarwah(a)linux.vnet.ibm.com>
>>> >> >> Signed-off-by: Corey
Bryant<coreyb(a)linux.vnet.ibm.com>
>>> >> >> ---
>>> >> >> examples/apparmor/libvirt-qemu | 21
++++++++++++++++++++-
>>> >> >> 1 files changed, 20 insertions(+), 1 deletions(-)
>>> >> >>
>>> >> >> diff --git a/examples/apparmor/libvirt-qemu
>>> >> b/examples/apparmor/libvirt-qemu
>>> >> >> index 10cdd36..766a334 100644
>>> >> >> --- a/examples/apparmor/libvirt-qemu
>>> >> >> +++ b/examples/apparmor/libvirt-qemu
>>> >> >> @@ -1,4 +1,4 @@
>>> >> >> -# Last Modified: Mon Apr 5 15:11:27 2010
>>> >> >> +# Last Modified: Fri Mar 9 14:43:22 2012
>>> >> >>
>>> >> >> #include <abstractions/base>
>>> >> >> #include <abstractions/consoles>
>>> >> >> @@ -108,3 +108,22 @@
>>> >> >> /bin/dash rmix,
>>> >> >> /bin/dd rmix,
>>> >> >> /bin/cat rmix,
>>> >> >> +
>>> >> >> + /usr/libexec/qemu-bridge-helper Cx,
>>> >> >> + # child profile for bridge helper process
>>> >> >> + profile /usr/libexec/qemu-bridge-helper {
>>> >> >> + #include <abstractions/base>
>>> >> >> +
>>> >> >> + capability setuid,
>>> >> >> + capability setgid,
>>> >> >> + capability setpcap,
>>> >> >> + capability net_admin,
>>> >> >> +
>>> >> >> + network inet stream,
>>> >> >> +
>>> >> >> + /dev/net/tun rw,
>>> >> >> + /etc/qemu/** r,
>>> >> >> + owner @{PROC}/*/status r,
>>> >> >> +
>>> >> >> + /usr/libexec/qemu-bridge-helper rmix,
>>> >> >> + }
>>> >> >
>>> >> > Looking at the child profile itself, this seems fine.
>>> >> >
>>> >> > However, the Cx transition makes it so that all
libvirt-managed
>>> qemu
>>> >> > processes are allowed to execute this setuid helper and I
wonder
>>> if that
>>> >> > is too much? Ie, a guest running under libvirt's NAT
wouldn't
>>> need
>>> >> > access to this helper at all. I wonder if it would be better
to
>>> adjust
>>> >> > virt-aa-helper to add this transition and child profile
instead
>>> (the
>>> >> > child profile could theoretically still be in
>>> apparmor/libvirt-qemu, but
>>> >> > this is a bit messy)? Can we determine from the domain XML the
>>> >> > circumstances when /usr/libexec/qemu-bridge-helper will be
used?
Well, with the code in patch 1/3 and 2/3, if the actualinterface is
type='bridge', and libvirt is running unprivileged
(!driver->privileged), and the qemu capabilities has
QEMU_CAPS_NETDEV_BRIDGE, then the qemu-helper will definitely be used.
>>> If so,
>>> >> > virt-aa-helper could add the access only then. As a
side-benefit,
>>> >> > handling this in virt-aa-helper allows us at compile-time to
>>> adjust the
>>> >> > path to qemu-bridge-helper to use the configured path to the
>>> binary (ie,
>>> >> > some distros may not use /usr/libexec).
>>> >>
>>> >> Thanks a lot reviewing the apparmor patch. We cannot detemine from
>>> the
>>> >> domain XML whether /usr/libexec/qemu-bridge-helper will be used or
>>> not
>>> >> because we cannot determine whether we are running as privileged
>>> user
>>> >> or not.
>>> > Hmmm, that's too bad.
>>> >
>>> >> Is there a way we can specify the configured path to the
>>> >> binary in the current policy we have?
>>> >
>>> > Not in a convenient way, no. The policy is intended as example
>>> policy
>>> > anyway, and distributions are expected to modify this, so I don't
>>> think
>>> > this alone is a blocker.
>>>
>>> Right now the only way we can think of is that whenever in domain XML
>>> we see interface=bridge
>>> we set the policy for the qemu-bridge-helper even though we don't know
>>> if the qemu-bridge-helper
>>> is going to be used or not.
>>
>> Ah, well, that would at least be somewhat better and IMHO, worthwhile.
>
> Hi Jamie
> I started testing out the policy generation with the approach that if it
> checks inteface=bridge then only we generate
> the qemu-bridge-helper policies. I found 2 issues while trying to do
> that
> 1) There are a lot of ways to start bridge helper and the way libvirt is
> starting it is using -netdev bridge br=br0 and the executable
> is started by the qemu and not libvirt. So the way I can think of
> changing the path at compile time is to search for the executable in
> /usr/. Not being a big fan of this approach for searching the
> executable.
Can you add a new tag to the domain XML that allows specification of
the helper executable path? For example, adding <helper>:
<interface type='bridge'>
<source bridge='br0'/>
<helper='/usr/libexec/qemu-bridge-helper' />
</interface>
That's a bit opaque about its meaning, and I'm not sure it has a place
at this location in the XML - who (I mean "what person") is going to
determine the location of the bridge helper, and decide that it's even
needed? Especially since the idea here is to allow unprivileged users to
get full networking functionality, this config will be provided by an
unprivileged user - is there any risk in allowing an unprivileged user
to specify this binary's path? (other than the possibility they'll
misunderstand it and simply get it wrong, that is)
On the other hand, I think it will be useful to have *something* in the
XML that indicates this interface wants to use the qemu-helper method of
attaching to the bridge, for a reason that will come at the end of the
message:
To step back a bit and change the topic slightly (but it leads to the
"reason" I mention above :-):
My reservation about this patch series is that I want to be sure that it
is "future proof" in the eventuality that other methods of achieving the
same goal (proper networking support for guests started by
non-privileged users) become available.
In my opinion, having qemu execute a suid binary isn't the proper way to
solve the problem (however, it's currently the only way available), and
I'm hoping that eventually libvirt can be enhanced to achieve the
desired results in a different manner.
In particular, the problems I see with the qemu helper binary are:
1) A lot of time and trouble has been spent minimizing the capabilities
of qemu in order to reduce the amount of damage that could be done by a
compromised qemu process. The suid binary provides a way to "break out".
True it is a small and limited binary, but "low risk" does not mean "no
risk".
2) the text-file-based ACLs it uses will be cumbersome to manage (and
definitely outside the scope of libvirt). This really should be done by
policykit instead of home-grown ACL files.
3) the qemu helper binary can only handle creating a tap device and
connecting it to a bridge. It can't handle macvtap interfaces, libvirt's
virtual networks (unless the user just happens to know the name of the
bridge used for that interface), openvswitch bridges, nwfilter rules,
bandwidth limiting, 802.1Qb[gh]... You get the picture - there is a
*lot* of functionality that's off the table if the qemu helper binary is
used (unless it gains a lot of features and becomes hopelessly
intertwined with libvirt, and I don't think anyone wants that).
The way that I think the problem should be solved is this:
1) All of the network-related functionality in the system instance of
libvirt that is used by the qemu, lxc, etc. drivers internal to libvirt
(including the nwfilter subsystem, and everything in bridge_driver.c)
should be in a separate daemon from libvirtd, and made available via RPC
with a public API that uses policykit for authorization/authentication,
with separate selinux policies from libvirtd; maybe call it
"libvirt-networkd".
2) When the system instance of libvirtd is creating a domain, it should
call to libvirt-networkd via this API to do things like create a tap
device, connect it to a bridge, add nwfilter rules, etc.
3) likewise, when a session (unprivileged) instance of libvirt is
creating a domain, it also should call the same APIs, which (after
proper authentication/authorization via policykit) will connect it to
the privileged libvirt-networkd (or whatever its called) to perform the
operation.
The result will be that a guest created by an unprivileged user will
(dependent on policykit granting access) have access to *all* of the
network functionality that is available to a guest started by the system
libvirt instance, including libvirt's virtual networks, macvtap
interfaces, nwfilter rules, bandwidth limiting, etc. And managing who
gets access to what will be done in the same way as any other resource
gated by policykit. Additionally, libvirtd will no longer need many of
the capabilities/policies it currently needs to permit setting up the
networking (just as libvirt-networkd won't need all the
capabilities/policies required by libvirtd to handle other functions of
managing a guest's lifecycle).
So, how does this relate to my desire to see something in the XML
indicating that this interface should use the qemu helper? Well, I'm
thinking ahead to the day when the work I've outlined above is completed
- when it is included in a libvirt release, there will be people who
have guests with network configs that work thanks to the presence of the
qemu-helper support and an entry in the qemu-helper's ACL file. If they
upgrade their libvirt and their config stops working (due to their uid
not being authorized by policykit), they won't be happy. On the other
hand, once this more comprehensive solution is available, it should be
preferred by default. In order for that to be the case, we need to have
some sort of tag in the XML *now* that indicates "use the qemu-helper",
and otherwise pretend that it doesn't exist. That way when those people
upgrade, even though the new method of setting up networking (which may
not work without config changes) will be available, libvirt will
understand it should continue to use the "old" method for this
particular guest.
7:33 p.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
Laine Stump wrote:
The way that I think the problem should be solved is this:
1) All of the network-related functionality in the system instance of
libvirt that is used by the qemu, lxc, etc. drivers internal to libvirt
(including the nwfilter subsystem, and everything in bridge_driver.c)
should be in a separate daemon from libvirtd, and made available via RPC
with a public API that uses policykit for authorization/authentication,
with separate selinux policies from libvirtd; maybe call it
"libvirt-networkd".
2) When the system instance of libvirtd is creating a domain, it should
call to libvirt-networkd via this API to do things like create a tap
device, connect it to a bridge, add nwfilter rules, etc.
3) likewise, when a session (unprivileged) instance of libvirt is
creating a domain, it also should call the same APIs, which (after
proper authentication/authorization via policykit) will connect it to
the privileged libvirt-networkd (or whatever its called) to perform the
operation.
I wonder if the quantum project [1], which IIUC provides a lot of the
functionality you describe, could be used as "libvirtd-networkd".
Regards,
Jim
[1] https://github.com/openstack/quantum
4:17 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On Mon, Jul 30, 2012 at 06:33:54PM -0600, Jim Fehlig wrote:
Laine Stump wrote:
> The way that I think the problem should be solved is this:
>
> 1) All of the network-related functionality in the system instance of
> libvirt that is used by the qemu, lxc, etc. drivers internal to libvirt
> (including the nwfilter subsystem, and everything in bridge_driver.c)
> should be in a separate daemon from libvirtd, and made available via RPC
> with a public API that uses policykit for authorization/authentication,
> with separate selinux policies from libvirtd; maybe call it
> "libvirt-networkd".
>
> 2) When the system instance of libvirtd is creating a domain, it should
> call to libvirt-networkd via this API to do things like create a tap
> device, connect it to a bridge, add nwfilter rules, etc.
>
> 3) likewise, when a session (unprivileged) instance of libvirt is
> creating a domain, it also should call the same APIs, which (after
> proper authentication/authorization via policykit) will connect it to
> the privileged libvirt-networkd (or whatever its called) to perform the
> operation.
>
I wonder if the quantum project [1], which IIUC provides a lot of the
functionality you describe, could be used as "libvirtd-networkd".
Not as long its its a big mass of python. On the contrary, Quantum could
use libvirt's APIs.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
10:26 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On 07/27/2012 04:00 PM, Laine Stump wrote:
On 07/26/2012 11:54 PM, Corey Bryant wrote:
> On 07/26/2012 10:30 AM, rmarwah(a)linux.vnet.ibm.com wrote:
>> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>> On Mon, 2012-07-09 at 10:22 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>>>> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>>>
>>>>> On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
>>>>>> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>>>>>>
>>>>>>> On Fri, 2012-06-29 at 14:08 -0400,
rmarwah(a)linux.vnet.ibm.com
>>>> wrote:
>>>>>>>> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
>>>>>>>>
>>>>>>>> This patch provides AppArmor policy updates for the QEMU
bridge
>>>> helper.
>>>>>>>> The QEMU bridge helper is a SUID executable exec'd by
QEMU that
>>>> drops
>>>>>>>> capabilities to CAP_NET_ADMIN and adds a tap device to a
>>>> network bridge.
>>>>>>>>
>>>>>>>> Signed-off-by: Richa Marwaha
<rmarwah(a)linux.vnet.ibm.com>
>>>>>>>> Signed-off-by: Corey
Bryant<coreyb(a)linux.vnet.ibm.com>
>>>>>>>> ---
>>>>>>>> examples/apparmor/libvirt-qemu | 21
++++++++++++++++++++-
>>>>>>>> 1 files changed, 20 insertions(+), 1 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/examples/apparmor/libvirt-qemu
>>>>>> b/examples/apparmor/libvirt-qemu
>>>>>>>> index 10cdd36..766a334 100644
>>>>>>>> --- a/examples/apparmor/libvirt-qemu
>>>>>>>> +++ b/examples/apparmor/libvirt-qemu
>>>>>>>> @@ -1,4 +1,4 @@
>>>>>>>> -# Last Modified: Mon Apr 5 15:11:27 2010
>>>>>>>> +# Last Modified: Fri Mar 9 14:43:22 2012
>>>>>>>>
>>>>>>>> #include <abstractions/base>
>>>>>>>> #include <abstractions/consoles>
>>>>>>>> @@ -108,3 +108,22 @@
>>>>>>>> /bin/dash rmix,
>>>>>>>> /bin/dd rmix,
>>>>>>>> /bin/cat rmix,
>>>>>>>> +
>>>>>>>> + /usr/libexec/qemu-bridge-helper Cx,
>>>>>>>> + # child profile for bridge helper process
>>>>>>>> + profile /usr/libexec/qemu-bridge-helper {
>>>>>>>> + #include <abstractions/base>
>>>>>>>> +
>>>>>>>> + capability setuid,
>>>>>>>> + capability setgid,
>>>>>>>> + capability setpcap,
>>>>>>>> + capability net_admin,
>>>>>>>> +
>>>>>>>> + network inet stream,
>>>>>>>> +
>>>>>>>> + /dev/net/tun rw,
>>>>>>>> + /etc/qemu/** r,
>>>>>>>> + owner @{PROC}/*/status r,
>>>>>>>> +
>>>>>>>> + /usr/libexec/qemu-bridge-helper rmix,
>>>>>>>> + }
>>>>>>>
>>>>>>> Looking at the child profile itself, this seems fine.
>>>>>>>
>>>>>>> However, the Cx transition makes it so that all
libvirt-managed
>>>> qemu
>>>>>>> processes are allowed to execute this setuid helper and I
wonder
>>>> if that
>>>>>>> is too much? Ie, a guest running under libvirt's NAT
wouldn't
>>>> need
>>>>>>> access to this helper at all. I wonder if it would be better
to
>>>> adjust
>>>>>>> virt-aa-helper to add this transition and child profile
instead
>>>> (the
>>>>>>> child profile could theoretically still be in
>>>> apparmor/libvirt-qemu, but
>>>>>>> this is a bit messy)? Can we determine from the domain XML
the
>>>>>>> circumstances when /usr/libexec/qemu-bridge-helper will be
used?
Well, with the code in patch 1/3 and 2/3, if the actualinterface is
type='bridge', and libvirt is running unprivileged
(!driver->privileged), and the qemu capabilities has
QEMU_CAPS_NETDEV_BRIDGE, then the qemu-helper will definitely be used.
>>>> If so,
>>>>>>> virt-aa-helper could add the access only then. As a
side-benefit,
>>>>>>> handling this in virt-aa-helper allows us at compile-time to
>>>> adjust the
>>>>>>> path to qemu-bridge-helper to use the configured path to the
>>>> binary (ie,
>>>>>>> some distros may not use /usr/libexec).
>>>>>>
>>>>>> Thanks a lot reviewing the apparmor patch. We cannot detemine
from
>>>> the
>>>>>> domain XML whether /usr/libexec/qemu-bridge-helper will be used
or
>>>> not
>>>>>> because we cannot determine whether we are running as privileged
>>>> user
>>>>>> or not.
>>>>> Hmmm, that's too bad.
>>>>>
>>>>>> Is there a way we can specify the configured path to the
>>>>>> binary in the current policy we have?
>>>>>
>>>>> Not in a convenient way, no. The policy is intended as example
>>>> policy
>>>>> anyway, and distributions are expected to modify this, so I
don't
>>>> think
>>>>> this alone is a blocker.
>>>>
>>>> Right now the only way we can think of is that whenever in domain XML
>>>> we see interface=bridge
>>>> we set the policy for the qemu-bridge-helper even though we don't
know
>>>> if the qemu-bridge-helper
>>>> is going to be used or not.
>>>
>>> Ah, well, that would at least be somewhat better and IMHO, worthwhile.
>>
>> Hi Jamie
>> I started testing out the policy generation with the approach that if it
>> checks inteface=bridge then only we generate
>> the qemu-bridge-helper policies. I found 2 issues while trying to do
>> that
>> 1) There are a lot of ways to start bridge helper and the way libvirt is
>> starting it is using -netdev bridge br=br0 and the executable
>> is started by the qemu and not libvirt. So the way I can think of
>> changing the path at compile time is to search for the executable in
>> /usr/. Not being a big fan of this approach for searching the
>> executable.
>
> Can you add a new tag to the domain XML that allows specification of
> the helper executable path? For example, adding <helper>:
>
> <interface type='bridge'>
> <source bridge='br0'/>
> <helper='/usr/libexec/qemu-bridge-helper' />
> </interface>
That's a bit opaque about its meaning, and I'm not sure it has a place
at this location in the XML - who (I mean "what person") is going to
determine the location of the bridge helper, and decide that it's even
needed? Especially since the idea here is to allow unprivileged users to
get full networking functionality, this config will be provided by an
unprivileged user - is there any risk in allowing an unprivileged user
to specify this binary's path? (other than the possibility they'll
misunderstand it and simply get it wrong, that is)
On the other hand, I think it will be useful to have *something* in the
XML that indicates this interface wants to use the qemu-helper method of
attaching to the bridge, for a reason that will come at the end of the
message:
To step back a bit and change the topic slightly (but it leads to the
"reason" I mention above :-):
My reservation about this patch series is that I want to be sure that it
is "future proof" in the eventuality that other methods of achieving the
same goal (proper networking support for guests started by
non-privileged users) become available.
In my opinion, having qemu execute a suid binary isn't the proper way to
solve the problem (however, it's currently the only way available), and
I'm hoping that eventually libvirt can be enhanced to achieve the
desired results in a different manner.
In particular, the problems I see with the qemu helper binary are:
1) A lot of time and trouble has been spent minimizing the capabilities
of qemu in order to reduce the amount of damage that could be done by a
compromised qemu process. The suid binary provides a way to "break out".
True it is a small and limited binary, but "low risk" does not mean "no
risk".
2) the text-file-based ACLs it uses will be cumbersome to manage (and
definitely outside the scope of libvirt). This really should be done by
policykit instead of home-grown ACL files.
3) the qemu helper binary can only handle creating a tap device and
connecting it to a bridge. It can't handle macvtap interfaces, libvirt's
virtual networks (unless the user just happens to know the name of the
bridge used for that interface), openvswitch bridges, nwfilter rules,
bandwidth limiting, 802.1Qb[gh]... You get the picture - there is a
*lot* of functionality that's off the table if the qemu helper binary is
used (unless it gains a lot of features and becomes hopelessly
intertwined with libvirt, and I don't think anyone wants that).
The way that I think the problem should be solved is this:
1) All of the network-related functionality in the system instance of
libvirt that is used by the qemu, lxc, etc. drivers internal to libvirt
(including the nwfilter subsystem, and everything in bridge_driver.c)
should be in a separate daemon from libvirtd, and made available via RPC
with a public API that uses policykit for authorization/authentication,
with separate selinux policies from libvirtd; maybe call it
"libvirt-networkd".
2) When the system instance of libvirtd is creating a domain, it should
call to libvirt-networkd via this API to do things like create a tap
device, connect it to a bridge, add nwfilter rules, etc.
3) likewise, when a session (unprivileged) instance of libvirt is
creating a domain, it also should call the same APIs, which (after
proper authentication/authorization via policykit) will connect it to
the privileged libvirt-networkd (or whatever its called) to perform the
operation.
The result will be that a guest created by an unprivileged user will
(dependent on policykit granting access) have access to *all* of the
network functionality that is available to a guest started by the system
libvirt instance, including libvirt's virtual networks, macvtap
interfaces, nwfilter rules, bandwidth limiting, etc. And managing who
gets access to what will be done in the same way as any other resource
gated by policykit. Additionally, libvirtd will no longer need many of
the capabilities/policies it currently needs to permit setting up the
networking (just as libvirt-networkd won't need all the
capabilities/policies required by libvirtd to handle other functions of
managing a guest's lifecycle).
So, how does this relate to my desire to see something in the XML
indicating that this interface should use the qemu helper? Well, I'm
thinking ahead to the day when the work I've outlined above is completed
- when it is included in a libvirt release, there will be people who
have guests with network configs that work thanks to the presence of the
qemu-helper support and an entry in the qemu-helper's ACL file. If they
upgrade their libvirt and their config stops working (due to their uid
not being authorized by policykit), they won't be happy. On the other
hand, once this more comprehensive solution is available, it should be
preferred by default. In order for that to be the case, we need to have
some sort of tag in the XML *now* that indicates "use the qemu-helper",
and otherwise pretend that it doesn't exist. That way when those people
upgrade, even though the new method of setting up networking (which may
not work without config changes) will be available, libvirt will
understand it should continue to use the "old" method for this
particular guest.
That sounds like a good future direction that you've outlined.
At this point I wonder if we might be able to get away with no XML
modifications since we know that we only want to attempt to run the
helper when libvirt is running unprivileged.
--
Regards,
Corey
11:06 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On Tue, Jul 31, 2012 at 11:26:05AM -0400, Corey Bryant wrote:
At this point I wonder if we might be able to get away with no XML
modifications since we know that we only want to attempt to run the
helper when libvirt is running unprivileged.
I certainly don't expect there to be any changes to the XML for the
purposes of supporting this QEMU bridge helper proxy. It should be
automatically used when type=bridge or type=network on the
<interface>, as a private implementation detail of libvirt not
exposed to applications in the XML. When libvirt changes to
separate its nework setup code out of libvirtd, then we will be able
to transparently stop using the proxy helper.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:57 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
On 07/31/2012 12:06 PM, Daniel P. Berrange wrote:
On Tue, Jul 31, 2012 at 11:26:05AM -0400, Corey Bryant wrote:
>
> At this point I wonder if we might be able to get away with no XML
> modifications since we know that we only want to attempt to run the
> helper when libvirt is running unprivileged.
I certainly don't expect there to be any changes to the XML for the
purposes of supporting this QEMU bridge helper proxy. It should be
automatically used when type=bridge or type=network on the
<interface>, as a private implementation detail of libvirt not
exposed to applications in the XML. When libvirt changes to
separate its nework setup code out of libvirtd, then we will be able
to transparently stop using the proxy helper.
I would greatly prefer that as well, as long as nobody is bothered by
the potential upgrade problem I outlined in my previous mail - if there
is config required outside libvirt to enable particular users to use the
qemu helper, and if there is also extra config required to enable the
capability for users when libvirt no longer requires the qemu helper,
there could be cases where a working config would cease to work (until
extra measures were taken) after an upgrade.
If that situation is acceptable, then I also agree that no XML
modifications is much better.
10:11 a.m.
New subject: [libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates
Quoting Jamie Strandboge <jamie(a)canonical.com>:
On Mon, 2012-07-09 at 10:22 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> Quoting Jamie Strandboge <jamie(a)canonical.com>:
>
> > On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> >> Quoting Jamie Strandboge <jamie(a)canonical.com>:
> >>
> >> > On Fri, 2012-06-29 at 14:08 -0400, rmarwah(a)linux.vnet.ibm.com wrote:
> >> >> From: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
> >> >>
> >> >> This patch provides AppArmor policy updates for the QEMU
> bridge helper.
> >> >> The QEMU bridge helper is a SUID executable exec'd by QEMU that
drops
> >> >> capabilities to CAP_NET_ADMIN and adds a tap device to a
> network bridge.
> >> >>
> >> >> Signed-off-by: Richa Marwaha <rmarwah(a)linux.vnet.ibm.com>
> >> >> Signed-off-by: Corey Bryant<coreyb(a)linux.vnet.ibm.com>
> >> >> ---
> >> >> examples/apparmor/libvirt-qemu | 21 ++++++++++++++++++++-
> >> >> 1 files changed, 20 insertions(+), 1 deletions(-)
> >> >>
> >> >> diff --git a/examples/apparmor/libvirt-qemu
> >> b/examples/apparmor/libvirt-qemu
> >> >> index 10cdd36..766a334 100644
> >> >> --- a/examples/apparmor/libvirt-qemu
> >> >> +++ b/examples/apparmor/libvirt-qemu
> >> >> @@ -1,4 +1,4 @@
> >> >> -# Last Modified: Mon Apr 5 15:11:27 2010
> >> >> +# Last Modified: Fri Mar 9 14:43:22 2012
> >> >>
> >> >> #include <abstractions/base>
> >> >> #include <abstractions/consoles>
> >> >> @@ -108,3 +108,22 @@
> >> >> /bin/dash rmix,
> >> >> /bin/dd rmix,
> >> >> /bin/cat rmix,
> >> >> +
> >> >> + /usr/libexec/qemu-bridge-helper Cx,
> >> >> + # child profile for bridge helper process
> >> >> + profile /usr/libexec/qemu-bridge-helper {
> >> >> + #include <abstractions/base>
> >> >> +
> >> >> + capability setuid,
> >> >> + capability setgid,
> >> >> + capability setpcap,
> >> >> + capability net_admin,
> >> >> +
> >> >> + network inet stream,
> >> >> +
> >> >> + /dev/net/tun rw,
> >> >> + /etc/qemu/** r,
> >> >> + owner @{PROC}/*/status r,
> >> >> +
> >> >> + /usr/libexec/qemu-bridge-helper rmix,
> >> >> + }
> >> >
> >> > Looking at the child profile itself, this seems fine.
> >> >
> >> > However, the Cx transition makes it so that all libvirt-managed qemu
> >> > processes are allowed to execute this setuid helper and I
> wonder if that
> >> > is too much? Ie, a guest running under libvirt's NAT wouldn't
need
> >> > access to this helper at all. I wonder if it would be better to adjust
> >> > virt-aa-helper to add this transition and child profile instead (the
> >> > child profile could theoretically still be in
> apparmor/libvirt-qemu, but
> >> > this is a bit messy)? Can we determine from the domain XML the
> >> > circumstances when /usr/libexec/qemu-bridge-helper will be used? If
so,
> >> > virt-aa-helper could add the access only then. As a side-benefit,
> >> > handling this in virt-aa-helper allows us at compile-time to adjust
the
> >> > path to qemu-bridge-helper to use the configured path to the
> binary (ie,
> >> > some distros may not use /usr/libexec).
> >>
> >> Thanks a lot reviewing the apparmor patch. We cannot detemine from the
> >> domain XML whether /usr/libexec/qemu-bridge-helper will be used or not
> >> because we cannot determine whether we are running as privileged user
> >> or not.
> > Hmmm, that's too bad.
> >
> >> Is there a way we can specify the configured path to the
> >> binary in the current policy we have?
> >
> > Not in a convenient way, no. The policy is intended as example policy
> > anyway, and distributions are expected to modify this, so I don't think
> > this alone is a blocker.
>
> Right now the only way we can think of is that whenever in domain XML
> we see interface=bridge
> we set the policy for the qemu-bridge-helper even though we don't know
> if the qemu-bridge-helper
> is going to be used or not.
Ah, well, that would at least be somewhat better and IMHO, worthwhile.
Hi Jamie
As we discussed earlier that we cannot determine from the domain XML
whether the user is
privileged or not to have the generation of the apparmor policy for
qemu-bridge-helper. One
main purpose of doing this was to get the adjustment of the path of
the qemu-bridge-helper as
some distros don't have /usr/libexec. So I tried to have the
generation of the policies be done
when we see interface = bridge in the domain XML. In the process of
doing so it comes out that
there is not a good way to determine the location of the
qemu-bridge-helper other than the fact that
it is hard-coded in the virt-aa-helper code. Also could not find a
good way to have the generation of the child profile for
qemu-bridge-helper apparmor policy.
Since we have to hard-code the path of the executable I would suggest
that we keep the apparmor policies for
qemu-bridge-helper in libvirt-qemu only.
Kindly let me know your thoughts.
Regards
Richa
--
Jamie Strandboge | http://www.canonical.com
4500
days inactive
4532
days old
20 comments
7 participants
participants (7)
-
Corey Bryant -
Daniel P. Berrange -
Jamie Strandboge -
Jim Fehlig -
Laine Stump -
Michal Privoznik -
rmarwah＠linux.vnet.ibm.com