We will certainly not create a new phase type, but I can imagine some mechanism for saying "hey, it is expected to see some AVCs here, do not freak out" could be useful: either a parameter to phase?


Hmmm... I think the AVC check happens when we use rhts/beaker functions for reporting above. Do I understand correctly that AVC_ERROR controls this behavior for beaker, or is it something around selinux itself that disables avc reporting completely? If the former is the case, I think something like rlPhaseEnd --noavc could do the trick. 

I'm also not sure how the setenforce matches into this: I am pretty confident that nothing in phases should ever touch the enforcing settings. Do you expect that the phase should also switch enforcing off?

(In reply to comment #1)
> We will certainly not create a new phase type, but I can imagine some mechanism
> for saying "hey, it is expected to see some AVCs here, do not freak out" could
> be useful: either a parameter to phase?

^ this is sufficient 

> 
> 
> Hmmm... I think the AVC check happens when we use rhts/beaker functions for
> reporting above. Do I understand correctly that AVC_ERROR controls this
> behavior for beaker, or is it something around selinux itself that disables avc
> reporting completely? If the former is the case, I think something like
> rlPhaseEnd --noavc could do the trick. 

AVC_ERROR is beaker variable only. When set to '+no_avc_check' then beaker will not report any avc denial.

> 
> I'm also not sure how the setenforce matches into this: I am pretty confident
> that nothing in phases should ever touch the enforcing settings. Do you expect
> that the phase should also switch enforcing off?

Yes, it has to switch off selinux too. (it could be one more parameter)

Typical example is security defect in component which is reproducible ONLY when selinux is in permissive mode. Otherwise you will not reproduce any security issue -> selinux blocks it before.

(In reply to comment #2)
> > I'm also not sure how the setenforce matches into this: I am pretty confident
> > that nothing in phases should ever touch the enforcing settings. Do you expect
> > that the phase should also switch enforcing off?
> 
> Yes, it has to switch off selinux too. (it could be one more parameter)
> 
> Typical example is security defect in component which is reproducible ONLY when
> selinux is in permissive mode. Otherwise you will not reproduce any security
> issue -> selinux blocks it before.

I think this is something that the test itself needs to handle. I agree about abstracting the AVC_ERROR part, because this is something specific to beaker and tied to the reported result (I could intentionally provoke some AVCs so I can test something working with them), but if the test needs to be run in permissive, then it is the test's responsibility to set the environment. I do not think permissive/enforcing should be in any way related to phases or beakerlib itself. The commands are trivial, the amount of work saved is minimal.

Switching avc checker off is an overkill. Sure, you become able to reproduce the security issue but at the same time you might be masking other, valid AVC denials. I'd suggest using custom selinux modules allowing only the bare minimum needed for the security issue to be reproduced, leaving the rest of selinux enabled. 

-1 to this RFE

(In reply to comment #5)
> Because there are
> tests which mus stop selinux, and people sometimes forget to start it.

<tongue-in-cheek>
if only we'd have a tool that would detect those (clearly broken) tests, so we wouldn't need to workaround it in beakerlib.
</tongue-in-cheek>

Hi,
I think same, it is happend very rarerly
Problem part is more less, that when there will be something like rlRunNoSelinux then selinux will be disabled only for small amount of time, instead of disabling selinux for whole test.

Second reply is, that propably there is no difference beetween disabling selinux for short time and masking minimum with selinux module in case one isn't not selinux expert :-)
   Regards
   Honza

IMO rlRunNoSelinux/rlPhaseStartTestWithoutSelinux aka rlYouBrandit is no workaround, no hack, just make a testwriter life easier no to thing about such a variables.

I don't mean selinux switched off because I am lazy to file a bug against policy. Such a avc denial is expected, own policy is changing release by release.

There is high risk that you write WRONG test not reproducing any security issue in your component because selinux blocks it. But in such a case I dont care selinux I would like to write a test for component.

+1 for no selinux Phase

My point is that the whole 'AVC_ERROR=+no_avc_check' isn't a good way of reproducing security issue. Making a bad solution easier to use is ... bad? 

'AVC_ERROR=+no_avc_check' is good when you need to trigger the avc denial but don't want it to fail your test ~ for example when testing audit daemon itself

Hi,
As Pert wrote, its not about laziness, but policies are changing beetween releases, and not easy to write it (to be mininal and general together).
Yes for normal tests it is not usable, you must have selinux on,
but time by time, you need to write test where you know that selinux didn't permit it.
some examples:
* With selinux disabled you are able to write test in 2 hours, with selinux enabled you will write same test for 1 day (in case it is possible at all)
(for example you need to run some CVE security vulneability, run some reproducer app on privilegied port, what is not permitted it, or whatever you want) with selinux can it be impossible or much more time effort.
pros:
 * time effort
 * test is cleaner
 * easy to understand what happen
 * known(one) way how to change selinux

* Not each test is useful for selinux. From my point of view, there are usually 3 possibilitis:
 - any action with binary protected by selinux cause denial (more than one test causes the problem) (so you'll see it)
 - avc denial is periodically appearing after daemon started (then you also will see avc denial in another log)
 - avc denials appears only in beaker not in manual scheduling

* Only few tests need this way how to run phase or command.

So how to wrote Petr, It is for testers to do our life easier (be able write some type of test), not to hide any selinux problem.
Only in 1% (or less) of cases you use this feature, but will be easy to find it (also possible to do some report to beaker, that selinux test is disabled), instead of hacking with varibles and manually turning off selinux.
   Regards
   Honza

I also forgot another example.
You wrote test to fur some bug. But this test also find selinux problem, but it causes test malfunction.
You fill the bug, and now you aren't able to use this test, although you know it, but don't know when it will be repaired (same releas, next, never?).
Write own policy is nonsense from my pint of view, can do test very tricky and also it can hide some problem in future (when you forgot to remove you policy)
In case you can disable selinux, your test is testing what you want and easy to understand what happen, and bug for selinux is also filled.
   Honza

Not sure we're on the same page here, what I suggest is you do this:

1) run reproducer, it fails with selinux denial
2) debug denial, find out that it's not a policy bug but a correct behaviour (selinux doing its job, preventing malicious code from working)
3) fix the test to allow this particular form of exploit temporarily

1) and 2) have to be done in all cases. What we discuss here is 3).

I propose to do it like this:

audit2allow -M smdm < avc.txt
semodule -i smdm.pp
rlRun "exploit"
semodule -r smdm

avc.txt is the avc log message from 1) & 2). I don't see how your pros apply here


> * time effort
> * test is cleaner
> * easy to understand what happen
> * known(one) way how to change selinux

-1 for selinux-policy workarounds.

Policies do not change that much. Moreover it is not easy to ignore AVCs caused by particular command - you can e.g. ignore AVCs withing some time frame but that may hide AVC error of some deamon app running on the background.
According my experience most AVC denials in tests is just because you are storing files/data in wrong places (e.g. redirect command logs to /tmp while cmd is not allowed to write there). 

> You wrote test to fur some bug. But this test also find selinux problem, but 
> it causes test malfunction.

You may store required selinux rules in a custom module and load this module conditionaly just in case it is not allowed with policy itself. And print a reasonably important warning is such case.

Being a QE for selinux-policy I also encountered the problem you all are talking about. My automated tests would welcome following options:

1) disable the AVC subtest for a short period of time (e.g. one phase)
2) disable the AVC subtest completely (whole time the automated test is running)

Because I don't know if beaker supports something that matches the first option, I usually use the "AVC_ERROR=+no_avc_check" hack which has its consequences (I have to search for AVCs at the end of each test). The second thing about "AVC_ERROR=+no_avc_check" hack that I don't like is that it's written in the Makefile, it's not a beaker function.

Would anyone here use following beaker functions?
rlEnableAvcChecking
rlDisableAvcChecking

I need more info about the AVC_ERROR variable. How does it really work? 

I'm doing some assumptions here, please correct me if I am wrong.

The AVC test happens when I call rhts-report. If I understand that correctly, if I have AVC_ERROR=+no_avc_check set at this time, the AVC check does not happen: no FAILs in result, but the underlying selinux machinery still works (AVC denied-s are logged, and if the mode is enforcing, selinux blocks the bad behavior). What happens if I unset the AVC_ERROR after this, do no more bad stuff and call rhts-report again? AVC check will happen, but will it PASS or FAIL?

If it works correctly (from my point of view), i.e. PASS is reported for both cases, I think the most clean is the addition to rlPhaseEnd: it clearly marks the result of this phase does not take AVC problems into account.

In this case,

rlEnableAvcChecking
rlDisableAvcChecking

would not work as expected - the check is done at the time of reporting, so AVCs created *before* rlDisableAvcChecking would still be ignored in next rlPhaseEnd.

This only covers the cases where we *expect* some AVCs to happen, and we do not want Beaker to report them as FAILs. Setting the environment for the test to actually work with SELinux is a different story. We can help with some arcane magic making this useful, but wrapping 'setenforce 0/1' with rlFunction just will not happen, in phase or other places.

Hi,
For me it is not important how it'll be implemented.
As Ales wrote his command. It can be used as well (not disabling everything)
for example something like:

function rlRunSelinuxAllow{
#generate policy and store problems to avc.txt, I dont know hot to do fully automated, some selinux magic here :-)
$1 -> avc.tct
audit2allow -M "$1" < avc.txt
semodule -i smdm.pp
#run command as obvious
rlRun "$1" "$2" "$3"
# store policy name somewhere
touch /tmp/selinux/$PR
}

fuction rlRestoreSelinux{ 
for foo in `ls /tmp/selinux` 
semodule -r "$foo"
}

so function same as rlRun, but it automatically create policy for command you are calling. And If you want, you calls Restore at the end and it removes every created policy.
   Regards
   Honza

I'll probably split this RFE into two:

 - disabling selinux when it blocks some testing
 - disabling AVC test when we deliberately invoke AVC messages and process them ourselves

How many tests does produce unwanted AVCs? What are the real number we are talking about? I am afraid that providing requested functionality would just hide bugs in selinux-policy. If your test is a ordinary usecase (real scenario), you should file a selinux-policy bug regarding that. If you realy want to bypass the error until the bug is fixed in policy, use a custom module or switch to permissive and waive you AVC failures (just like every other failing TC with open bug). If your test is producing AVCs from its nature, you should use a custom selinux module to allow it (or don't audit).

Guys, you should finaly deal with the fact that selinux is now a common security layer in RHEL and stop looking for workarounds. Seems to me that you are more focusing on having your tables with test results green instead of ensuring that RHEL is working well.

Hi,
Number is small, so it will have only small/no impact to selinux testing.
But It is needed to have some std solution for that, than everybody have own workarounds (modules, disable)
   Honza

Actually it has an impact on selinux testing. The advantage of selinux module is that it addresses the particular permissions needed to do the required action and it makes you "think" whether these permissions should be granted by default (policy bug) or not. If you disable AVC logging for some period of time, you would hide also other AVCs unrelated to your problem that appeared at the same time.

The problem with your "not dealing with selinux is a fast way how to write a test for my bug" is that it hides real policy bug. If you would go through selinux-policy bugs, you would realize that there are bugs with trivial reproducers. It makes me think why these bugs were not reported by our QA. Am I really the first person who got the idea to start/stop/restart this service??? "don't deal with selinux ATM" approach might be the answer.

Your comment was:
(In reply to comment #19)
> If your test is a ordinary usecase (real
> scenario), you should file a selinux-policy bug regarding that. If you realy
> want to bypass the error until the bug is fixed in policy, use a custom module
> or switch to permissive and waive you AVC failures (just like every other
> failing TC with open bug).

^ You are absolutely right and of course that's a bug. I fill such a case into
    bugzilla.

> If your test is producing AVCs from its nature, you
> should use a custom selinux module to allow it (or don't audit).

For me its this case, own policy fits only for one rhel release, and with
another minor release its changed again. Its waste of time to write again and
again policy.
Its really expected that avc appears. In other words its fail positive.

> Guys, you should finaly deal with the fact that selinux is now a common
> security layer in RHEL and stop looking for workarounds. Seems to me that you
> are more focusing on having your tables with test results green instead of
> ensuring that RHEL is working well.

Man, this is not any workaround. Its simplification of more steps.
I don't know if your team has priority to have a green tests.

Among the steps to reproduce you need to switch off selinux. There is bug which
is reproducible when selinux is not blocking it.

One of my examples is here:
https://bugzilla.redhat.com/show_bug.cgi?id=319921#c2

Petr's motivation is to avoid test failure because of AVC denial since it is expected (he is actually testing the denial). I think the good solution for this is to have something like AVC whitelist, e.g.
  rlAVCIgnore domainx_t domainy_t:file write
Selinux/AVC test would ignore AVCs on the whitelist. Using this approach you can test particular AVC appearance without ignoring other AVCs that might appear at the same time.

(In reply to comment #23)
if there is such an implemented phase / run in beakerlib it exactly fits my testsuite.

about implementation to beakerlib: is it possible to say to beaker ignore such an AVC denial retrospectively ?

I'm ok with the proposed rlAVCIgnore for reproducing exploits. But wouldn't it require a pair command that will revert the changes? To ensure a proper cleanup? 
Then it doesn't seem like a substantial improvement over the bare

audit2allow -M smdm < avc.txt && semodule -i smdm.pp
...
semodule -r smdm


+0 to this, I guess

(In reply to comment #25)
> I'm ok with the proposed rlAVCIgnore for reproducing exploits. But wouldn't it
> require a pair command that will revert the changes? To ensure a proper
> cleanup? 

Depends whether the "whitelist' should take effect 'till the end of the current section or 'till the end of the whole test. I can imagine both. For the first case the cleanup can be done automatically at the end of the section, for the second case I think cleanup is not necessary at all.

> Then it doesn't seem like a substantial improvement over the bare
> 
> audit2allow -M smdm < avc.txt && semodule -i smdm.pp
> ...
> semodule -r smdm

What Peter is trying to do is to test that the access has been actually denied!, i.e.
1. execute the command
2. Verify the access has been blocked and AVC denial has been logged.
His problem is that the actuall AVC (the one he is checking) is causing the selinux test to fail at the same time.

semodule approach is allowing the access - this is not Peter's request.

Originaly I also understood that the purpose of the request is to allow the access (disable selinux, i.e. permissive mode) but as Petr confirmed on irc, he just doesn't want the selinux test to fail because of some specific denials.

(In reply to comment #6)
> Switching avc checker off is an overkill.

+1

I bet in 99% of cases it'd be enough if each and every false positive reported by Beaker would be fixed in Beaker (and in a timely manner)

I guess psklenar has put me on CC of this bug because I've asked the team (and he had the "solution" for me) how to avoid test reporting fail due to selinux check when in fact nothing fails

writing & applying a policy that allows the test to do what I need to do, then cleaning up properly, is not that hard (wink on jscotka!), it runs fine locally, but when it comes to Beaker ... I read the logs forth and back, back and forth, I see no single denial or any problem, yet still the result is reported as Fail

(In reply to comment #26)
> What Peter is trying to do is to test that the access has been actually
> denied!, i.e.
> 1. execute the command
> 2. Verify the access has been blocked and AVC denial has been logged.
> His problem is that the actuall AVC (the one he is checking) is causing the
> selinux test to fail at the same time.
> 
> semodule approach is allowing the access - this is not Peter's request.

this is valid point - however I do not think it should be achieved via turning of selinux checking but rather via saying somehow "this denial is expected" (and potentially, if requested to, failing the other way round - if that denial doesn't happen)

(In reply to comment #26)
> Depends whether the "whitelist' should take effect 'till the end of the current
> section or 'till the end of the whole test. I can imagine both. For the first
> case the cleanup can be done automatically at the end of the section, for the
> second case I think cleanup is not necessary at all.
sorry, but 
-1 to automatic cleanups happening in rlPhaseEnd (what if I want the effect to survive to the next phase?)
-1 to the effect surviving the whole test. Such a test would not leave the system in the same state it started in. Thus is by definition (well, my definition at least) broken.
  

> What Peter is trying to do is to test that the access has been actually
> denied!, i.e.
I see. In this case temporarily disabling the avc checker (via AVC_ERROR) is (currently & AFAIK) the best method.

The semodule method works for the "need to reproduce a security issue but selinux gets in the way" problem.

(In reply to comment #27)
> writing & applying a policy that allows the test to do what I need to do, then
> cleaning up properly, is not that hard (wink on jscotka!), it runs fine
> locally, but when it comes to Beaker ... I read the logs forth and back, back
> and forth, I see no single denial or any problem, yet still the result is
> reported as Fail
weird, I smell beaker bug

> this is valid point - however I do not think it should be achieved via turning
> of selinux checking but rather via saying somehow "this denial is expected"
> (and potentially, if requested to, failing the other way round - if that denial
> doesn't happen)
I like this. Clean solution. How to get it? RFE on Beaker to provide more fine-grained setting for the avc checker?

(In reply to comment #28)
> (In reply to comment #26)
> > Depends whether the "whitelist' should take effect 'till the end of the current
> > section or 'till the end of the whole test. I can imagine both. For the first
> > case the cleanup can be done automatically at the end of the section, for the
> > second case I think cleanup is not necessary at all.
> sorry, but 
> -1 to automatic cleanups happening in rlPhaseEnd (what if I want the effect to
> survive to the next phase?)
> -1 to the effect surviving the whole test. Such a test would not leave the
> system in the same state it started in. Thus is by definition (well, my
> definition at least) broken.

I am not sure whether we are talking about the same thing. Let me summarize it first. The feature Peter is requesting is not "disable selinux" for particular command but do not let the test fail just because specific AVC appeared. Switching selinux to permissive is a bad idea (it won't hide that AVC anyway) and I believe that switching off AVC check for some period of time is also bad idea too because it might hide different AVC too (and thus hide a possible bug). Simple example: I am writing a test for net-snmp. snmpd is refreshing his cache every minute. If this cache refresh would generate AVC denial during the period when the AVC checker is off, I would miss that (and possibly an important bug too). This is why I don't think that switching AVC checker off is a good idea.

Back to your comments. This is a question of implementation. From my point of view AVC checker could be updated to take into account "AVC whitelist". It should not be difficult to filter out whitelisted AVCs from the "audit.log" and check whether there are any AVCs remaining (=>fail) or not (=>pass). 

Whitelist persistance (Phase vs. Whole test) is a matter of implementation. I can't imagine that beaker developers might have difficulties to implement options like 
rlAVCIgnore --phase ....
rlAVCIgnore --test ...
and choose a default behavior.

AFAIK every test run is keeping it's "runtime" data in a specific directory in /tmp. If you would store whitelist in such directory, you don't need any cleanup at all because following job gets its own directory.

> > What Peter is trying to do is to test that the access has been actually
> > denied!, i.e.
> I see. In this case temporarily disabling the avc checker (via AVC_ERROR) is
> (currently & AFAIK) the best method.

I don't agree because of reasons mentioned above.


> > this is valid point - however I do not think it should be achieved via turning
> > of selinux checking but rather via saying somehow "this denial is expected"
> > (and potentially, if requested to, failing the other way round - if that denial
> > doesn't happen)
> I like this. Clean solution. How to get it? RFE on Beaker to provide more
> fine-grained setting for the avc checker?

This is actually the goal of my rlAVCIgnore function. :-P

PS: I am not discussing anymore the question of overriding default selinux policy (e.g. via switching to permissive or loading a custom module) because this is not what Petr is trying to address in this bug.

This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.

There is a library(selinux-policy/common) which concentrates SELinux functions. It is natural to request it to be implemented there.

Feel free to reopen this bug if you want me to close it again, most probably. :-)