SELinuxProject / selinux-testsuite

This is the upstream SELinux testsuite which is designed as a basic set of regression tests for the SELinux kernel functionality.
GNU General Public License v2.0
54 stars 43 forks source link

Testsuite was failing on RHEL7 #7

Closed rhatdan closed 7 years ago

rhatdan commented 7 years ago

ls -Z on current coreutils is different then on RHEL7, so this is causing getfilecon tests to fail. Changed to use getfattr.

Also perl does not like ($output eq "unconfined...") Changing to ($output == "unconfined...")

Makes it work on RHEL and Fedora

rhatdan commented 7 years ago

@rhvgoyal PTAL

rhatdan commented 7 years ago

BTW I added the kernel_read_system_state(client,mounter) because for some reason python -c "import os; os.access("/etc", os.R_OK)" Attempts to read /proc/meminfo.

rhvgoyal commented 7 years ago

I can confirm that this fixes the issue for me and now selinux-testsuite passes on rhel7 with backported selinux patches.

pcmoore commented 7 years ago

We've faced the 'ls -Z' problem before, see commit d5cb90922cbf53d456581bd762fdda9255298b9b. I'm not sure I have a strong opinion about which fix is right, but I think we should be consistent and fix the same way across the testsuite. We also should make sure that the getfattr output is consistent across releases; have you tested this on older RHEL and/or Fedora releases?

I think it would also be good to split this into three patches: one for the coreutils fix, one for the eq fix, and one for the kernel_read_system_state(...) fix.

rhatdan commented 7 years ago

I have only tested the getxattr fix on Rawhide and RHEL7. But I am pretty sure this has not changed in years.

I thought about using:

python -c "import selinux, sys; print(selinux.getfilecon(sys.argv[1]))[1]"

But I don't know if you want to bring python into the test suite.

rhatdan commented 7 years ago

Split the patch into three patches.

stephensmalley commented 7 years ago

perl docs say that eq is for string comparison, == is for numeric comparison. So your last change makes no sense. IMHO, the coreutils change was a backward-incompatible change that never should have happened and will no doubt break many scripts as people move to RHEL7.

stephensmalley commented 7 years ago

$cat testeq.perl

!/usr/bin/perl

$str = "This is a test"; if ($str eq "This is not a test") { print "eq wrongly thought that the strings matched\n"; } else { print "eq correctly detected that the strings do not match\n"; } if ($str == "This is not a test") { print "== wrongly thought that the strings matched (because they have the same numeric value, i.e. 0)\n"; } else { print "== correctly detected that the strings do not match"; } $ ./testeq.perl eq correctly detected that the strings do not match == wrongly thought that the strings matched (because they have the same numeric value, i.e. 0)

IOW, the problem is not the test code - you have an actual failure on RHEL7. Don't hide that.

rhatdan commented 7 years ago

I can go back and look at it, but we were seeing the correct strings when I output the text. I could not see a "\n" in the text, but will try again with the eq and add a chomp() to the object.

rhatdan commented 7 years ago

@stephensmalley @pcmoore PTAL @rhvgoyal reports these are working for him on RHEL.

stephensmalley commented 7 years ago

LGTM, passes on rawhide. Any objections, @pcmoore ?

stephensmalley commented 7 years ago

Hmmm...it passed on rawhide's 4.8.0 kernels but is failing with 4.9.0. But I see that even the existing selinux-testsuite is failing there too on 4.9.0: overlay/test ............ mount: mount none on /home/sds/selinux-testsuite/tests/overlay/container1/merged failed: Operation not supported

No tests run!

overlay/test ............ Dubious, test returned 255 (wstat 65280, 0xff00) Failed 101/101 subtests

Looks like a kernel config problem with 4.9.0: [ 205.571695] SELinux: (dev overlay, type overlay) has no xattr support

stephensmalley commented 7 years ago

Seem to be some other problems with that kernel too; getting stack dumps. BTW, SELinux is still reporting undefined permissions there: [ 163.221107] SELinux: Permission validate_trans in class security not defined in policy. [ 163.221115] SELinux: Permission module_load in class system not defined in policy. [ 163.221175] SELinux: the above unknown classes and permissions will be allowed I thought that got fixed.

stephensmalley commented 7 years ago

Not a kernel config problem, but rather the work.xattr changes from Andreas Gruenbacher seem to have broken overlayfs xattr support, at least as used/tested by SELinux?

rhatdan commented 7 years ago

Yes we have been seeing the same issues. @rhvgoyal is looking into it.

pcmoore commented 7 years ago

I saw some kernel oopses earlier this week with my Copr testing kernels; it looked like some locking changes in the vfs layer were causing problems. I've been working my way through the bisection but progress has been slow due to other commitments this week.

rhvgoyal commented 7 years ago

A fix for selinux issue is available now. Al Viro has applied it. Waiting for it to be merged in linus tree.

http://marc.info/?l=linux-fsdevel&m=147640702913273&w=2

stephensmalley commented 7 years ago

Ok, I'm fine with this change for selinux-testsuite. Any objections?

pcmoore commented 7 years ago

@stephensmalley Not from me assuming that getfattr provides a consistent output; in fact we should probably change the other 'ls -Z' tests if that is the case.

@rhvgoyal I'm guessing that patch from Al doesn't fix the locking problem on 4.9-rc0 kernels?

stephensmalley commented 7 years ago

No, it just fixes the overlay has no xattr support bug.

pcmoore commented 7 years ago

Okay, that's what I thought by looking at the patch, but one can always hope :)

rhvgoyal commented 7 years ago

@pcmoore, right, that patch does not fix the locking issues.