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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/13/20 1:00 PM, Richard Haines wrote:
On Mon, 2020-01-13 at 09:07 -0500, Stephen Smalley wrote:
On 1/13/20 8:52 AM, Stephen Smalley wrote:
On 1/12/20 11:04 AM, Richard Haines wrote:
On Fri, 2020-01-10 at 13:18 -0500, Stephen Smalley wrote:
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@xxxxxxxxxxxxxx>
---
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.

Some more info on who calls set_task_ioprio:

fs/ext4/super.c calls 'set_task_ioprio' in two places using:
      set_task_ioprio(sbi->s_journal->j_task, journal_ioprio);
The return codes are not checked. This code was added 11 years
ago.

fs/btrfs/reada.c also calls 'set_task_ioprio' in two places
using:
      set_task_ioprio(current, BTRFS_IOPRIO_READA);
The return codes are not checked.

As can be seen the ext4 module does not use 'current'. I have
patched
kernel 5.5-rc5 to use 'current' and it now works as you expected.
Also
the kernel_t:process { setsched } rules can be removed.
As the problem will exist for some time, I've added to the test
policy:
      kernel_dontaudit_setsched(filesystemdomain)

It appears that most of the refpolicy modules do the same.

This seems like a kernel bug to me. I assume that these
filesystems
expect the I/O priority to be always set in these cases
irrespective of
the permissions of the current process.  Either they should be
using
some internal helper function ala a new set_task_ioprio_noperm()
that
skips permission checking or they should be temporarily overriding
their
cred to the init cred before doing this.  Probably a topic for
linux-fsdevel and/or the respective per-filesystem mailing lists.

Also, looks like kernel_dontaudit_setsched() isn't defined by
upstream
refpolicy so you'll need the usual ifdefery in test_policy.if to
allow
this to build against refpolicy to appease travis-ci.


I'll change this to domain_setpriority_all_domains() in V4 as that is
in both Fedora and Ref Policy.

That would likely break the task_setscheduler test. Try kernel_setsched() instead, or if that fails, just use an ifdef as we do for other interfaces that don't exist in refpolicy.





[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux