]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
(MODULES-4093) Tighten SELinux permissions on persistent files
authorDavid Schmitt <david.schmitt@puppet.com>
Sat, 19 Nov 2016 14:25:35 +0000 (15:25 +0100)
committerDavid Schmitt <david.schmitt@puppet.com>
Sat, 19 Nov 2016 18:23:35 +0000 (18:23 +0000)
RHEL7's /usr/libexec/iptables/iptables.init creates the /etc/sysconfig/iptables
file with the wrong selinux parameters, causing spurious changes on the next
run:

    [root@ns57zjx0zb7s0b5 ~]# rm -f /etc/sysconfig/iptables
    [root@ns57zjx0zb7s0b5 ~]# ls -la /etc/sysconfig/iptables
    ls: cannot access /etc/sysconfig/iptables: No such file or directory
    [root@ns57zjx0zb7s0b5 ~]# iptables -A INPUT --source 8.8.8.8 -j REJECT
    [root@ns57zjx0zb7s0b5 ~]# /usr/libexec/iptables/iptables.init save
    iptables: Saving firewall rules to /etc/sysconfig/iptables:[  OK  ]
    [root@ns57zjx0zb7s0b5 ~]# ls -la /etc/sysconfig/iptables
    -rw-------. 1 root root 259 Nov 19 06:02 /etc/sysconfig/iptables
    [root@ns57zjx0zb7s0b5 ~]# /opt/puppetlabs/bin/puppet apply --verbose selinux.pp
    Notice: Compiled catalog for ns57zjx0zb7s0b5.delivery.puppetlabs.net in environment production in 0.08 seconds
    Info: Applying configuration version '1479564151'
    Notice: /Stage[main]/Main/File[/etc/sysconfig/iptables]/seluser: seluser changed 'unconfined_u' to 'system_u'
    Notice: /Stage[main]/Main/File[/etc/sysconfig/iptables]/seltype: seltype changed 'etc_t' to 'system_conf_t'
    Notice: Applied catalog in 0.03 seconds
    [root@ns57zjx0zb7s0b5 ~]# X Error of failed request:  RenderBadPicture (invalid Picture parameter)

To fix this, this patch changes the order in which puppet checks the resources.
Instead of managing the persistence file before the service, now we manage the
file after all firewall rules. The firewall provider persists the rules to disk
causing the /etc/sysconfig/iptables file to be created. Managing its
permissions afterwards leads to one-run idempotency.

To see why this change is legal, consider the possible initial states for a
moment:

* fresh install: the /etc/sysconfig/iptables file does not exist
  in the previous implementation, puppet would create it empty with the correct
  permissions, only to have it overwritten when persisting the firewall rules
* fixed point: the /etc/sysconfig/iptables file already exists with the correct
  permissions, and nothing has changed. The order of resource application is
  irrelevant
* a firewall rule has changed: the firewall type will persist the rules
  changing the permissions to a invalid state, puppet will fix it in the same
  agent run.
* the /etc/sysconfig/iptables file is in an invalid state: this might be the
  most annoying case. In the original version, puppet would have fixed the file
  before touching the service. Now the service could arguably fail to start if
  the permissions are really bad. Puppet will still fix the issue, and start
  the service on the next run, so I do not consider this to be a big problem.

lib/puppet/type/firewall.rb
manifests/linux/redhat.pp

index 71e5394b75eb0fdc28b8630054665872752b1199..d24b05029c352f508d4840b80420283843316205 100644 (file)
@@ -1527,6 +1527,14 @@ Puppet::Type.newtype(:firewall) do
     end
   end
 
+  # autobefore is only provided since puppet 4.0
+  if Puppet.version.to_f >= 4.0
+    # On RHEL 7 this needs to be threaded correctly to manage SE Linux permissions after persisting the rules
+    autobefore(:file) do
+      [ '/etc/sysconfig/iptables' ]
+    end
+  end
+
   validate do
     debug("[validate]")
 
@@ -1681,7 +1689,7 @@ Puppet::Type.newtype(:firewall) do
       unless value(:jump).to_s == "NFQUEUE"
         self.fail "Paramter queue_number and queue_bypass require jump => NFQUEUE"
       end
-    end  
+    end
 
   end
 end
index 44ced37b72c6248f5a15cccf04bfb0686e8a8c3a..f203e141ea405ad86aeb30764f535334e4fa7536 100644 (file)
@@ -56,27 +56,12 @@ class firewall::linux::redhat (
     ensure    => $ensure,
     enable    => $enable,
     hasstatus => true,
-    require   => File["/etc/sysconfig/${service_name}"],
-  }
-
-  # Redhat 7 selinux user context for /etc/sysconfig/iptables is set to unconfined_u
-  case $::selinux {
-    #lint:ignore:quoted_booleans
-    'true',true: {
-      case $::operatingsystemrelease {
-        /^(6|7)\..*/: { $seluser = 'unconfined_u' }
-        default: { $seluser = 'system_u' }
-      }
-    }
-    #lint:endignore
-    default:     { $seluser = undef }
   }
 
   file { "/etc/sysconfig/${service_name}":
-    ensure  => present,
-    owner   => 'root',
-    group   => 'root',
-    mode    => '0600',
-    seluser => $seluser,
+    ensure => present,
+    owner  => 'root',
+    group  => 'root',
+    mode   => '0600',
   }
 }