From: Lev Popov <lev@spotify.com>
Date: Tue, 8 Mar 2016 22:07:01 +0000 (+0300)
Subject: support for multiple ipsets in a rule
X-Git-Tag: 1.8.2~31^2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=445b28fc7b4ba8ed56f61caa4ca247a40150510a;p=puppet-modules%2Fpuppetlabs-firewall.git

support for multiple ipsets in a rule

Support for multiple ipsets in a single rule. This feature is very handy
if you need to match source and destination from different ipsets.
Iptables arguments are a bit wierd, but it works, details are in
https://utcc.utoronto.ca/~cks/space/blog/linux/IptablesIpsetsMultipleMatches
---

diff --git a/README.markdown b/README.markdown
index e76f822..896c28c 100644
--- a/README.markdown
+++ b/README.markdown
@@ -572,7 +572,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.
 
diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index dd92362..e9ce2eb 100644
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -344,8 +344,14 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
     # --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.
@@ -438,6 +444,8 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
       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',
@@ -496,7 +504,6 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
       :dport,
       :dst_range,
       :dst_type,
-      :ipset,
       :port,
       :proto,
       :source,
@@ -638,7 +645,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
         #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+/)
@@ -669,10 +676,20 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
         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
diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index 2529d19..b3bdd80 100644
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -1188,14 +1188,23 @@ Puppet::Type.newtype(:firewall) do
     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
diff --git a/spec/acceptance/firewall_spec.rb b/spec/acceptance/firewall_spec.rb
index def7d17..7fa7969 100644
--- a/spec/acceptance/firewall_spec.rb
+++ b/spec/acceptance/firewall_spec.rb
@@ -1533,24 +1533,31 @@ describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfami
               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
 
@@ -1559,7 +1566,7 @@ describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfami
 
           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