Hi Ansis,

Ansis Atteka <ansisatt...@gmail.com> writes:

> On 29 March 2017 at 12:25, Aaron Conole <acon...@redhat.com> wrote:
>
> Thanks for doing this. I should have probably built OVS with your patch, but 
> since my RHEL setup is down
> at the moment, then I will do that later. Anyway see my comments inline.

Thank you so much for the review.

>  Set the selinux permissions for DPDK.  After this patch, the openvswitch
>  context label has access to the following resources:
>    * hugepage filesystems
>    * vfio devices
>    * additional unix socket permissions
>    * additional filesystem permissions 
>
>  Additionally, the openvswitch policy is now stored as template file to be
>  built up based on options passed to configure.
>
> Instead of ./configuring fixed policy at build time there is also SELinux 
> booleans feature. See
> https://wiki.gentoo.org/wiki/SELinux/Tutorials/Using_SELinux_booleans for 
> more details. I know
> that other open source projects use that feature to loosen-up their SELinux 
> policies to enable certain
> functional features at run-time that otherwise are disabled by default.

Cool, I didn't know much about it.  I'll look at it.

> While I haven't looked in that SELinux area deeply enough, is there a reason, 
> you don't want to use
> SElinux booleans to allow access to DPDK related resources at run-time? The 
> only obvious argument to
> me is that it would requires administrator to explicitly invoke the setsebool 
> call.

So, I guess first thing was I didn't know about selinux boolean.  I'll
look at it, and follow up.  I guess having to do manual intervention is
a bit inconvenient in my opinion.  And my thinking is that most
distributions who want SELinux + DPDK would just implement a policy like
I do to make it easier for the user.

But I will take a look at it.

>  Signed-off-by: Aaron Conole <acon...@redhat.com>
>
>  ---
>   selinux/automake.mk              |  1 +
>   selinux/openvswitch-custom.te    | 16 ----------------
>   selinux/openvswitch-custom.te.in | 40
>  ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 41 insertions(+), 16 deletions(-)
>   delete mode 100644 selinux/openvswitch-custom.te
>   create mode 100644 selinux/openvswitch-custom.te.in
>
>  diff --git a/selinux/automake.mk b/selinux/automake.mk
>  index 1088f36..6fc30b3 100644
>  --- a/selinux/automake.mk
>  +++ b/selinux/automake.mk
>  @@ -6,4 +6,5 @@
>   # without warranty of any kind.
>
>   EXTRA_DIST += \
>  +        selinux/openvswitch-custom.te.in \
>
>           selinux/openvswitch-custom.te
>  diff --git a/selinux/openvswitch-custom.te b/selinux/openvswitch-custom.te
>  deleted file mode 100644
>  index 47ddb56..0000000
>  --- a/selinux/openvswitch-custom.te
>  +++ /dev/null
>  @@ -1,16 +0,0 @@
>  -module openvswitch-custom 1.0.1;
>  -
>  -require {
>  -        type openvswitch_t;
>  -        type openvswitch_tmp_t;
>  -        type ifconfig_exec_t;
>  -        type hostname_exec_t;
>  -        class netlink_socket { setopt getopt create connect getattr write 
> read };
>  -        class file { write getattr read open execute execute_no_trans };
>  -}
>  -
>  -#============= 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 };
>  diff --git a/selinux/openvswitch-custom.te.in 
> b/selinux/openvswitch-custom.te.in
>  new file mode 100644
>  index 0000000..3a0afc6
>  --- /dev/null
>  +++ b/selinux/openvswitch-custom.te.in
>  @@ -0,0 +1,40 @@
>  +module openvswitch-custom 1.0.1;
>  +
>  +require {
>  +        type openvswitch_t;
>  +        type openvswitch_tmp_t;
>  +        type ifconfig_exec_t;
>  +        type hostname_exec_t;
>  +        class netlink_socket { setopt getopt create connect getattr write 
> read };
>  +        class file { write getattr read open execute execute_no_trans };
>  +}
>  +
>  +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 
> };
>
> I am sorry, but it is not clear to me to what are possible $1 values. I would 
> prefer to keep SElinux types
> hardcoded, if possible.

Hrrm?  This is an selinux macro, so $1 gets substituted based on the
arguments.  And below, the call passes openvswitch as $1, so it becomes
openvswitch_t.

Did I misunderstand your comment?

>  +')
>  +
>  +#============= 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 };
>  +
>  +@begin_dpdk@
>  +dpdk_perms(openvswitch)
>  +@end_dpdk@
>  --
>  2.9.3
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to