Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SELinux policies for plugins #15

Closed
wants to merge 7 commits into from
Closed

Conversation

lzap
Copy link
Member

@lzap lzap commented Mar 3, 2014

Do not merge yet - I will be adding policies for all our plugins in this PR.
Please review.

@lzap lzap changed the title Fixes #4527 - websockify rules SELinux policies for plugins Apr 25, 2014
@lzap
Copy link
Member Author

lzap commented Apr 25, 2014

Renamed this PR to "SELinux policies for plugins".

I am done with this, @domcleal

miscfiles_read_localization(websockify_t)
sysnet_read_config(websockify_t)

# Prevent websockify from contacting ABRT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that to stop the exceptions from websockify from propagating into ABRT? Bit hacky doing it here if so ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is websockify throws a python stracktrace everytime we try to connect to occupied port. And this is how Foreman works - it loops the ports until websockify starts ok. The problem is that python on RHEL6 is configured with ABRT integration by default, therefore every single try causes ABRT run, which is also quite slow.

I was not able to blacklist websockify, after some discussion with ABRT folks, I have decided this trick to mute that guy. The process is not able to connect to ABRT.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm aware... but I don't think adding SELinux policy is the way to fix that, especially as it doesn't help in non-enforcing configurations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm aware... but I don't think adding SELinux policy is the way to fix that, especially as it doesn't help in non-enforcing configurations.

But we need to add it here anyway. We can either do dontaudit or allow.
From the two options, dontaudit makes more sense - it also solves the
problem.

ABRT features blacklisting, but that works on a package level. Therefore
on a Foreman installation, we can initiate ABRT investigation 10 times
per click.

To me dontaudit looks like the better option than allow.

Yes I admit proper fix should be done. We can either:

  1. Drop BlackListedPaths in /etc/abrt/abrt-action-save-package-data.conf
    (but when I initially tested this it was not working). We need augeas
    to do this.

  2. Send upstream patch not to bubble up the main method to websockify.

I think this is a different thing, we should raise another issue/pr for
that.

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's use the original issue number #4527 for investigation into that. I'd prefer to stop it triggering the exception, frankly.

However I think if there's a macro that permits access to ABRT we should actually use this and then people can choose to log the exceptions by changing the config, particularly if we stop it triggering exceptions while finding a free port. This will be more useful in the long term.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping?

@domcleal
Copy link
Contributor

Do we have something to permit passenger_t to execute websockify_exec_t, or does it have free reign to execute any label right now?

@lzap
Copy link
Member Author

lzap commented Apr 25, 2014

It has quite loose rights on execution, yeah.

[root@nightly ~]# sesearch -s passenger_t -t websockify_exec_t --all
ERROR: Cannot get avrules: Neverallow rules requested but not available
Found 1 semantic av rules:
   allow passenger_t websockify_exec_t : file { read getattr execute open } ; 

Found 1 semantic te rules:
   type_transition passenger_t websockify_exec_t : process websockify_t; 

@lzap
Copy link
Member Author

lzap commented Apr 25, 2014

But this is covered by

application_domain(websockify_t, websockify_exec_t)
domtrans_pattern(passenger_t, websockify_exec_t, websockify_t)

@domcleal
Copy link
Contributor

Ah yes, thanks, I missed that.

@domcleal
Copy link
Contributor

I don't think that #5446 is our bug, it should be handled in the distribution policy.

(From my reading of the AVC, it's not the puppetmaster at all, it's just Postfix searching a dir it probably shouldn't be.)

@lzap
Copy link
Member Author

lzap commented Apr 25, 2014

On Fri, Apr 25, 2014 at 05:48:11AM -0700, Dominic Cleal wrote:

I don't think that #5446 is our bug, it should be handled in the distribution policy.

But we already carry bunch of fixes there.

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

@lzap
Copy link
Member Author

lzap commented Apr 25, 2014

Ok I have reworded issue number for the websockify, sorry. You check for this as well? Uh.

@domcleal
Copy link
Contributor

We don't carry any fixes for Postfix, do we - assuming as I said, that I'm reading the AVC correctly?

@domcleal
Copy link
Contributor

Issue number in the websockify commit still seems to point to #4527 rather than #4569?

@lzap
Copy link
Member Author

lzap commented Apr 25, 2014

Youre right, I missed that tiny detail. Dropped.

@lzap
Copy link
Member Author

lzap commented Apr 28, 2014

Hmm reworded incorrectly.

Thanks.

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

@lzap
Copy link
Member Author

lzap commented Apr 28, 2014

novnc/websockify#126

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

@lzap
Copy link
Member Author

lzap commented Apr 28, 2014

Added one more fix

@lzap
Copy link
Member Author

lzap commented Apr 28, 2014

Ok pushed, review, then I will squash.

@domcleal
Copy link
Contributor

👍 thanks!

@lzap
Copy link
Member Author

lzap commented Apr 28, 2014

Squashed

@domcleal
Copy link
Contributor

My F19 scratch build is failing: http://koji.katello.org/koji/getfile?taskID=103204&name=build.log

Compiling targeted foreman module
foreman.te":280:ERROR 'type bin_t is not within scope' at token ';' on line 7575:
typealias bin_t alias foreman_hook_t;
/usr/bin/checkmodule:  error(s) encountered while parsing configuration
/usr/bin/checkmodule:  loading policy configuration from tmp/foreman.tmp

# Foreman Hooks plugin
#

typealias bin_t alias foreman_hook_t;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we just missing bin_t from requires?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were, fixed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about that, thanks.

I am currently enjoying quite interesting gdb session :-)

Later,

Lukas "lzap" Zapletal
irc: lzap #theforeman

@domcleal
Copy link
Contributor

Merged as c446dc8..0e094fe for 1.5.0. Thanks very much @lzap!

@domcleal domcleal closed this Apr 28, 2014
@lzap lzap deleted the plugins branch August 6, 2014 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants