[ovs-dev,v2,2/5] selinux: create a transition type for module loading

Defines a type 'openvswitch_load_module_t' used exclusively for loading
modules.  This means that the 'openvswitch_t' domain won't require
access to the module loading facility - such access can only happen
after transitioning through the 'openvswitch_load_module_exec_t'
transition context.

A future commit will instruct the selinux policy on how to label the
appropriate script with extended attributes to make use of this new domain.

Acked-By: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
 selinux/openvswitch-custom.te.in | 79 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 74 insertions(+), 5 deletions(-)

On Fri, 4 May 2018 at 11:28, Aaron Conole <aconole@redhat.com> wrote:

> Defines a type 'openvswitch_load_module_t' used exclusively for loading
> modules.  This means that the 'openvswitch_t' domain won't require
> access to the module loading facility - such access can only happen
> after transitioning through the 'openvswitch_load_module_exec_t'
> transition context.

> A future commit will instruct the selinux policy on how to label the
> appropriate script with extended attributes to make use of this new
domain.

> Acked-By: Timothy Redaelli <tredaelli@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
>   selinux/openvswitch-custom.te.in | 79
+++++++++++++++++++++++++++++++++++++---
>   1 file changed, 74 insertions(+), 5 deletions(-)

> diff --git a/selinux/openvswitch-custom.te.in b/selinux/
openvswitch-custom.te.in
> index db3cf6d8d..31e8fab15 100644
> --- a/selinux/openvswitch-custom.te.in
> +++ b/selinux/openvswitch-custom.te.in
> @@ -1,13 +1,31 @@
>   module openvswitch-custom 1.0.1;

>   require {
> +        role system_r;
> +        role object_r;
> +
>           type openvswitch_t;
>           type openvswitch_rw_t;
>           type openvswitch_tmp_t;
>           type openvswitch_var_run_t;

> +        type bin_t;
>           type ifconfig_exec_t;
> +        type init_t;
> +        type init_var_run_t;
> +        type insmod_exec_t;
>           type hostname_exec_t;
> +        type modules_conf_t;
> +        type modules_object_t;
> +        type passwd_file_t;
> +        type plymouth_exec_t;
> +        type proc_t;
> +        type shell_exec_t;
> +        type sssd_t;
> +        type sssd_public_t;
> +        type sssd_var_lib_t;
> +        type sysfs_t;
> +        type systemd_unit_file_t;
>           type tun_tap_device_t;

>   @begin_dpdk@
> @@ -21,18 +39,36 @@ require {

>           class capability { dac_override audit_write };
>           class chr_file { write getattr read open ioctl };
> -        class dir { write remove_name add_name lock read };
> -        class file { write getattr read open execute execute_no_trans
create unlink };
> +        class dir { write remove_name add_name lock read getattr search
open };
> +        class fd { use };
> +        class file { write getattr read open execute execute_no_trans
create unlink map entrypoint lock ioctl };
> +        class fifo_file { getattr read write append ioctl lock open };
> +        class filesystem getattr;
> +        class lnk_file { read open };
>           class netlink_audit_socket { create nlmsg_relay audit_write read
write };
>           class netlink_socket { setopt getopt create connect getattr
write read };
> -        class unix_stream_socket { write getattr read connectto connect
setopt getopt sendto accept bind recvfrom acceptfrom };
> +        class sock_file { write };
> +        class system module_load;
> +        class process { sigchld signull transition noatsecure siginh
rlimitinh };
> +        class unix_stream_socket { write getattr read connectto connect
setopt getopt sendto accept bind recvfrom acceptfrom ioctl };

>   @begin_dpdk@
> -        class sock_file { read write append getattr open };
> +        class sock_file { read append getattr open };
>           class tun_socket { relabelfrom relabelto create };
>   @end_dpdk@
>   }

> +#============= Set up the transition domain =============
> +type openvswitch_load_module_exec_t;
> +type openvswitch_load_module_t;
> +
> +domain_type(openvswitch_load_module_exec_t);
> +domain_type(openvswitch_load_module_t);
> +role object_r types openvswitch_load_module_exec_t;
> +role system_r types openvswitch_load_module_t;
> +domain_entry_file(openvswitch_load_module_t,
openvswitch_load_module_exec_t);
> +domtrans_pattern(openvswitch_t, openvswitch_load_module_exec_t,
openvswitch_load_module_t);
> +
>   #============= openvswitch_t ==============
>   allow openvswitch_t self:capability { dac_override audit_write };
>   allow openvswitch_t self:netlink_audit_socket { create nlmsg_relay
audit_write read write };
> @@ -41,10 +77,11 @@ allow openvswitch_t self:netlink_socket { setopt
getopt create connect getattr w
>   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_rw_t:dir { write remove_name add_name
lock read };
> +allow openvswitch_t openvswitch_rw_t:dir { write remove_name add_name
lock read getattr open search };
>   allow openvswitch_t openvswitch_rw_t:file { write getattr read open
execute execute_no_trans create unlink };
>   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 openvswitch_var_run_t:dir { getattr read open search
};
>   allow openvswitch_t tun_tap_device_t:chr_file { read write getattr open
ioctl };

>   @begin_dpdk@
> @@ -58,3 +95,35 @@ allow openvswitch_t svirt_tmpfs_t:sock_file { read
write append getattr open };
>   allow openvswitch_t svirt_t:unix_stream_socket { connectto read write
getattr sendto recvfrom setopt };
>   allow openvswitch_t vfio_device_t:chr_file { read write open ioctl
getattr };
>   @end_dpdk@
> +
> +#============= Transition allows =============
> +type_transition openvswitch_t openvswitch_load_module_exec_t:process
openvswitch_load_module_t;
> +allow openvswitch_t openvswitch_load_module_exec_t:file { execute read
open getattr };
> +allow openvswitch_t openvswitch_load_module_t:process transition;
> +
> +allow openvswitch_load_module_t bin_t:file { execute execute_no_trans
map };
> +allow openvswitch_load_module_t init_t:unix_stream_socket { getattr
ioctl read write };
> +allow openvswitch_load_module_t init_var_run_t:dir { getattr read open
search };
> +allow openvswitch_load_module_t insmod_exec_t:file { execute
execute_no_trans getattr map open read };
> +allow openvswitch_load_module_t modules_conf_t:dir { getattr open read
search };
> +allow openvswitch_load_module_t modules_conf_t:file { getattr open read
};
> +allow openvswitch_load_module_t modules_object_t:file { map getattr open
read };
> +allow openvswitch_load_module_t modules_object_t:dir { getattr open read
search };
> +allow openvswitch_load_module_t openvswitch_load_module_exec_t:file {
entrypoint };
> +allow openvswitch_load_module_t passwd_file_t:file { getattr open read };
Were these rules auto generated with audit2allow?

It is not obvious to me why ovs-kmod-ctl process running under
openvswitch_load_module_t would need the {read, open, getattr} permissions
to files with passwd_file_t label (presumably /etc/passwd)?



> +allow openvswitch_load_module_t plymouth_exec_t:file { getattr read open
execute execute_no_trans map };
Same for plymouth.

Perhaps this could be just due to my incomplete understanding what it takes
to reload kernel module. If you know the answer already please feel free to
chime in. If not, I will try to regenerate policy on my end and then we can
do a diff.

> +allow openvswitch_load_module_t proc_t:file { getattr open read };
> +allow openvswitch_load_module_t self:system module_load;
> +allow openvswitch_load_module_t self:process { siginh noatsecure
rlimitinh siginh };
> +allow openvswitch_load_module_t shell_exec_t:file { map execute read
open getattr };
> +allow openvswitch_load_module_t sssd_public_t:dir { getattr open read
search };
> +allow openvswitch_load_module_t sssd_public_t:file { getattr map open
read };
> +allow openvswitch_load_module_t sssd_t:unix_stream_socket connectto;
> +allow openvswitch_load_module_t sssd_var_lib_t:dir { getattr open read
search };
> +allow openvswitch_load_module_t sssd_var_lib_t:sock_file write;
> +allow openvswitch_load_module_t sysfs_t:dir { getattr open read search };
> +allow openvswitch_load_module_t sysfs_t:file { open read };
> +allow openvswitch_load_module_t sysfs_t:lnk_file { read open };
> +allow openvswitch_load_module_t systemd_unit_file_t:dir getattr;
> +
> +kernel_load_module(openvswitch_load_module_t);
> --
> 2.14.3

Ansis Atteka <ansisatteka@gmail.com> writes:

> On Fri, 4 May 2018 at 11:28, Aaron Conole <aconole@redhat.com> wrote:
>
>> Defines a type 'openvswitch_load_module_t' used exclusively for loading
>> modules.  This means that the 'openvswitch_t' domain won't require
>> access to the module loading facility - such access can only happen
>> after transitioning through the 'openvswitch_load_module_exec_t'
>> transition context.
>
>> A future commit will instruct the selinux policy on how to label the
>> appropriate script with extended attributes to make use of this new
> domain.
>
>> Acked-By: Timothy Redaelli <tredaelli@redhat.com>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>>   selinux/openvswitch-custom.te.in | 79
> +++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 74 insertions(+), 5 deletions(-)
>
>> diff --git a/selinux/openvswitch-custom.te.in b/selinux/
> openvswitch-custom.te.in
>> index db3cf6d8d..31e8fab15 100644
>> --- a/selinux/openvswitch-custom.te.in
>> +++ b/selinux/openvswitch-custom.te.in
>> @@ -1,13 +1,31 @@
>>   module openvswitch-custom 1.0.1;
>
>>   require {
>> +        role system_r;
>> +        role object_r;
>> +
>>           type openvswitch_t;
>>           type openvswitch_rw_t;
>>           type openvswitch_tmp_t;
>>           type openvswitch_var_run_t;
>
>> +        type bin_t;
>>           type ifconfig_exec_t;
>> +        type init_t;
>> +        type init_var_run_t;
>> +        type insmod_exec_t;
>>           type hostname_exec_t;
>> +        type modules_conf_t;
>> +        type modules_object_t;
>> +        type passwd_file_t;
>> +        type plymouth_exec_t;
>> +        type proc_t;
>> +        type shell_exec_t;
>> +        type sssd_t;
>> +        type sssd_public_t;
>> +        type sssd_var_lib_t;
>> +        type sysfs_t;
>> +        type systemd_unit_file_t;
>>           type tun_tap_device_t;
>
>>   @begin_dpdk@
>> @@ -21,18 +39,36 @@ require {
>
>>           class capability { dac_override audit_write };
>>           class chr_file { write getattr read open ioctl };
>> -        class dir { write remove_name add_name lock read };
>> -        class file { write getattr read open execute execute_no_trans
> create unlink };
>> +        class dir { write remove_name add_name lock read getattr search
> open };
>> +        class fd { use };
>> +        class file { write getattr read open execute execute_no_trans
> create unlink map entrypoint lock ioctl };
>> +        class fifo_file { getattr read write append ioctl lock open };
>> +        class filesystem getattr;
>> +        class lnk_file { read open };
>>           class netlink_audit_socket { create nlmsg_relay audit_write read
> write };
>>           class netlink_socket { setopt getopt create connect getattr
> write read };
>> -        class unix_stream_socket { write getattr read connectto connect
> setopt getopt sendto accept bind recvfrom acceptfrom };
>> +        class sock_file { write };
>> +        class system module_load;
>> +        class process { sigchld signull transition noatsecure siginh
> rlimitinh };
>> +        class unix_stream_socket { write getattr read connectto connect
> setopt getopt sendto accept bind recvfrom acceptfrom ioctl };
>
>>   @begin_dpdk@
>> -        class sock_file { read write append getattr open };
>> +        class sock_file { read append getattr open };
>>           class tun_socket { relabelfrom relabelto create };
>>   @end_dpdk@
>>   }
>
>> +#============= Set up the transition domain =============
>> +type openvswitch_load_module_exec_t;
>> +type openvswitch_load_module_t;
>> +
>> +domain_type(openvswitch_load_module_exec_t);
>> +domain_type(openvswitch_load_module_t);
>> +role object_r types openvswitch_load_module_exec_t;
>> +role system_r types openvswitch_load_module_t;
>> +domain_entry_file(openvswitch_load_module_t,
> openvswitch_load_module_exec_t);
>> +domtrans_pattern(openvswitch_t, openvswitch_load_module_exec_t,
> openvswitch_load_module_t);
>> +
>>   #============= openvswitch_t ==============
>>   allow openvswitch_t self:capability { dac_override audit_write };
>>   allow openvswitch_t self:netlink_audit_socket { create nlmsg_relay
> audit_write read write };
>> @@ -41,10 +77,11 @@ allow openvswitch_t self:netlink_socket { setopt
> getopt create connect getattr w
>>   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_rw_t:dir { write remove_name add_name
> lock read };
>> +allow openvswitch_t openvswitch_rw_t:dir { write remove_name add_name
> lock read getattr open search };
>>   allow openvswitch_t openvswitch_rw_t:file { write getattr read open
> execute execute_no_trans create unlink };
>>   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 openvswitch_var_run_t:dir { getattr read open search
> };
>>   allow openvswitch_t tun_tap_device_t:chr_file { read write getattr open
> ioctl };
>
>>   @begin_dpdk@
>> @@ -58,3 +95,35 @@ allow openvswitch_t svirt_tmpfs_t:sock_file { read
> write append getattr open };
>>   allow openvswitch_t svirt_t:unix_stream_socket { connectto read write
> getattr sendto recvfrom setopt };
>>   allow openvswitch_t vfio_device_t:chr_file { read write open ioctl
> getattr };
>>   @end_dpdk@
>> +
>> +#============= Transition allows =============
>> +type_transition openvswitch_t openvswitch_load_module_exec_t:process
> openvswitch_load_module_t;
>> +allow openvswitch_t openvswitch_load_module_exec_t:file { execute read
> open getattr };
>> +allow openvswitch_t openvswitch_load_module_t:process transition;
>> +
>> +allow openvswitch_load_module_t bin_t:file { execute execute_no_trans
> map };
>> +allow openvswitch_load_module_t init_t:unix_stream_socket { getattr
> ioctl read write };
>> +allow openvswitch_load_module_t init_var_run_t:dir { getattr read open
> search };
>> +allow openvswitch_load_module_t insmod_exec_t:file { execute
> execute_no_trans getattr map open read };
>> +allow openvswitch_load_module_t modules_conf_t:dir { getattr open read
> search };
>> +allow openvswitch_load_module_t modules_conf_t:file { getattr open read
> };
>> +allow openvswitch_load_module_t modules_object_t:file { map getattr open
> read };
>> +allow openvswitch_load_module_t modules_object_t:dir { getattr open read
> search };
>> +allow openvswitch_load_module_t openvswitch_load_module_exec_t:file {
> entrypoint };
>> +allow openvswitch_load_module_t passwd_file_t:file { getattr open read };
> Were these rules auto generated with audit2allow?
>
> It is not obvious to me why ovs-kmod-ctl process running under
> openvswitch_load_module_t would need the {read, open, getattr} permissions
> to files with passwd_file_t label (presumably /etc/passwd)?

I think (just guessing, because it's not obvious to me either, and I
can't seem to find the cause) it's because when the domain transition
happens some of the PAM stack is run which goes through pam_unix,
pam_sss, and pam_systemd on my system.  Perhaps that's why these are
read required.

Thanks for the review, Ansis!  I'll submit a v3 today which keeps these
policy markers.

>
>> +allow openvswitch_load_module_t plymouth_exec_t:file { getattr read open
> execute execute_no_trans map };
> Same for plymouth.
>
> Perhaps this could be just due to my incomplete understanding what it takes
> to reload kernel module. If you know the answer already please feel free to
> chime in. If not, I will try to regenerate policy on my end and then we can
> do a diff.

I think this is used when loading modules so that the plymouth system
registers the module or something?  I do find plymouth being referenced
in some of the kernel-pkg scripts on the system (for example,
/usr/sbin/new-kernel-pkg), so I believe it's somehow a requirement (at
least on my f27 system).

>> +allow openvswitch_load_module_t proc_t:file { getattr open read };
>> +allow openvswitch_load_module_t self:system module_load;
>> +allow openvswitch_load_module_t self:process { siginh noatsecure
> rlimitinh siginh };
>> +allow openvswitch_load_module_t shell_exec_t:file { map execute read
> open getattr };
>> +allow openvswitch_load_module_t sssd_public_t:dir { getattr open read
> search };
>> +allow openvswitch_load_module_t sssd_public_t:file { getattr map open
> read };
>> +allow openvswitch_load_module_t sssd_t:unix_stream_socket connectto;
>> +allow openvswitch_load_module_t sssd_var_lib_t:dir { getattr open read
> search };
>> +allow openvswitch_load_module_t sssd_var_lib_t:sock_file write;
>> +allow openvswitch_load_module_t sysfs_t:dir { getattr open read search };
>> +allow openvswitch_load_module_t sysfs_t:file { open read };
>> +allow openvswitch_load_module_t sysfs_t:lnk_file { read open };
>> +allow openvswitch_load_module_t systemd_unit_file_t:dir getattr;
>> +
>> +kernel_load_module(openvswitch_load_module_t);
>> --
>> 2.14.3
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev