(SIMP-6839) Audit service and kernel config align (#126)

- Ensure that the auditd service is not managed if the kernel is not enforcing auditing - Add an acceptance test for toggling disabling auditing without modifying the kernel parameter SIMP-6839 #comment Ensure that the audit service can be correctly managed based on the kernel state Co-authored-by: op-ct <[email protected]>
simp · Aug 13, 2020 · 90ee8dd · 90ee8dd
1 parent 10420c5
commit 90ee8dd
Show file tree

Hide file tree

Showing 9 changed files with 138 additions and 16 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,9 @@
+* Wed Aug 12 2020 Trevor Vaughan <[email protected]> - 8.6.0-0
+- Ensure that the auditd service is not managed if the kernel is not enforcing
+  auditing
+- Add an acceptance test for toggling disabling auditing without modifying the
+  kernel parameter
+
 * Fri Aug 07 2020 Marcel Fischer <[email protected]> - 8.5.3-0
 - Add `INCREMENTAL_ASYNC` to possible values for `$::auditd::flush`
 

diff --git a/manifests/config.pp b/manifests/config.pp
@@ -76,7 +76,7 @@
     group   => $auditd::log_group,
     mode    => $log_file_mode,
     content => "${_auditd_conf_common}${_auditd_conf_main}${_auditd_conf_last}\n",
-    notify  => Service['auditd']
+    notify  => Class['auditd::service']
   }
 
   if defined('$auditd::plugin_dir') {

diff --git a/manifests/config/audisp_service.pp b/manifests/config/audisp_service.pp
@@ -14,7 +14,7 @@
   exec { 'Restart Audispd':
     command => '/bin/true',
     unless  => "/usr/bin/pgrep -f ${auditd::dispatcher}",
-    notify  => Service[$auditd::service_name]
+    notify  => Class['auditd::service']
   }
 
 }
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -255,6 +255,8 @@
   Boolean                                 $write_logs               = $log_format ? { 'NOLOG' => false, default => true }
 ) {
 
+  include 'auditd::service'
+
   if $enable {
     unless $space_left > $admin_space_left {
       fail('Auditd requires $space_left to be greater than $admin_space_left, otherwise it will not start')
@@ -309,7 +311,6 @@
 
     include 'auditd::install'
     include 'auditd::config'
-    include 'auditd::service'
 
     Class['auditd::install']
     -> Class['auditd::config']

diff --git a/manifests/service.pp b/manifests/service.pp
@@ -8,20 +8,40 @@
 # @param enable
 #   ``enable`` state from the service resource
 #
+# @param bypass_kernel_check
+#   Do not check to see if the kernel is enforcing auditing before trying to
+#   manage the service.
+#
+#   * This may be required if auditing is not being actively managed in the
+#     kernel and someone has stopped the auditd service by hand.
+#
+# @param warn_if_reboot_required
+#   Add a ``reboot_notify`` warning if the system requires a reboot before the
+#   service can be managed.
+#
 # @author https://github.com/simp/pupmod-simp-auditd/graphs/contributors
 #
 class auditd::service (
-  $ensure = 'running',
-  $enable = true
+  Variant[String[1],Boolean] $ensure                  = pick(getvar('auditd::enable'), 'running'),
+  Boolean                    $enable                  = pick(getvar('auditd::enable'), true),
+  Boolean                    $bypass_kernel_check     = false,
+  Boolean                    $warn_if_reboot_required = true
 ){
   assert_private()
 
-  # CCE-27058-7
-  service { $::auditd::service_name:
-    ensure  => $ensure,
-    enable  => $enable,
-    start   => "/sbin/service ${auditd::service_name} start",
-    stop    => "/sbin/service ${auditd::service_name} stop",
-    restart => "/sbin/service ${auditd::service_name} restart"
+  if $bypass_kernel_check or $facts.dig('simplib__auditd', 'kernel_enforcing') {
+    # CCE-27058-7
+    service { $auditd::service_name:
+      ensure  => $ensure,
+      enable  => $enable,
+      start   => "/sbin/service ${auditd::service_name} start",
+      stop    => "/sbin/service ${auditd::service_name} stop",
+      restart => "/sbin/service ${auditd::service_name} restart"
+    }
+  }
+  elsif $warn_if_reboot_required {
+    reboot_notify { "${auditd::service_name} service":
+      reason =>  "The ${auditd::service_name} service cannot be started when the kernel is not enforcing auditing"
+    }
   }
 }
diff --git a/metadata.json b/metadata.json
@@ -1,6 +1,6 @@
 {
   "name": "simp-auditd",
-  "version": "8.5.3",
+  "version": "8.6.0",
   "author": "SIMP Team",
   "summary": "A SIMP puppet module for managing auditd and audispd",
   "license": "Apache-2.0",

diff --git a/spec/acceptance/suites/default/90_disable_audit_spec.rb b/spec/acceptance/suites/default/90_disable_audit_spec.rb
@@ -0,0 +1,69 @@
+require 'spec_helper_acceptance'
+
+test_name 'disabling auditing via auditd class'
+
+describe 'auditd class with simp auditd profile' do
+  let(:enable_hieradata) {
+    {
+      'pki::cacerts_sources'    => ['file:///etc/pki/simp-testing/pki/cacerts'] ,
+      'pki::private_key_source' => "file:///etc/pki/simp-testing/pki/private/%{fqdn}.pem",
+      'pki::public_key_source'  => "file:///etc/pki/simp-testing/pki/public/%{fqdn}.pub",
+    }
+  }
+
+  let(:disable_hieradata) {
+    {
+      'pki::cacerts_sources'    => ['file:///etc/pki/simp-testing/pki/cacerts'] ,
+      'pki::private_key_source' => "file:///etc/pki/simp-testing/pki/private/%{fqdn}.pem",
+      'pki::public_key_source'  => "file:///etc/pki/simp-testing/pki/public/%{fqdn}.pub",
+      'auditd::enable' => false
+    }
+  }
+
+  let(:manifest) {
+    <<-EOS
+      class { 'auditd': }
+    EOS
+  }
+
+  hosts.each do |host|
+    context "on #{host}" do
+      context 'ensure that auditing is enabled' do
+        it 'should work with no errors' do
+          set_hieradata_on(host, enable_hieradata)
+          apply_manifest_on(host, manifest, :catch_failures => true)
+
+          host.reboot
+        end
+      end
+
+      context 'disabling auditd' do
+        it 'should work with no errors' do
+          set_hieradata_on(host, disable_hieradata)
+          apply_manifest_on(host, manifest, :catch_failures => true)
+        end
+
+        it 'should kill the auditd service' do
+          result = YAML.safe_load(on(host, 'puppet resource service auditd --to_yaml').stdout)
+
+          expect(result['service']['auditd']['ensure']).to eq('stopped')
+          expect(result['service']['auditd']['enable']).to eq('false')
+        end
+
+        it 'should require reboot on subsequent run' do
+          result = apply_manifest_on(host, manifest, :catch_failures => true)
+          expect(result.output).to include('audit => modified')
+
+          # Reboot to disable auditing in the kernel
+          host.reboot
+        end
+
+        it 'should have kernel-level audit disabled on reboot' do
+          retry_on(host, 'grep "audit=0" /proc/cmdline',
+            { :max_retries => 30, :verbose => true }
+          )
+        end
+      end
+    end
+  end
+end
diff --git a/spec/acceptance/suites/default/99_disable_audit_kernel_spec.rb b/spec/acceptance/suites/default/99_disable_audit_kernel_spec.rb
@@ -7,6 +7,14 @@
 test_name 'disabling kernel auditing via auditd class'
 
 describe 'auditd class with simp auditd profile' do
+  let(:enable_hieradata) {
+    {
+      'pki::cacerts_sources'    => ['file:///etc/pki/simp-testing/pki/cacerts'] ,
+      'pki::private_key_source' => "file:///etc/pki/simp-testing/pki/private/%{fqdn}.pem",
+      'pki::public_key_source'  => "file:///etc/pki/simp-testing/pki/public/%{fqdn}.pub",
+    }
+  }
+
   let(:disable_hieradata) {
     {
       'pki::cacerts_sources'    => ['file:///etc/pki/simp-testing/pki/cacerts'] ,
@@ -24,6 +32,15 @@ class { 'auditd': }
 
   hosts.each do |host|
     context "on #{host}" do
+      context 'ensure that auditing is enabled' do
+        it 'should work with no errors' do
+          set_hieradata_on(host, enable_hieradata)
+          apply_manifest_on(host, manifest, :catch_failures => true)
+
+          host.reboot
+        end
+      end
+
       context 'disabling auditd at the kernel level' do
         it 'should work with no errors' do
           set_hieradata_on(host, disable_hieradata)

diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb
@@ -13,14 +13,23 @@
   context 'supported operating systems' do
     on_supported_os.each do |os, os_facts|
       context "on #{os}" do
-        let(:facts) {os_facts}
+        let(:facts) do
+          os_facts.merge(
+            {
+              :simplib__auditd => {
+                'enabled' => true,
+                'kernel_enforcing' => true
+              }
+            }
+          )
+        end
 
         context 'auditd with default parameters' do
           let(:params) {{ }}
           it_behaves_like 'a structured module'
           it {
             is_expected.to contain_service('auditd').with({
-              :ensure  => 'running',
+              :ensure  => true,
               :enable  => true,
               :start   => "/sbin/service auditd start",
               :stop    => "/sbin/service auditd stop",
@@ -49,7 +58,7 @@
           it { is_expected.to contain_class('auditd::config::grub').with_enable(false) }
           it { is_expected.to_not contain_class('auditd::install') }
           it { is_expected.to_not contain_class('auditd::config') }
-          it { is_expected.to_not contain_class('auditd::service') }
+          it { is_expected.to contain_class('auditd::service') }
         end
 
       end