Bug 669665 - Beaker avc subtest reports false positives (run tasks as unconfined)
Summary: Beaker avc subtest reports false positives (run tasks as unconfined)
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Beaker
Classification: Retired
Component: beah
Version: 0.6
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
Assignee: Marian Csontos
QA Contact:
URL:
Whiteboard:
: 667747 (view as bug list)
Depends On:
Blocks: 545868 623920 667986
TreeView+ depends on / blocked
 
Reported: 2011-01-14 11:38 UTC by Šimon Lukašík
Modified: 2019-05-22 13:37 UTC (History)
8 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2011-01-28 09:30:29 UTC
Embargoed:


Attachments (Terms of Use)
example log of avc subtest (1.87 KB, text/plain)
2011-01-14 11:39 UTC, Šimon Lukašík
no flags Details

Description Šimon Lukašík 2011-01-14 11:38:17 UTC
Description of problem:
After most recent upgrade, beaker started to report false positives
on avc denials. Or maybe the ping command runs within different context.

Version-Release number of selected component (if applicable):

How reproducible:
always

Steps to Reproduce:
1. Do following in your test:
2. ping -c 3 -q $( hostname ) &> $( mktemp )
3.
  
Actual results:
When you run the command in the test, the avc subtest fails,
although when you run the command on a machine manually
none message is generated to the audit log.

Expected results:
Avc subtest should pass.

Additional info:
Note the success=yes in the log.

Comment 1 Šimon Lukašík 2011-01-14 11:39:05 UTC
Created attachment 473504 [details]
example log of avc subtest

Comment 3 Šimon Lukašík 2011-01-14 12:56:22 UTC
Seems that ping command runs in different selinux context, when runned
within the test compared to the manual run.

After review, feel free to close as notabug. Sorry for noise.

Comment 4 Marian Csontos 2011-01-14 12:57:45 UTC
Bill, is this a result of the change in selinux policy for rhts package which removed /tmp rules [1] ?

Though we should not use /tmp it is used by pretty every beakerlib test as there is a `mktemp` in rhts-wizard generated template.

We could try to change TMPDIR but I am sure that is not always desirable as there are scenarios which really do require TMPDIR == /tmp: e.g. selinux would not catch genuine AVC's related to writing to /tmp. And I am sure there are other use cases.

[1] http://git.fedorahosted.org/git/?p=rhts.git;a=blobdiff;f=selinux/rhts.te;h=902cce83f9b98a79781523679e8fab39b03ec7ba;hp=d42410cfeb61e5a206dc4a0c926f9d71fba2aacd;hb=0648fd41f89b7d3d104c33df7167eee8590b83cb;hpb=39d60c7281b42ba199af71ce36621b6c209fbaa5

Comment 5 Bill Peck 2011-01-14 13:45:05 UTC
Hi Marian,

Yes, this is probably related to removing the rule from rhts-test-env which allowed init_r to write to tmp_t.

I could put it back, but I think we really need to look hard at how to run the tests like a login user.  

It must be possible to do the following when launching a test from beah.

1 - switch selinux context to unconfined_u ( I believe that is what a normal user is
2 - use a login shell


There is runcon to change the selinux context.  I may have to update our policy to allow us to do those transitions.

The goal here is to act the same as a user running the test.  And then it would be ok for the test to write to /tmp since it would be a user and not a system daemon.

Comment 6 Marian Csontos 2011-01-14 14:22:14 UTC
Definitely a way to go! Though this is compatible with rhts, we need a way to change policy in the tests. IMO we need to update harness policy to allow for that.

And as beah has no policy yet could we have updated the rhts policy for now to simple use unconfined?

This would likely solve most of the SElinux problems and there is a few of them.

Comment 7 Bill Peck 2011-01-14 15:03:32 UTC
First thing to do would be to issue a runcon from beah I believe.  The default policy may allow it to just work.  If we get a denial I can use audit2allow to create a rule to allow it to work.   But as it stands now, the policy itself will not make it magically happen.  beah would have to do it.

Do you understand?

Comment 8 Jan Hutař 2011-01-14 15:03:48 UTC
What is described in comment #5 would be great!

Could we get that fix mentioned in comment #4 reverted in the mean time as a Beaker hotfix?

Comment 9 Jan Hutař 2011-01-14 15:05:37 UTC
Ad comment #7: when we want to run some command in unconfined_t (as root) and not in init_t (as beaker test), we are using this function:

function rhn_helper_run_with_selinux() {
  local command="$1"
  local expected=${2:-0}
  local coment=$3
  if ! getenforce | grep -q Disabled; then
    # Determine correct SELinux context for runcon
    local suser='root'
    local srole='system_r'
    local stype='unconfined_t'
    local additional='-l s0'
    if ( grep -q 'Red Hat Enterprise Linux' /etc/redhat-release && [ $( rlGetDistroRelease | tail -n 1 ) -ge 6 ] ) || \
       ( grep -q 'Fedora' /etc/redhat-release && [ $( rlGetDistroRelease | tail -n 1 ) -ge 12 ] ); then
      suser='unconfined_u'
      srole='unconfined_r'
      additional='-l s0-s0:c0.c1023'
    fi
    if grep -q 'Red Hat Enterprise Linux' /etc/redhat-release && [ $( rlGetDistroRelease | tail -n 1 ) -le 4 ]; then
      additional=''
    fi
    local runcon_comm="runcon -u $suser -r $srole -t $stype $additional"
    # Run command with SELinux context of the root
    rlRun "$runcon_comm -- $command" "$expected" "$coment"
  else
    rlRun "$command" "$expected" "$coment"
  fi
}

Comment 10 Bill Peck 2011-01-14 15:39:22 UTC
I'm building an updated rhts-test-env package with the selinux changes reverted.

I'll let you know when its ready for testing.

Comment 11 Daniel Walsh 2011-01-14 17:00:43 UTC
if [ selinuxenabled ]  would be better

Comment 12 Marian Csontos 2011-01-17 11:58:01 UTC
Sorry for being lazy *** and asking for policy changes without actually trying it out: I did not expect that, but runcon works fine without any policy changes :-)

Thanks Bill for kicking me off.

Thanks Jan for you code.

I will try to get it tested and included in next update.

Comment 13 Marian Csontos 2011-01-17 19:06:05 UTC
Seems to work fine on EL5 and 6, will upload to stage soon.

Comment 14 Marian Csontos 2011-01-20 05:41:34 UTC
Here's the job with beta harness from beaker-stage:

  https://beaker.engineering.redhat.com/jobs/47341

There is still one AVC error but this looks like a genuine failure. Simon, could you have a look, please?

Comment 15 Šimon Lukašík 2011-01-20 07:12:48 UTC
Hello Marian,
I am sorry, but we cannot use this (cloning sat540 recipe) as verification,
in the tests there are already workarounds(*) for the denial we spoke about.

If You can point me to new packages, I can do manual verification.

(*) 1) Our own policy module
    2) ping runs unconfined now

Comment 16 Marian Csontos 2011-01-20 09:02:07 UTC
See the repo element in the recipe. All required packages are there...

Be sure to pick the correct distro and architecture.
See dirs at https://beaker-stage.app.eng.bos.redhat.com/harness/

Specifically you will need rhts-4.29 and beah-0.6.20-1.git.18.*

Comment 17 Šimon Lukašík 2011-01-21 11:04:43 UTC
Thanks, I have cloned your job, and manually removed workarounds, 
and it seems to be fine.

  https://beaker.engineering.redhat.com/jobs/47641

None of avc subtests failed.

Comment 19 Marian Csontos 2011-01-24 13:40:56 UTC
== patch1: Run rhts tasks in unconfined context ==

http://git.fedorahosted.org/git/?p=beah.git;a=commitdiff;h=fd1d1e5fecba1fdad875ea332eea272908c694e6

The tasks should execute within the same context as when executed with
'make run' and unconfined seems a good default.

Changing context for started services should be handled by policy.

=== Comment ===

This is adapted from beakerlib, but I wanted avoid another dependency...

== patch2: Fixed path ==

http://git.fedorahosted.org/git/?p=beah.git;a=commitdiff;h=4723316aeeec912bca55962523a7628fb71cc059

=== Comment ===

No comment is necessary... I left in relative path... :-/


== patch3: Fixed redhat-release for el6. ==

http://git.fedorahosted.org/git/?p=beah.git;a=commitdiff;h=ac456b81a8d6ab8f06647d9c1a31966b31b61d1d

=== Comment ===

el6 does not have redhat-release installed by default, but may have a
redhat-release-server installed which does not work well with original
mechanism.

Comment 20 Marian Csontos 2011-01-27 07:11:08 UTC
*** Bug 667747 has been marked as a duplicate of this bug. ***

Comment 21 Jan Pazdziora (Red Hat) 2011-01-27 08:01:24 UTC
(In reply to comment #19)
> == patch3: Fixed redhat-release for el6. ==
> 
> http://git.fedorahosted.org/git/?p=beah.git;a=commitdiff;h=ac456b81a8d6ab8f06647d9c1a31966b31b61d1d
> 
> === Comment ===
> 
> el6 does not have redhat-release installed by default, but may have a
> redhat-release-server installed which does not work well with original
> mechanism.

I'm not completely clear what the intent here is but you can use

  # rpm -q --whatprovides redhat-release

on both RHEL 5, RHEL 6, and on Fedoras to get the name and version of the OS you are running on.

Comment 22 Marian Csontos 2011-01-27 11:13:36 UTC
(In reply to comment #21)
> (In reply to comment #19)
> > == patch3: Fixed redhat-release for el6. ==
> > 
> > http://git.fedorahosted.org/git/?p=beah.git;a=commitdiff;h=ac456b81a8d6ab8f06647d9c1a31966b31b61d1d
> > 
> > === Comment ===
> > 
> > el6 does not have redhat-release installed by default, but may have a
> > redhat-release-server installed which does not work well with original
> > mechanism.
> 
> I'm not completely clear what the intent here is but you can use
> 
>   # rpm -q --whatprovides redhat-release
> 
> on both RHEL 5, RHEL 6, and on Fedoras to get the name and version of the OS
> you are running on.

Thanks, looks much better.

Will queue this patch for 0.6.4 - we do not want it to delay 0.6.3 and it's harmless.


Note You need to log in before you can comment on or make changes to this bug.