opendev
/
puppet-cgit
Code Issues Proposed changes

Fix logic in selinux execs 

Without this patch, the logic for managing selinux rules faces two
problems:

1. The use of the refreshonly is problematic. If for whatever reason
   the semanage command fails or is not executed in the course of a puppet
   run, a second puppet run can only fix the selinux problem if it is also
   changing the state of the file resource to which the exec is
   subscribed. If there is no change made to that file, puppet will not
   attempt to re-execute the semanage command and the rule will remain
   broken but unreported.

2. Using a system-modifying command as a value to the onlyif or unless
   parameters is bad practice. If the command in the onlyif fails (or
   if the command in the unless succeeds), the command in the command
   parameter will not be executed so puppet will report no changes,
   even though a change has occurred. The onlyif or unless parameters
   are intended to examine the state of the system to determine whether
   an action is needed, never to modify the system.

This patch removes the refreshonly parameters from the execs in
cgit::selinux in order to fix problem 1. This alone exacerbates problem
2 because when the exec is not tied to a file resource it always fails
to add the port after the first time, and so reports modifying the port
on every run. To fix this, this patch changes the onlyif to an unless
that examines whether the desired rule exists, and if not first tries
to add the port and then to modify the port if the port was already
added.

Change-Id: I98fa561b5367cd5fe11ff61479aa8b899db07a5a
Depends-On: I9d359b3fc71c7a83b6094f7ee535ab8418f20468
Depends-On: Iaa9c8cda7a2eae904eb8f25cfa33be249b2b4cab
This commit is contained in:
Colleen Murphy 2015-07-20 14:02:58 -07:00
parent 6c36488c23
commit 08457a8e9e
1 changed files with 14 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
manifests/selinux.pp
View File

@ -28,28 +28,26 @@ class cgit::selinux {
}
exec { 'cgit_allow_http_port':
# If we cannot add the rule modify the existing rule.
onlyif => "bash -c \'! semanage port -a -t http_port_t -p tcp ${::cgit::http_port}\'",
command => "semanage port -m -t http_port_t -p tcp ${::cgit::http_port}",
path => '/bin:/usr/sbin',
before => Service['httpd'],
subscribe => File['/etc/httpd/conf/httpd.conf'],
refreshonly => true,
unless => "semanage port -l | grep \'http_port_t.*tcp.*${::cgit::http_port}\'",
command => "semanage port -a -t http_port_t -p tcp ${::cgit::http_port} \
|| semanage port -m -t http_port_t -p tcp ${::cgit::http_port}",
path => '/bin:/usr/sbin',
before => Service['httpd'],
subscribe => File['/etc/httpd/conf/httpd.conf'],
}
exec { 'cgit_allow_https_port':
# If we cannot add the rule modify the existing rule.
onlyif => "bash -c \'! semanage port -a -t http_port_t -p tcp ${::cgit::https_port}\'",
command => "semanage port -m -t http_port_t -p tcp ${::cgit::https_port}",
path => '/bin:/usr/sbin',
subscribe => File['/etc/httpd/conf.d/ssl.conf'],
refreshonly => true,
unless => "semanage port -l | grep \'http_port_t.*tcp.*${::cgit::https_port}\'",
command => "semanage port -a -t http_port_t -p tcp ${::cgit::https_port} \
|| semanage port -m -t http_port_t -p tcp ${::cgit::https_port}",
path => '/bin:/usr/sbin',
subscribe => File['/etc/httpd/conf.d/ssl.conf'],
}
exec { 'cgit_allow_git_daemon_port':
# If we cannot add the rule modify the existing rule.
onlyif => "bash -c \'! semanage port -a -t git_port_t -p tcp ${::cgit::daemon_port}\'",
command => "semanage port -m -t git_port_t -p tcp ${::cgit::daemon_port}",
unless => "semanage port -l | grep \'git_port_t.*tcp.*${::cgit::daemon_port}\'",
command => "semanage port -a -t git_port_t -p tcp ${::cgit::daemon_port} \
|| semanage port -m -t git_port_t -p tcp ${::cgit::daemon_port}",
path => '/bin:/usr/sbin',
before => Service[$::cgit::git_daemon_service_name],
subscribe => File['git-daemon-init-script'],