BREAKING: Redesign selinux::module parameters

[vinzent]

Current situation

• it only allows to build modules with only *.te files. no *.if and .fc extensions are possible
• it does not allow to install an already compiled module
• with some policies it's not needed to install make and co. it may install too much software not required and not wishfull on some systems.

Proposed interface

# only minimal module building (checkmodule, semodule_package)
selinux::module { 'mymodule':
  builder => 'minimal',
  source_te => 'puppet:///modules/profile/mymodule.te',
} 

# this will install additional required packages
# and build with make -f /usr/share/selinux/devel/Makefile
selinux::module { 'mymodule':
  ensure => 'present',
  builder => 'makefile', # DEFAULT 
  source_te => 'puppet:///modules/profile/mymodule.te',
  source_if => 'puppet:///modules/profile/mymodule.if',
  source_fc => 'puppet:///modules/profile/mymodule.fc',
} 

# no compiling at all, only loading an already compiled binary
selinux::module { 'mymodule':
  source_pp => 'puppet:///modules/profile/mymodule.pp',
} 

# remove a module
selinux::module { 'mymodule':
  ensure => 'absent',
} 

selinux::module { 'mymodule':
  ensure => 'disabled',
} 

Things to consider

• newer semodule utilities have the ability to specify a module priority (-X,--priority=PRIORITY) which isn't supported by the selmodule type. semodule defaults to priority 400 if nothing is defined (seen on EL7, Fedora 25)
• semodule provides the ability to disable and enable modules which the selmodule type does not support, valid usecase was given in Add the ability to *disable* an existing module #60
• Ensure puppet agent throws errors if build or load of modules fails . It must fail every run until build and loading of the module succeeds

[vinzent]

@rnelson0 and @oranenj - your opinions?

[oranenj]

Why not just a source_dir parameter? I think for the simple cases we should allow source_te perhaps as well, but having to specify everything separately is not good either, I think.

[vinzent]

Why not just a source_dir parameter? I think for the simple cases we should allow source_te perhaps as well, but having to specify everything separately is not good either, I think.

I personally have all my custom modules in 1 folder. this would sync all my modules at once.

[vinzent]

@oranenj - tried to write your proposal down as code.

selinux::module { 'mymodule':
  ensure => 'present',
  builder => 'makefile', # DEFAULT 
  source_dir => 'puppet:///modules/profile/mymodule',
} 

• how would I distinct if a user wants to load a pre-compiled module?
• will it load and build one module or all modules in dir?
• howto ensure the content of the directory is named as expected?
• if we stop managing the directory, do we remove it on the system?

what about dedicated defined types?

selinux::module_build::dir { 'mymodule':
  ensure => 'present',
  builder => 'makefile', # DEFAULT 
  source => 'puppet:///modules/profile/mymoduledir',
} 

selinux::module_build { 'mymodule':
  ensure => 'present',
  builder => 'makefile', # DEFAULT 
  source  => 'puppet:///modules/profile/mymodule.te',
  source_fc => 'puppet:///modules/profile/mymodule.fc',
  source_if => 'puppet:///modules/profile/mymodule.if',
} 

selinux::module { 'mymodule':
  ensure => 'present',
  # if we wanted a default for source, we needed to ensure that all module_build
  # builded modules reside at an expected place.
  source => ' puppet:///modules/profile/mymodule.pp',
} 

seems bloated to me.

Still find the dedicated param per file the better option. I can easily validate the parameter, save the files with expected names, decide what the user wants (compile+load or just load). And the parameter names are descriptive/indicative what the content should be (at least IMHO).

[TJM]

I agree, and like how this looks. We have had the need to setup custom policies as RPMs due to the need to keep "make" and company off our production servers. This would be nice. It does occur to me, that in addition, there should be a way to ensure the version, somehow, but maybe that would be implemented by the type/provider?

I can see the case where the policy is updated on the puppet server and the update needs to be propagated to the servers that it is installed on. Maybe RPM's are not such a bad idea, after all? hopefully I didn't complicate this :)

~tommy

[oranenj]

Hmm, yeah. I can see how separate parameters would make the implementation much easier.

We need to be really careful with implementing this, since there are many corner cases where a compilation/installation/upgrade of the SELinux module might fail, and we don't want to end up in a situation where the next puppet runs won't even notice that something is wrong as seems to be the case with the current version sometimes.

I think the module definitely should manage the source dir and the source files completely (eg. something like $vardir/puppet-selinux/$modulename), and purge them when it stops managing the module (it does not need to remove the module). Perhaps output the resulting .pp file into another path that does not get purged?

Also, how does selmodule behave when the policy source is updated without corresponding a version bump in the source? IIRC the current version has problems with that case. Can we detect such a case and issue a warning? The File resource has validate_cmd nowadays, those might be useful.

It might be possible to write our own provider for selmodule that extends the built-in provider with any functionality we might need.

[vinzent]

We need to be really careful with implementing this, since there are many corner cases where a
compilation/installation/upgrade of the SELinux module might fail, and we don't want to end up in a
situation where the next puppet runs won't even notice that something is wrong as seems to be the case
with the current version sometimes.

regarding the build/load indempotentcy or constant error-ing when things fail. i thought about something like:

class selkinux::config::module {
  # manage dir to store sources/binaries of selinux modules
  # purge not puppet managed files
  file { '/var/lib/puppet/puppet-selinux/modules':
    ensure => 'directory',
    recurse => true,
    purge => true,
    force => true,
  }
}

define selinux::module {
  require selinux::config::module

  #file resources for
  #   /var/lib/puppet/puppet-selinux/modules/$title.te, 
  #  /var/lib/puppet/puppet-selinux/modules/$title.if, 
  #  /var/lib/puppet/puppet-selinux/modules/$title.fc,   ->

  exec { "build-module-$title":
    command => "build-command || (rm -f /var/lib/puppet/puppet-selinux/modules/$title.pp && exit 1)",
    unless => 'test -f /var/lib/puppet/puppet-selinux/modules/$title.pp.needsload && test -f /var/lib/puppet/puppet-selinux/modules/$title.pp',
  } ->
  exec { 'build-module-$title-needs-load':
    # will only be run if previous exec did succeed and the needsload stanza does not exist
    command => 'touch /var/lib/puppet/puppet-selinux/modules/$title.pp.needsload',
    unless => 'test -f /var/lib/puppet/puppet-selinux/modules/$title.pp.needsload',
  } ->
  selmodule {  $title: 
    ensure => present' 
  } ->
  exec { 'build-module-$title-load-succeeded':
    # will only be run if selmodule did succeed and .loaded stanza does not exist.
    command => 'rm /var/lib/puppet/puppet-selinux/modules/$title.pp.needsload && touch /var/lib/puppet/puppet-selinux/modules/$title.pp.loaded',
    unless => 'test -f /var/lib/puppet/puppet-selinux/modules/$title.pp.loaded',
  } 

  # ensure files do not get purged (if they exist)
  file { [
    /var/lib/puppet/puppet-selinux/modules/$title.pp, 
   /var/lib/puppet/puppet-selinux/modules/$title.pp.needsload, 
   /var/lib/puppet/puppet-selinux/modules/$title.pp.loaded
  ]:
  owner => 'root',
  group => 'root',
  mode => '0640',
}  

[vinzent]

Also, how does selmodule behave when the policy source is updated without corresponding a version
bump in the source? IIRC the current version has problems with that case. Can we detect such a case and
issue a warning? The File resource has validate_cmd nowadays, those might be useful.

the selmodule utility happily installs/overrides an already existing module with -i. it doesnt require a force or somewhat. this is what the selmodule type uses.

unfortunatly selinux utility upstream thought that version numbers in modules are useless and removed the ability to display module versions with the semodule utlity. Fedora 25 example:

$ sudo semodule -l | head -5
abrt
accountsd
acct
afs
aiccu
$ sudo semodule -lfull | head -5     
400 sandbox           pp disabled
400 tmpreaper         pp         
200 container         pp         
100 abrt              pp         

no versions anymore.

They even introduced this new userspace utlities with EL 7.3 and broke the selmodule types syncversion parameter (https://docs.puppet.com/puppet/latest/type.html#selmodule-attribute-syncversion). Customers complained about this change within a major version and they will re-introduce it for 7.3 as bugfix.

anyway you cannot expect to get the version number anymore. so you really don't know which version of your module is loaded. the only way is to track exit codes of commands and ensure they get retried every puppet run.

[vinzent]

@oranenj but maybe this could also be implemented as type/provider - but on the type/provider side i'm very newbie. I think it would then need a dedicated type/provider for building modules as it is a complet other business than loading/removing a module.

[vinzent]

ways to compile a module:

the refpolicy style way:

• having a directory with mymodule.te, mymodule.if and mymodule.te files (only .te is mandatory)
• to compile run make -f /usr/share/selinux/devel/Makefile mymodule.pp
• creates a tmp/ file with temp files an the mymodule.pp, only recompiles the module if it thinks the source has changed since
• requires the selinux-policy-devel package (and tools like make which it depends on)

the minimal way:

• have a *.te and a *.fc file (only .te is mandatory)
• create *.mod file: /usr/bin/checkmodule -M -m -o ${name}.mod {name}.te
• create *.pp file: /usr/bin/semodule_package -o ${name}.pp --fc ${name}.fc -m ${name}.mod
• doesn't require selinux-policy-devel - but you can't use the interfaces and patterns

[oranenj]

I implemented the suggested changes in the linked PR and I'm pretty happy with how it looks. If no-one has any ideas for an even better interface, feel free to merge that in.