[libvirt] [Patch v2 3/3] apparmor: QEMU bridge helper policy updates

[Corey Bryant]

On 07/26/2012 10:30 AM, rmarwah at linux.vnet.ibm.com wrote:
>
> Quoting Jamie Strandboge <jamie at canonical.com>:
>
>> On Mon, 2012-07-09 at 10:22 -0400, rmarwah at linux.vnet.ibm.com wrote:
>>> Quoting Jamie Strandboge <jamie at canonical.com>:
>>>
>>> > On Tue, 2012-07-03 at 12:05 -0400, rmarwah at linux.vnet.ibm.com wrote:
>>> >> Quoting Jamie Strandboge <jamie at canonical.com>:
>>> >>
>>> >> > On Fri, 2012-06-29 at 14:08 -0400, rmarwah at linux.vnet.ibm.com
>>> wrote:
>>> >> >> From: Richa Marwaha <rmarwah at 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 at linux.vnet.ibm.com>
>>> >> >> Signed-off-by: Corey Bryant<coreyb at 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