Re: [PATCH V2 1/1] selinux-testsuite: Add filesystem tests

[Stephen Smalley <sds@tycho.nsa.gov>]

On 1/10/20 1:09 PM, Richard Haines wrote:
> On Thu, 2020-01-09 at 12:19 -0500, Stephen Smalley wrote:
>> On 1/9/20 10:07 AM, Richard Haines wrote:
>>> Test filesystem permissions and setfscreatecon(3).
>>>
>>>   From kernels 5.5 filesystem { watch } is also tested.
>>>
>>> Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
>>> ---
>>> diff --git a/policy/test_filesystem.te b/policy/test_filesystem.te
>>> new file mode 100644
>>> index 0000000..2eee1fc
>>> --- /dev/null
>>> +++ b/policy/test_filesystem.te
>>> @@ -0,0 +1,324 @@
>>> +#
>>> +######### Test filesystem permissions policy module ##########
>>> +#
>>> +attribute filesystemdomain;
>>> +
>>> +#################### Create a test file context
>>> ######################
>>> +type test_filesystem_filecon_t;
>>> +unconfined_runs_test(test_filesystem_filecon_t)
>>> +
>>> +################# Test all functions ##########################
>>> +type test_filesystem_t;
>>> +domain_type(test_filesystem_t)
>>> +unconfined_runs_test(test_filesystem_t)
>>> +typeattribute test_filesystem_t testdomain;
>>> +typeattribute test_filesystem_t filesystemdomain;
>>> +
>>> +allow test_filesystem_t self:capability { sys_admin };
>>> +allow test_filesystem_t self:filesystem { mount remount quotamod
>>> relabelfrom relabelto unmount quotaget };
>>> +allow test_filesystem_t self:dir { mounton add_name write };
>>> +allow test_filesystem_t test_file_t:dir { mounton write
>>> remove_name rmdir };
>>> +# Create test file
>>> +allow test_filesystem_t self:dir { add_name write };
>>> +allow test_filesystem_t self:file { create relabelfrom relabelto
>>> };
>>> +
>>> +fs_mount_all_fs(test_filesystem_t)
>>> +fs_remount_all_fs(test_filesystem_t)
>>> +fs_unmount_all_fs(test_filesystem_t)
>>> +fs_relabelfrom_all_fs(test_filesystem_t)
>>> +fs_get_xattr_fs_quotas(test_filesystem_t)
>>> +files_search_all(test_filesystem_t)
>>> +# Required for mount opts
>>> "rootcontext=system_u:object_r:test_filesystem_t:s0";
>>> +fs_associate(test_filesystem_t)
>>> +fs_getattr_xattr_fs(test_filesystem_t)
>>> +
>>> +# For running quotacheck(8)
>>> +files_type(test_filesystem_t)
>>> +# Update quotas
>>> +fs_set_all_quotas(test_filesystem_t)
>>> +allow test_filesystem_t self:file { quotaon };
>>> +# Create test file and change context:
>>> +fs_associate(test_filesystem_filecon_t)
>>> +allow test_filesystem_t test_filesystem_filecon_t:file { open read
>>> getattr relabelto write };
>>> +dontaudit test_filesystem_t kernel_t:process { setsched };
>>
>> Why do you need these dontaudit statements?  It seems like a kernel
>> bug
>> if something is triggering a setsched permission check on the
>> kernel_t
>> domain?  Something the kernel module is doing during initialization?
>>
> 
> I've tracked this down to them all being called from block/ioprio.c
> with: security_task_setioprio(task, ioprio) -> selinux_task_setioprio
> 
> Why the SECCLASS_PROCESS, PROCESS__SETSCHED I've no idea. The following
> also use SET/GETSCHED permission:
> 
> selinux_task_getioprio, selinux_task_setnice, selinux_task_movememory

The confusing bit is that it is between test_filesystem_t and kernel_t. 
If the process was setting its own ioprio, then I'd expect to see the 
denial between test_filesystem_t and test_filesystem_t aka self.  If the 
process inserted a kernel module and the module initializer spawned a 
kernel thread that set its ioprio, I would expect it to be kernel_t to 
kernel_t.