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

[rmarwah at linux.vnet.ibm.com]

Quoting Corey Bryant <coreyb at linux.vnet.ibm.com>:

> 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  
> ...

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