]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
(SEC-944) Identify duplicate system rules
authorCraig Gumbley <craiggumbley@gmail.com>
Tue, 22 Feb 2022 12:33:01 +0000 (12:33 +0000)
committerCraig Gumbley <craiggumbley@gmail.com>
Tue, 22 Feb 2022 14:22:41 +0000 (14:22 +0000)
In certain situations it is possible for an unmanaged rule to exist on
the target system that has the same comment as the rule specified in
the manifest.

When this condition is true puppet will ignore the the unmanaged rule
and continue to apply the rule in the manifest. This is because the
firewall module uses the comment field in IPT as it's namevar and
therefore expects it to be a unique identifier. In the case of IPT this
is not true given that you can have multiple rules with the same
comment.

This commit adds a check that will identify system rules that have their
comment field set to the same value as a rule in the manifest. If we
enter a situation where any of the duplicate counts are greater than 1
then we will respond with a configurable action. The behaviour of this
can be configured via the onduplicaterulebehaviour parameter.

lib/puppet/provider/firewall/iptables.rb

index f193d5b8eaf552f0ba2363bb8a9b7406153b58e4..9087aa569a65d1a3ff75108eeab20fcab0bb9959 100644 (file)
@@ -401,9 +401,29 @@ Puppet::Type.type(:firewall).provide :iptables, parent: Puppet::Provider::Firewa
   end
 
   def exists?
+    if duplicate_rule?(@property_hash[:name])
+      core_message = "Duplicate rule found for #{@property_hash[:name]}."
+      case @resource.parameters[:onduplicaterulebehaviour].value
+      when :error
+        raise   "#{core_message} Skipping update."
+      when :warn
+        warning "#{core_message}. This may add an additional rule to the system."
+      when :ignore
+        debug "#{core_message}. This may add an additional rule to the system."
+      end
+    end
+
     properties[:ensure] != :absent
   end
 
+  def duplicate_rule?(rule)
+    rules = self.class.instances
+    system_rule_count = Hash.new(0)
+    rules.each { |r| system_rule_count[r.name] += 1 }
+    duplicate_rules = rules.select { |r| system_rule_count[r.name] > 1 }
+    duplicate_rules.select { |r| r.name == rule }.any?
+  end
+
   # Flush the property hash once done.
   def flush
     debug('[flush]')