[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]