Re: [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&gt;: > >> On Mon, 2012-07-09 at 10:22 -0400, rmarwah(a)linux.vnet.ibm.com wrote: >>> Quoting Jamie Strandboge <jamie(a)canonical.com&gt;: >>> >>>> On Tue, 2012-07-03 at 12:05 -0400, rmarwah(a)linux.vnet.ibm.com wrote: >>>>> Quoting Jamie Strandboge <jamie(a)canonical.com&gt;: >>>>> >>>>> > 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&gt; >>>>> >> >>>>> >> 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&gt; >>>>> >> Signed-off-by: Corey Bryant&lt;coreyb(a)linux.vnet.ibm.com&gt; >>>>> >> --- >>>>> >> 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