[libvirt] [PATCH] apparmor: QEMU bridge helper policy updates

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. For more details on the helper, please refer to: http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03562.html Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com> --- examples/apparmor/libvirt-qemu | 22 +++++++++++++++++++++- 1 files changed, 21 insertions(+), 1 deletions(-) diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu index 10cdd36..ee9ba5d 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,23 @@ /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, + @{PROC}/*/status r, + + /usr/libexec/qemu-bridge-helper rmix, + } -- 1.7.7

On Mon, 2012-03-12 at 09:13 -0400, Corey Bryant wrote:
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. For more details on the helper, please refer to:
http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03562.html
Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
I've not used the helper personally, but the policy makes sense overall though. I do have a few questions:
+ capability setuid, + capability setgid,
I'm assuming these are needed because qemu-bridge-helper drops privileges?
+ capability setpcap,
Can you explain why this capability is needed by qemu-bridge-helper?
+ network inet stream,
I understood why net_admin was needed, but this one is less clear. Why does qemu-bridge-helper need this?
+ /etc/qemu/** r,
I'm not familiar with this directory. What does qemu-bridge-helper need from this directory?
+ @{PROC}/*/status r,
Is it possible to use this instead: owner @{PROC}/*/status r, Thanks! -- Jamie Strandboge | http://www.canonical.com

Thanks for your comments. One thing I should have pointed out before is that libvirt doesn't have support for the bridge helper yet. I hard coded the qemu options in the domain XML to test this. I'm not sure if that would prevent this patch from getting in or not. On 03/12/2012 02:36 PM, Jamie Strandboge wrote:
On Mon, 2012-03-12 at 09:13 -0400, Corey Bryant wrote:
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. For more details on the helper, please refer to:
http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03562.html
Signed-off-by: Corey Bryant<coreyb@linux.vnet.ibm.com>
I've not used the helper personally, but the policy makes sense overall though. I do have a few questions:
+ capability setuid, + capability setgid,
I'm assuming these are needed because qemu-bridge-helper drops privileges?
Yes, exactly.
+ capability setpcap,
Can you explain why this capability is needed by qemu-bridge-helper?
This is required to modify the bounding capability set. Here are the calls the helper uses to modify capabilities: capng_clear(CAPNG_SELECT_BOTH); capng_update(CAPNG_ADD, CAPNG_EFFECTIVE | CAPNG_PERMITTED, CAP_NET_ADMIN); capng_change_id(getuid(), getgid(), CAPNG_CLEAR_BOUNDING);
+ network inet stream,
I understood why net_admin was needed, but this one is less clear. Why does qemu-bridge-helper need this?
Good question. I'm going to test without this and see if it's necessary. I'm wondering if it's a subset of net_admin, and I may have added this before I added net_admin.
+ /etc/qemu/** r,
I'm not familiar with this directory. What does qemu-bridge-helper need from this directory?
ACL config files can be configured to allow/disallow bridge access to specific users. These files are read from /etc/qemu/.
+ @{PROC}/*/status r,
Is it possible to use this instead: owner @{PROC}/*/status r,
I would imagine this is ok to update with owner. I'll test it out and submit a v2 if there aren't any issues.
Thanks!
Thank you! -- Regards, Corey

On 03/12/2012 05:07 PM, Corey Bryant wrote: ...
+ network inet stream,
I understood why net_admin was needed, but this one is less clear. Why does qemu-bridge-helper need this?
Good question. I'm going to test without this and see if it's necessary. I'm wondering if it's a subset of net_admin, and I may have added this before I added net_admin.
I just submitted patch v2. The only change is addition of the owner keyword (see below). I tested without 'network inet stream' and got an AVC denial so I kept it in the policy. We open an AF_INET/SOCK_STREAM socket in the helper, which I believe this maps to. ...
+ @{PROC}/*/status r,
Is it possible to use this instead: owner @{PROC}/*/status r,
I would imagine this is ok to update with owner. I'll test it out and submit a v2 if there aren't any issues.
-- Regards, Corey
participants (2)
-
Corey Bryant
-
Jamie Strandboge