Merge pull request puppetlabs#615 from nabam/multiple-ipset

support for multiple ipsets in a rule

Author: jonnytdevops

README.markdown

@@ -576,7 +576,7 @@ If Puppet is managing the iptables or iptables-persistent packages, and the prov
 
 * `ipsec_policy`: Sets the ipsec policy type. Valid values are 'none', 'ipsec'. Requires the `ipsec_policy` feature.
 
-* `ipset`: Matches IP sets. Value must be 'ipset_name (src|dst|src,dst)' and can be negated by putting ! in front. Requires ipset kernel module.
+* `ipset`: Matches IP sets. Value must be 'ipset_name (src|dst|src,dst)' and can be negated by putting ! in front. Requires ipset kernel module. Will accept a single element or an array.
 
 * `isfirstfrag`: If true, matches when the packet is the first fragment of a fragmented ipv6 packet. Cannot be negated. Supported by ipv6 only. Valid values are 'true', 'false'. Requires the `isfirstfrag` feature.
 

lib/puppet/provider/firewall/iptables.rb

@@ -352,8 +352,14 @@ def self.rule_to_hash(line, table, counter)
     # --tcp-flags takes two values; we cheat by adding " around it
     # so it behaves like --comment
     values = values.gsub(/(!\s+)?--tcp-flags (\S*) (\S*)/, '--tcp-flags "\1\2 \3"')
-    # ditto for --match-set
-    values = values.sub(/(!\s+)?--match-set (\S*) (\S*)/, '--match-set "\1\2 \3"')
+    # --match-set can have multiple values with weird iptables format
+    if values =~ /-m set --match-set/
+      values = values.gsub(/(!\s+)?--match-set (\S*) (\S*)/, '--match-set \1\2 \3')
+      ind  = values.index('-m set --match-set')
+      sets = values.scan(/-m set --match-set ((?:!\s+)?\S* \S*)/)
+      values = values.gsub(/-m set --match-set (!\s+)?\S* \S* /, '')
+      values.insert(ind, "-m set --match-set \"#{sets.join(';')}\" ")
+    end
     # we do a similar thing for negated address masks (source and destination).
     values = values.gsub(/(-\S+) (!)\s?(\S*)/,'\1 "\2 \3"')
     # the actual rule will have the ! mark before the option.
@@ -446,6 +452,8 @@ def self.rule_to_hash(line, table, counter)
       hash[prop] = hash[prop].split(',') if ! hash[prop].nil?
     end
 
+    hash[:ipset] = hash[:ipset].split(';') if ! hash[:ipset].nil?
+
     ## clean up DSCP class to HEX mappings
     valid_dscp_classes = {
       '0x0a' => 'af11',
@@ -507,7 +515,6 @@ def self.rule_to_hash(line, table, counter)
       :dport,
       :dst_range,
       :dst_type,
-      :ipset,
       :port,
       :proto,
       :source,
@@ -649,7 +656,7 @@ def general_args
         #ruby 1.8.7 can't .match Symbols ------------------ ^
         resource_value = resource_value.to_s.sub!(/^!\s*/, '').to_sym
         args.insert(-2, '!')
-      elsif resource_value.is_a?(Array)
+      elsif resource_value.is_a?(Array) and res != :ipset
         should_negate = resource_value.index do |value|
           #ruby 1.8.7 can't .match symbols
           value.to_s.match(/^(!)\s+/)
@@ -680,10 +687,20 @@ def general_args
         end
       end
 
+      # ipset can accept multiple values with weird iptables arguments
+      if res == :ipset
+        resource_value.join(" #{[resource_map[res]].flatten.first} ").split(' ').each do |a|
+          if a.sub!(/^!\s*/, '')
+            # Negate ipset options
+            args.insert(-2, '!')
+          end
+
+          args << a if a.length > 0
+        end
       # our tcp_flags takes a single string with comma lists separated
       # by space
       # --tcp-flags expects two arguments
-      if res == :tcp_flags or res == :ipset
+      elsif res == :tcp_flags
         one, two = resource_value.split(' ')
         args << one
         args << two

lib/puppet/type/firewall.rb

@@ -1190,14 +1190,23 @@ def insync?(is)
     EOS
   end
 
-  newproperty(:ipset, :required_features => :ipset) do
+  newproperty(:ipset, :required_features => :ipset, :array_matching => :all) do
     desc <<-EOS
       Matches against the specified ipset list.
-      Requires ipset kernel module.
+      Requires ipset kernel module. Will accept a single element or an array.
       The value is the name of the blacklist, followed by a space, and then
       'src' and/or 'dst' separated by a comma.
       For example: 'blacklist src,dst'
     EOS
+
+    def is_to_s(value)
+      should_to_s(value)
+    end
+
+    def should_to_s(value)
+      value = [value] unless value.is_a?(Array)
+      value.join(', ')
+    end
   end
 
   newproperty(:checksum_fill, :required_features => :iptables) do

spec/acceptance/firewall_spec.rb

@@ -1531,24 +1531,31 @@ class { '::firewall': }
               require => Package['ipset'],
             }
             class { '::firewall': }
-            exec { 'create ipset':
+            exec { 'create ipset blacklist':
               command => 'ipset create blacklist hash:ip,port family inet6 maxelem 1024 hashsize 65535 timeout 120',
               path    => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
               require => Package['ipset'],
             }
-            exec { 'add blacklist':
+            -> exec { 'create ipset honeypot':
+              command => 'ipset create honeypot hash:ip family inet6 maxelem 1024 hashsize 65535 timeout 120',
+              path    => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
+            }
+            -> exec { 'add blacklist':
               command => 'ipset add blacklist 2001:db8::1,80',
               path    => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
-              require => Exec['create ipset'],
+            }
+            -> exec { 'add honeypot':
+              command => 'ipset add honeypot 2001:db8::5',
+              path    => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
             }
             firewall { '612 - test':
               ensure   => present,
               chain    => 'INPUT',
               proto    => tcp,
               action   => drop,
-              ipset    => 'blacklist src,src',
+              ipset    => ['blacklist src,dst', '! honeypot dst'],
               provider => 'ip6tables',
-              require  => Exec['add blacklist'],
+              require  => Exec['add honeypot'],
             }
             EOS
 
@@ -1557,7 +1564,7 @@ class { '::firewall': }
 
           it 'should contain the rule' do
             shell('ip6tables-save') do |r|
-              expect(r.stdout).to match(/-A INPUT -p tcp -m comment --comment "612 - test" -m set --match-set blacklist src,src -j DROP/)
+              expect(r.stdout).to match(/-A INPUT -p tcp -m comment --comment "612 - test" -m set --match-set blacklist src,dst -m set ! --match-set honeypot dst -j DROP/)
             end
           end
         end