Re: [ovs-dev] [PATCH] selinux: Allow creating tap devices.

[Aaron Conole <aconole@redhat.com>]

Aaron Conole <aconole@redhat.com> writes:

> Daniele Di Proietto <diproiettod@vmware.com> writes:
>
>> On 26/01/2017 12:35, "Ansis Atteka" <ansisatteka@gmail.com> wrote:
>>>
>>>
>>>On 26 January 2017 at 21:24, Aaron Conole 
>>><aconole@redhat.com> wrote:
>>>
>>>Daniele Di Proietto <diproiettod@vmware.com> writes:
>>>
>>>> On 25/01/2017 00:01, "Ansis Atteka" <ansisatteka@gmail.com> wrote:
>>>>
>>>>>On Jan 25, 2017 4:22 AM, "Daniele Di Proietto" <diproiettod@vmware.com> wrote:
>>>>>
>>>>>Current SELinux policy in RHEL and Fedora doesn't allow the creation of
>>>>>TAP devices.
>>>>>
>>>>>A tap device is used by dpif-netdev to create internal devices.
>>>>>
>>>>>Without this patch, adding any bridge backed by the userspace datapath
>>>>>would fail.
>>>>>
>>>>>This doesn't mean that we can run Open vSwitch with DPDK under SELinux
>>>>>yet, but at least we can use the userspace datapath.
>>>>>
>>>>>Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
>>>
>>>I just noticed this, sorry for jumping in late.
>>>
>>>>>Acked-by: Ansis Atteka <aatteka@ovn.org>
>>>>>
>>>>>
>>>>>I saw that other open source projects like OpenVPN use rw_file_perms
>>>>> shortcut macro. Not sure how relevant that is for OVS but that macro
>>>>> expands to a little more function calls than what you have
>>>>> below. Maybe we don't need it, if what you have
>>>>> just worked.
>>>>
>>>> Thanks a lot for the review.
>>>>
>>>> I cooked this up using audit2allow and I tested it on fedora 25.  I'm
>>>> now able to create and delete userspace bridges, without any further
>>>> complaints from selinux
>>>
>>>I have the following openvswitch-custom.te that did work to run
>>>ovs+dpdk under selinux and pass traffic:
>>>
>>>
>>>Thanks for posting this. I think that this is really helpful to
>>> gather all necessary OVS+DPDK rules from different sources to make
>>> sure that nothing is missed.
>> +1, thanks a lot
>>> 
>>>
>>>
>>>-------------------- 8< --------------------
>>>
>>>require {
>>>        type openvswitch_t;
>>>        type openvswitch_tmp_t;
>>>        type openvswitch_var_run_t;
>>>        type ifconfig_exec_t;
>>>        type hostname_exec_t;
>>>        type vfio_device_t;
>>>        type kernel_t;
>>>        type tun_tap_device_t;
>>>        type hugetlbfs_t;
>>>        type init_t;
>>>        class netlink_socket { setopt getopt create connect getattr write read };
>>>        class file { write getattr read open execute execute_no_trans create unlink };
>>>        class chr_file { write getattr read open ioctl };
>>>        class unix_stream_socket { write getattr read connectto connect setopt getopt sendto accept bind recvfrom acceptfrom };
>>>        class dir { write remove_name add_name lock read };
>>>}
>>>
>>>#============= openvswitch_t ==============
>>>allow openvswitch_t self:netlink_socket { setopt getopt create connect getattr write read };
>>>allow openvswitch_t hostname_exec_t:file { read getattr open execute execute_no_trans };
>>>allow openvswitch_t ifconfig_exec_t:file { read getattr open execute execute_no_trans };
>>>allow openvswitch_t openvswitch_tmp_t:file { execute execute_no_trans };
>>>allow openvswitch_t openvswitch_tmp_t:unix_stream_socket { write getattr read connectto connect setopt getopt sendto accept bind recvfrom acceptfrom };
>>>allow openvswitch_t vfio_device_t:chr_file { read write open ioctl getattr };
>>>allow openvswitch_t tun_tap_device_t:chr_file { read write getattr open ioctl };
>>>allow openvswitch_t hugetlbfs_t:dir { write remove_name add_name lock read };
>>>allow openvswitch_t hugetlbfs_t:file { create unlink };
>>>allow openvswitch_t kernel_t:unix_stream_socket { write getattr read connectto connect setopt getopt sendto accept bind recvfrom acceptfrom };
>>>allow openvswitch_t init_t:file { read open };
>>>
>>>-------------------- >8 --------------------
>>>
>>>You'll note that this change gives the openvswitch complete access to
>>>hugetlbfs label, which might be the biggest scary part.
>>>
>>>
>>>There is also option to use SELinux switches that allow to activate only subset of SElinux rules on a "per OVS feature basis" if there is risk that because of DPDK whitelise we could be unconditionally loosening up SElinux policy too much for non-DPDK
>>> cases. See [https://wiki.centos.org/TipsAndTricks/SelinuxBooleans] for more details.
>> Ok, so perhaps we should require tun_tap_device_t permissions only if
>> we enable userspace support with a boolean.
>> I just posted this piece because the corresponding code is in
>> openvswitch source tree.
>> The rest of the permissions (hugepages, vfio) are required because of
>> code that's in the dpdk library.  Is there a way to put these in DPDK
>> and then just call a macro here, like
>> dpdk_perms(openvswitch_t)
>
> Below is an example of the macro:
>
> -------------------- 8< --------------------
>
> define(`dpdk_perms', `
> 	gen_require(`
> 		type vfio_device_t;
> 		type kernel_t;
> 		type hugetlbfs_t;
> 		class file { write getattr read open execute execute_no_trans
> 			create unlink };
> 		class chr_file { write getattr read open ioctl };
> 		class unix_stream_socket { write getattr read connectto connect
> 			 setopt getopt sendto accept bind recvfrom acceptfrom };
> 		class dir { write remove_name add_name lock read };
> 	')
>
> 	allow $1_t vfio_device_t:chr_file { read write open ioctl getattr };
> 	allow $1_t hugetlbfs_t:dir { write remove_name add_name lock read };
> 	allow $1_t hugetlbfs_t:file { create unlink };
> 	allow $1_t kernel_t:unix_stream_socket { write getattr read connectto
> 		connect setopt getopt sendto accept bind recvfrom acceptfrom };
> ')
>
> -------------------- >8 --------------------
>
> And then it can be called at the end of the .te file as:
>
>   dpdk_perms(openvswitch)
>
> I am not sure how best to install this in the end system to make sure
> that it gets included properly - I'll ask around here and maybe get an
> answer (or even post a patch to the dpdk mailing list).  I did try
> making a .te file with this macro and a policy definition, but I wasn't
> able to reference it from within openvswitch-custom.te; most likely I
> will need to figure out where my configuration is wrong.

So, here's what I've done so far with the above;  I'm running with the
attached patch - admittedly, it needs to be integrated so that it can be
disabled/enabled based on --with-dpdk flag.

I have tested it out, and it seems to work - I've passed some traffic,
and am able to run (as non-root user, even! :) through some basic
traffic scenarios.

Do you think it's the right thing now to integrate this into the
configure/make system so that openvswitch-custom.te can use the macro
when dpdk is enabled?

-Aaron