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

Corey Bryant coreyb at linux.vnet.ibm.com
Tue Jul 31 15:26:05 UTC 2012



On 07/27/2012 04:00 PM, Laine Stump wrote:
> On 07/26/2012 11:54 PM, Corey Bryant wrote:
>> 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?
>
> Well, with the code in patch 1/3 and 2/3, if the actualinterface is
> type='bridge', and libvirt is running unprivileged
> (!driver->privileged), and the qemu capabilities has
> QEMU_CAPS_NETDEV_BRIDGE, then the qemu-helper will definitely 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's a bit opaque about its meaning, and I'm not sure it has a place
> at this location in the XML - who (I mean "what person") is going to
> determine the location of the bridge helper, and decide that it's even
> needed? Especially since the idea here is to allow unprivileged users to
> get full networking functionality, this config will be provided by an
> unprivileged user - is there any risk in allowing an unprivileged user
> to specify this binary's path? (other than the possibility they'll
> misunderstand it and simply get it wrong, that is)
>
> On the other hand, I think it will be useful to have *something* in the
> XML that indicates this interface wants to use the qemu-helper method of
> attaching to the bridge, for a reason that will come at the end of the
> message:
>
> To step back a bit and change the topic slightly (but it leads to the
> "reason" I mention above :-):
>
> My reservation about this patch series is that I want to be sure that it
> is "future proof" in the eventuality that other methods of achieving the
> same goal (proper networking support for guests started by
> non-privileged users) become available.
>
> In my opinion, having qemu execute a suid binary isn't the proper way to
> solve the problem (however, it's currently the only way available), and
> I'm hoping that eventually libvirt can be enhanced to achieve the
> desired results in a different manner.
>
> In particular, the problems I see with the qemu helper binary are:
>
> 1) A lot of time and trouble has been spent minimizing the capabilities
> of qemu in order to reduce the amount of damage that could be done by a
> compromised qemu process. The suid binary provides a way to "break out".
> True it is a small and limited binary, but "low risk" does not mean "no
> risk".
>
> 2) the text-file-based ACLs it uses will be cumbersome to manage (and
> definitely outside the scope of libvirt). This really should be done by
> policykit instead  of home-grown ACL files.
>
> 3) the qemu helper binary can only handle creating a tap device and
> connecting it to a bridge. It can't handle macvtap interfaces, libvirt's
> virtual networks (unless the user just happens to know the name of the
> bridge used for that interface), openvswitch bridges, nwfilter rules,
> bandwidth limiting, 802.1Qb[gh]... You get the picture - there is a
> *lot* of functionality that's off the table if the qemu helper binary is
> used (unless it gains a lot of features and becomes hopelessly
> intertwined with libvirt, and I don't think anyone wants that).
>
> The way that I think the problem should be solved is this:
>
> 1) All of the network-related functionality in the system instance of
> libvirt that is used by the qemu, lxc, etc. drivers internal to libvirt
> (including the nwfilter subsystem, and everything in bridge_driver.c)
> should be in a separate daemon from libvirtd, and made available via RPC
> with a public API that uses policykit for authorization/authentication,
> with separate selinux policies from libvirtd; maybe call it
> "libvirt-networkd".
>
> 2) When the system instance of libvirtd is creating a domain, it should
> call to libvirt-networkd via this API to do things like create a tap
> device, connect it to a bridge, add nwfilter rules, etc.
>
> 3) likewise, when a session (unprivileged) instance of libvirt is
> creating a domain, it also should call the same APIs, which (after
> proper authentication/authorization via policykit) will connect it to
> the privileged libvirt-networkd (or whatever its called) to perform the
> operation.
>
> The result will be that a guest created by an unprivileged user will
> (dependent on policykit granting access) have access to *all* of the
> network functionality that is available to a guest started by the system
> libvirt instance, including libvirt's virtual networks, macvtap
> interfaces, nwfilter rules, bandwidth limiting, etc. And managing who
> gets access to what will be done in the same way as any other resource
> gated by policykit. Additionally, libvirtd will no longer need many of
> the capabilities/policies it currently needs to permit setting up the
> networking (just as libvirt-networkd won't need all the
> capabilities/policies required by libvirtd to handle other functions of
> managing a guest's lifecycle).
>
> So, how does this relate to my desire to see something in the XML
> indicating that this interface should use the qemu helper? Well, I'm
> thinking ahead to the day when the work I've outlined above is completed
> - when it is included in a libvirt release, there will be people who
> have guests with network configs that work thanks to the presence of the
> qemu-helper support and an entry in the qemu-helper's ACL file. If they
> upgrade their libvirt and their config stops working (due to their uid
> not being authorized by policykit), they won't be happy. On the other
> hand, once this more comprehensive solution is available, it should be
> preferred by default. In order for that to be the case, we need to have
> some sort of tag in the XML *now* that indicates "use the qemu-helper",
> and otherwise pretend that it doesn't exist. That way when those people
> upgrade, even though the new method of setting up networking (which may
> not work without config changes) will be available, libvirt will
> understand it should continue to use the "old" method for this
> particular guest.

That sounds like a good future direction that you've outlined.

At this point I wonder if we might be able to get away with no XML 
modifications since we know that we only want to attempt to run the 
helper when libvirt is running unprivileged.

-- 
Regards,
Corey





More information about the libvir-list mailing list