Merge pull request #2313 from david22swan/maint/codebase_hardening

(maint) Codebase Hardening

Author: chelnak

manifests/custom_config.pp

@@ -32,6 +32,9 @@
 #   This parameter is used only if the `verify_config` parameter's value is `true`. If the 
 #   `verify_command` fails, the Puppet run deletes the configuration file and raises an error, 
 #   but does not notify the Apache service.
+#   Command can be passed through as either a String, i.e. `'/usr/sbin/apache2ctl -t'`
+#   An array, i.e. `['/usr/sbin/apache2ctl', '-t']`
+#   Or an array of arrays with each one having to pass succesfully, i.e. `[['/usr/sbin/apache2ctl', '-t'], '/usr/sbin/apache2ctl -t']`
 #
 # @param verify_config
 #   Specifies whether to validate the configuration file before notifying the Apache service.
@@ -49,18 +52,18 @@
 #   show_diff property for configuration file resource
 #
 define apache::custom_config (
-  Enum['absent', 'present'] $ensure     = 'present',
-  Stdlib::Absolutepath $confdir         = $apache::confd_dir,
-  Optional[String] $content             = undef,
-  Apache::Vhost::Priority $priority     = 25,
-  Optional[String] $source              = undef,
-  String $verify_command                = $apache::params::verify_command,
-  Boolean $verify_config                = true,
-  Optional[String] $filename            = undef,
-  Optional[String] $owner               = undef,
-  Optional[String] $group               = undef,
-  Optional[Stdlib::Filemode] $file_mode = undef,
-  Boolean $show_diff                    = true,
+  Enum['absent', 'present'] $ensure                                    = 'present',
+  Stdlib::Absolutepath $confdir                                        = $apache::confd_dir,
+  Optional[String] $content                                            = undef,
+  Apache::Vhost::Priority $priority                                    = 25,
+  Optional[String] $source                                             = undef,
+  Variant[String, Array[String], Array[Array[String]]] $verify_command = $apache::params::verify_command,
+  Boolean $verify_config                                               = true,
+  Optional[String] $filename                                           = undef,
+  Optional[String] $owner                                              = undef,
+  Optional[String] $group                                              = undef,
+  Optional[Stdlib::Filemode] $file_mode                                = undef,
+  Boolean $show_diff                                                   = true,
 ) {
   if $content and $source {
     fail('Only one of $content and $source can be specified.')
@@ -90,11 +93,12 @@
     $notifies = undef
   }
 
+  $_file_path = "${confdir}/${_filename}"
   $_file_mode = pick($file_mode, $apache::file_mode)
 
   file { "apache_${name}":
     ensure    => $ensure,
-    path      => "${confdir}/${_filename}",
+    path      => $_file_path,
     owner     => $owner,
     group     => $group,
     mode      => $_file_mode,
@@ -115,9 +119,10 @@
       require     => Anchor['::apache::modules_set_up'],
     }
 
+    $remove_command = ['/bin/rm', $_file_path]
     exec { "remove ${name} if invalid":
-      command     => "/bin/rm ${confdir}/${_filename}",
-      unless      => $verify_command,
+      command     => $remove_command,
+      unless      => [$verify_command],
       subscribe   => File["apache_${name}"],
       refreshonly => true,
     }

manifests/init.pp

@@ -635,7 +635,9 @@
     path => '/bin:/sbin:/usr/bin:/usr/sbin',
   }
 
+  $confd_command = ['mkdir', $confd_dir]
   exec { "mkdir ${confd_dir}":
+    command => $confd_command,
     creates => $confd_dir,
     require => Package['httpd'],
   }
@@ -660,7 +662,9 @@
   }
 
   if ! defined(File[$mod_dir]) {
+    $mod_command = ['mkdir', $mod_dir]
     exec { "mkdir ${mod_dir}":
+      command => $mod_command,
       creates => $mod_dir,
       require => Package['httpd'],
     }
@@ -678,7 +682,9 @@
 
   if $mod_enable_dir and ! defined(File[$mod_enable_dir]) {
     $mod_load_dir = $mod_enable_dir
+    $mod_enable_command = ['mkdir', $mod_enable_dir]
     exec { "mkdir ${mod_enable_dir}":
+      command => $mod_enable_command,
       creates => $mod_enable_dir,
       require => Package['httpd'],
     }
@@ -694,7 +700,9 @@
   }
 
   if ! defined(File[$vhost_dir]) {
+    $vhost_command = ['mkdir', $vhost_dir]
     exec { "mkdir ${vhost_dir}":
+      command => $vhost_command,
       creates => $vhost_dir,
       require => Package['httpd'],
     }
@@ -709,7 +717,9 @@
 
   if $vhost_enable_dir and ! defined(File[$vhost_enable_dir]) and $manage_vhost_enable_dir {
     $vhost_load_dir = $vhost_enable_dir
+    $vhost_load_command = ['mkdir', $vhost_load_dir]
     exec { "mkdir ${vhost_load_dir}":
+      command => $vhost_load_command,
       creates => $vhost_load_dir,
       require => Package['httpd'],
     }

manifests/mpm/disable_mpm_event.pp

@@ -1,19 +1,28 @@
 # @summary disable Apache-Module event
 class apache::mpm::disable_mpm_event {
+  $event_command = ['/usr/sbin/a2dismod', 'event']
+  $event_onlyif = [['/usr/bin/test', '-e', join([$apache::mod_enable_dir, 'event.load'],'/')]]
   exec { '/usr/sbin/a2dismod event':
-    onlyif  => "/usr/bin/test -e ${apache::mod_enable_dir}/event.load",
+    command => $event_command,
+    onlyif  => $event_onlyif,
     require => Package['httpd'],
     before  => Class['apache::service'],
   }
+
+  $event_load_command = ['/bin/rm', join([$apache::mod_enable_dir, 'event_event.load'],'/')]
+  $event_load_onlyif = [['/usr/bin/test', '-e', join([$apache::mod_enable_dir, 'event_event.load'],'/')]]
   exec { 'remove distribution event load file':
-    command => "/bin/rm ${apache::mod_enable_dir}/mpm_event.load",
-    onlyif  => "/usr/bin/test -e ${apache::mod_enable_dir}/mpm_event.load",
+    command => $event_load_command,
+    onlyif  => $event_load_onlyif,
     require => Package['httpd'],
     before  => Class['apache::service'],
   }
+
+  $event_conf_command = ['/bin/rm', join([$apache::mod_enable_dir, 'event_event.conf'],'/')]
+  $event_conf_onlyif = [['/usr/bin/test', '-e', join([$apache::mod_enable_dir, 'event_event.conf'],'/')]]
   exec { 'remove distribution event conf file':
-    command => "/bin/rm ${apache::mod_enable_dir}/mpm_event.conf",
-    onlyif  => "/usr/bin/test -e ${apache::mod_enable_dir}/mpm_event.conf",
+    command => $event_conf_command,
+    onlyif  => $event_conf_onlyif,
     require => Package['httpd'],
     before  => Class['apache::service'],
   }

manifests/mpm/disable_mpm_prefork.pp

@@ -1,7 +1,10 @@
 # @summary disable Apache-Module prefork
 class apache::mpm::disable_mpm_prefork {
+  $prefork_command = ['/usr/sbin/a2dismod', 'prefork']
+  $prefork_onlyif = [['/usr/bin/test', '-e', join([$apache::mod_enable_dir, 'prefork.load'],'/')]]
   exec { '/usr/sbin/a2dismod prefork':
-    onlyif  => "/usr/bin/test -e ${apache::mod_enable_dir}/prefork.load",
+    command => $prefork_command,
+    onlyif  => $prefork_onlyif,
     require => Package['httpd'],
     before  => Class['apache::service'],
   }

manifests/mpm/disable_mpm_worker.pp

@@ -1,7 +1,10 @@
 # @summary disable Apache-Module worker
 class apache::mpm::disable_mpm_worker {
+  $worker_command = ['/usr/sbin/a2dismod', 'worker']
+  $worker_onlyif = [['/usr/bin/test', '-e', join([$apache::mod_enable_dir, 'worker.load'],'/')]]
   exec { '/usr/sbin/a2dismod worker':
-    onlyif  => "/usr/bin/test -e ${apache::mod_enable_dir}/worker.load",
+    command => $worker_command,
+    onlyif  => $worker_onlyif,
     require => Package['httpd'],
     before  => Class['apache::service'],
   }

manifests/params.pp

@@ -713,13 +713,13 @@
   }
 
   if $facts['os']['name'] == 'SLES' {
-    $verify_command = '/usr/sbin/apache2ctl -t'
+    $verify_command = ['/usr/sbin/apache2ctl', '-t']
   } elsif $facts['os']['name'] == 'FreeBSD' {
-    $verify_command = '/usr/local/sbin/apachectl -t'
+    $verify_command = ['/usr/local/sbin/apachectl', '-t']
   } elsif ($apache::version::scl_httpd_version) {
-    $verify_command = "/opt/rh/${_scl_httpd_name}/root/usr/sbin/apachectl -t"
+    $verify_command = ["/opt/rh/${_scl_httpd_name}/root/usr/sbin/apachectl", '-t']
   } else {
-    $verify_command = '/usr/sbin/apachectl -t'
+    $verify_command = ['/usr/sbin/apachectl', '-t']
   }
 
   if $facts['os']['family'] == 'RedHat' and versioncmp($facts['os']['release']['major'], '8') >= 0 {

spec/defines/custom_config_spec.rb

@@ -21,14 +21,14 @@
 
     it {
       is_expected.to contain_exec('syntax verification for rspec')
-        .with('refreshonly' => 'true', 'command' => '/usr/sbin/apachectl -t')
+        .with('refreshonly' => 'true', 'command' => ['/usr/sbin/apachectl', '-t'])
         .that_subscribes_to('File[apache_rspec]')
         .that_notifies('Class[Apache::Service]')
         .that_comes_before('Exec[remove rspec if invalid]')
     }
     it {
       is_expected.to contain_exec('remove rspec if invalid')
-        .with('unless' => '/usr/sbin/apachectl -t', 'refreshonly' => 'true')
+        .with('unless' => [['/usr/sbin/apachectl', '-t']], 'refreshonly' => 'true')
         .that_subscribes_to('File[apache_rspec]')
     }
     it {
@@ -43,16 +43,16 @@
         'confdir'        => '/dne',
         'priority'       => 30,
         'source'         => 'puppet:///modules/apache/test',
-        'verify_command' => '/bin/true',
+        'verify_command' => ['/bin/true'],
       }
     end
 
     it {
-      is_expected.to contain_exec('syntax verification for rspec').with('command' => '/bin/true')
+      is_expected.to contain_exec('syntax verification for rspec').with('command' => ['/bin/true'])
     }
     it {
-      is_expected.to contain_exec('remove rspec if invalid').with('command' => '/bin/rm /dne/30-rspec.conf',
-                                                                  'unless' => '/bin/true')
+      is_expected.to contain_exec('remove rspec if invalid').with('command' => ['/bin/rm', '/dne/30-rspec.conf'],
+                                                                  'unless' => [['/bin/true']])
     }
     it {
       is_expected.to contain_file('apache_rspec')