-
-
Notifications
You must be signed in to change notification settings - Fork 148
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
BREAKING: Refactor selinux::module #189
Changes from 6 commits
11833ca
f94da68
c6257f2
1512735
555297b
8da2298
1f91bc7
8c9ef4e
d10fa4d
1df7b55
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,15 @@ running system. | |
* Mailinglist: <[email protected]> | ||
([groups.io Webinterface](https://groups.io/g/voxpupuli/topics)) | ||
|
||
## Upgrading from puppet-selinux 0.8.x | ||
|
||
* Previously, module building always used the refpolicy framework. The default | ||
module builder is now 'simple', which uses only checkmodule. Not all features are | ||
supported with this builder. | ||
|
||
To build modules using the refpolicy framework like previous versions did, | ||
specify the 'refpolicy' builder either explicitly per module or globally | ||
via the main class | ||
|
||
## Known problems / limitations | ||
|
||
|
@@ -51,8 +60,6 @@ running system. | |
does) the order is important. If you add /my/folder before /my/folder/subfolder | ||
only /my/folder will match (limitation of SELinux). There is no such limitation | ||
to file-contexts defined in SELinux modules. (GH-121) | ||
* `selinux::module` only allows to add a type enforcment file (`*.te`) but no | ||
interfaces (`*.if`) or file-contexts (`*.fc`). | ||
* While SELinux is disabled the defined types `selinux::boolean`, | ||
`selinux::fcontext`, `selinux::port` will produce puppet agent runtime errors | ||
because the used tools fail. | ||
|
@@ -97,12 +104,15 @@ are `target`, `minimum`, and `mls`). Note that disabling SELinux requires a rebo | |
to fully take effect. It will run in `permissive` mode until then. | ||
|
||
|
||
### Deploy a custom module | ||
### Deploy a custom module using the refpolicy framework | ||
|
||
```puppet | ||
selinux::module { 'resnet-puppet': | ||
ensure => 'present', | ||
source => 'puppet:///modules/site_puppet/site-puppet.te', | ||
ensure => 'present', | ||
source_te => 'puppet:///modules/site_puppet/site-puppet.te', | ||
source_fc => 'puppet:///modules/site_puppet/site-puppet.fc', | ||
source_if => 'puppet:///modules/site_puppet/site-puppet.if', | ||
builder => 'refpolicy' | ||
} | ||
``` | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
#!/bin/sh | ||
module_name="$1" | ||
set -e | ||
checkmodule -M -m -o ${module_name}.mod ${module_name}.te | ||
package_args="-o ${module_name}.pp -m ${module_name}.mod" | ||
if [ -f "${module_name}.fc" ]; then | ||
package_args="${package_args} --fc ${module_name}.fc" | ||
fi | ||
|
||
semodule_package ${package_args} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
require 'puppet' | ||
Facter.add(:selinux_agent_vardir) do | ||
setcode do | ||
Puppet.settings['vardir'] | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,29 +14,33 @@ | |
# @param type sets the selinux type | ||
# Default value: undef | ||
# Allowed values: (targeted|minimum|mls|undef) | ||
# @param sx_mod_dir directory where to store puppet managed selinux modules | ||
# Default value: /usr/share/selinux | ||
# Allowed values: absolute path | ||
# @param makefile the path to the systems SELinux makefile | ||
# @param makefile the path to the system's SELinux makefile | ||
# Default value: /usr/share/selinux/devel/Makefile | ||
# Allowed value: absolute path | ||
# @param manage_package manage the package for selinux tools | ||
# @param manage_package manage the package for selinux tools and refpolicy | ||
# Default value: true | ||
# @param package_name sets the name for the selinux tools package | ||
# Default value: OS dependent (see params.pp) | ||
# @param refpolicy_package_name sets the name for the refpolicy development package, required for the | ||
# refpolicy module builder | ||
# Default value: OS dependent (see params.pp) | ||
# @param default_builder which builder to use by default with selinux::module | ||
# Default value: refpolicy | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. default is simple isn't it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a leftover when I had refpolicy as the default earlier, but I think simple is better because it has fewer dependencies. |
||
# @param boolean Hash of selinux::boolean resource parameters | ||
# @param fcontext Hash of selinux::fcontext resource parameters | ||
# @param module Hash of selinux::module resource parameters | ||
# @param permissive Hash of selinux::module resource parameters | ||
# @param port Hash of selinux::port resource parameters | ||
# | ||
class selinux ( | ||
$mode = $::selinux::params::mode, | ||
$type = $::selinux::params::type, | ||
$sx_mod_dir = $::selinux::params::sx_mod_dir, | ||
$makefile = $::selinux::params::makefile, | ||
$manage_package = $::selinux::params::manage_package, | ||
$package_name = $::selinux::params::package_name, | ||
$mode = $::selinux::params::mode, | ||
$type = $::selinux::params::type, | ||
$makefile = $::selinux::params::refpolicy_makefile, | ||
$manage_package = $::selinux::params::manage_package, | ||
$package_name = $::selinux::params::package_name, | ||
$refpolicy_package_name = $::selinux::params::refpolicy_package_name, | ||
$module_build_root = $::selinux::params::module_build_root, | ||
$default_builder = 'simple', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's a good choice that the default is the one with the least required packages. |
||
|
||
### START Hiera Lookups ### | ||
$boolean = undef, | ||
|
@@ -58,7 +62,6 @@ | |
default => 'undef', | ||
} | ||
|
||
validate_absolute_path($sx_mod_dir) | ||
validate_re($mode_real, ['^enforcing$', '^permissive$', '^disabled$', '^undef$'], "Valid modes are enforcing, permissive, and disabled. Received: ${mode}") | ||
validate_re($type_real, ['^targeted$', '^minimum$', '^mls$', '^undef$'], "Valid types are targeted, minimum, and mls. Received: ${type}") | ||
validate_string($makefile) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,75 +9,114 @@ | |
# | ||
# @example compile and load the apache module | ||
# selinux::module{ 'apache': | ||
# ensure => 'present', | ||
# source => 'puppet:///modules/selinux/apache.te', | ||
# ensure => 'present', | ||
# source_te => 'puppet:///modules/selinux/apache.te', | ||
# builder => 'simple' | ||
# } | ||
# | ||
# @param ensure present or absent | ||
# @param sx_mod_dir path where source is stored and the module built. | ||
# Valid values: absolute path | ||
# @param source the source file (either a puppet URI or local file) of the SELinux .te file | ||
# @param content content of the source .te file | ||
# @param makefile absolute path to the selinux-devel Makefile | ||
# @param prefix (DEPRECATED) the prefix to add to the loaded module. Defaults to ''. | ||
# Does not work with CentOS >= 7.2 and Fedora >= 24 SELinux tools. | ||
# @param source_te the source file (either a puppet URI or local file) of the SELinux .te file | ||
# @param source_fc the source file (either a puppet URI or local file) of the SELinux .fc file | ||
# @param source_if the source file (either a puppet URI or local file) of the SELinux .if file | ||
# @param builder either 'simple' or 'refpolicy'. The simple builder attempts to use checkmodule | ||
# to build the module, whereas 'refpolicy' uses the refpolicy framework, but requires 'make' | ||
# @param syncversion selmodule syncversion param | ||
define selinux::module( | ||
$source = undef, | ||
$content = undef, | ||
$ensure = 'present', | ||
$makefile = '/usr/share/selinux/devel/Makefile', | ||
$prefix = '', | ||
$sx_mod_dir = '/usr/share/selinux', | ||
Optional[String] $source_te = undef, | ||
Optional[String] $source_fc = undef, | ||
Optional[String] $source_if = undef, | ||
Enum['absent', 'present'] $ensure = 'present', | ||
Optional[Enum['simple', 'refpolicy']] $builder = undef, | ||
$syncversion = undef, | ||
) { | ||
|
||
if $builder == 'refpolicy' { | ||
require ::selinux::refpolicy_package | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we maybe make this But maybe at all not important and just a personal opinion of mine. :) |
||
} | ||
|
||
include ::selinux | ||
|
||
if ($builder == 'simple' and $source_if != undef) { | ||
fail("The simple builder does not support the 'source_if' parameter") | ||
} | ||
|
||
# let's just make doubly sure that this is an absolute path: | ||
validate_absolute_path($::selinux::config::module_build_dir) | ||
|
||
$module_dir = "${::selinux::config::module_build_dir}/${title}" | ||
$module_file = "${module_dir}/${title}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If one manages multiple refpolicy style policies, lets call them A and B, with *.if files and A depends on an interface provided by B's *.if I don't know if they require to be in the same directory so that the makefile finds the interfaces. this path creates a directory for every module. Is this a requirement? There should IMHO never be a same named policy source file. With newer (>= CentOS 7.3.) the refpolicy style will fail if filename and resulting policy name are not the same. |
||
|
||
$build_command = pick($builder, $::selinux::default_builder, 'none') ? { | ||
'simple' => shellquote("${::selinux::config::module_build_dir}/selinux_build_module.sh", $title), | ||
'refpolicy' => shellquote('make', '-f', '/usr/share/selinux/devel/Makefile', "${title}.pp"), | ||
'none' => fail('No builder or default builder specified') | ||
} | ||
|
||
Anchor['selinux::module pre'] -> | ||
Selinux::Module[$title] -> | ||
Anchor['selinux::module post'] | ||
$has_source = (pick($source_te, $source_fc, $source_if, false) != false) | ||
|
||
validate_re($ensure, [ '^present$', '^absent$' ], '$ensure must be "present" or "absent"') | ||
if $ensure == 'present' and $source == undef and $content == undef { | ||
fail("You must provide 'source' or 'content' field for selinux module") | ||
} | ||
if $source != undef { | ||
validate_string($source) | ||
} | ||
if $content != undef { | ||
validate_string($content) | ||
} | ||
validate_string($prefix) | ||
validate_absolute_path($sx_mod_dir) | ||
validate_absolute_path($makefile) | ||
if $syncversion != undef { | ||
validate_bool($syncversion) | ||
} | ||
if $has_source and $ensure == 'present' { | ||
file {$module_dir: | ||
ensure => directory, | ||
} | ||
|
||
if $source_te { | ||
file {"${module_file}.te": | ||
ensure => 'file', | ||
source => $source_te, | ||
notify => Exec["clean-module-${title}"], | ||
} | ||
} | ||
if $source_fc { | ||
file {"${module_file}.fc": | ||
ensure => 'file', | ||
source => $source_fc, | ||
notify => Exec["clean-module-${title}"], | ||
} | ||
} | ||
if $source_if { | ||
file {"${module_file}.if": | ||
ensure => 'file', | ||
source => $source_if, | ||
notify => Exec["clean-module-${title}"], | ||
} | ||
} | ||
exec { "clean-module-${title}": | ||
path => '/bin:/usr/bin', | ||
cwd => $module_dir, | ||
command => "rm -f '${title}.pp' loaded", | ||
refreshonly => true, | ||
notify => Exec["build-module-${title}"], | ||
} | ||
|
||
## Begin Configuration | ||
file { "${sx_mod_dir}/${prefix}${name}.te": | ||
ensure => $ensure, | ||
owner => 'root', | ||
group => 'root', | ||
mode => '0644', | ||
source => $source, | ||
content => $content, | ||
exec { "build-module-${title}": | ||
path => '/bin:/usr/bin', | ||
cwd => $module_dir, | ||
command => "${build_command} || (rm -f ${title}.pp loaded && exit 1)", | ||
creates => "${module_file}.pp", | ||
notify => Exec["install-module-${title}"], | ||
} | ||
# we need to install the module manually because selmodule is kind of dumb. It ends up | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what kind of dumb? can you elaborate more? Should we file a bug on the puppetlabs jira? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's because of the syncversion bug, I think. It can only check that the module is loaded, and if you recompile a module that's loaded, it won't actually detect the change. Installing it manually is a workaround for that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, the lack of an AVC might be because the files are in the libdir which contains executables anyway (ie. facts and such). The policy seems to allow puppetagent_t to do quite a lot, so might be that it just has permission to execute whatever is in the libdir |
||
# working fine, though. | ||
exec { "install-module-${title}": | ||
path => '/sbin:/usr/sbin:/bin:/usr/bin', | ||
cwd => $module_dir, | ||
command => "semodule -i ${title}.pp && touch loaded", | ||
creates => "${module_dir}/loaded", | ||
before => Selmodule[$title], | ||
} | ||
} | ||
~> | ||
exec { "${sx_mod_dir}/${prefix}${name}.pp": | ||
# Only allow refresh in the event that the initial .te file is updated. | ||
command => shellquote('make', '-f', $makefile, "${prefix}${name}.pp"), | ||
path => '/bin:/sbin:/usr/bin:/usr/sbin', | ||
refreshonly => true, | ||
cwd => $sx_mod_dir, | ||
$module_path = $has_source ? { | ||
true => "${module_file}.pp", | ||
false => undef | ||
} | ||
-> | ||
selmodule { $name: | ||
|
||
selmodule { $title: | ||
# Load the module if it has changed or was not loaded | ||
# Warning: change the .te version! | ||
ensure => $ensure, | ||
selmodulepath => "${sx_mod_dir}/${prefix}${name}.pp", | ||
syncversion => $syncversion, | ||
selmodulepath => $module_path, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this file needs
bin_t
seltype or puppet agent will maybe throw SELinux AVC's when it executes?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should review seltypes of the directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get any SELinux errors running the module, and I run my desktop in enforcing mode, so it should be fine. The acceptance tests also seemed fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even if the puppet agent process runs as
puppetagent_t
type it does not produce an AVC when it executes the selinux_build_module.sh script which has typeusr_t
. so this is fine. I just wonder why it does not.by default the puppet agent process even does run in
unconfinged_service_t
(orinitrc_
t) because the paths in the refpolicy were not adapted to the /opt/puppetlabs change.