(MODULES-450) Enable rule inversion

README.markdown

@@ -198,6 +198,28 @@ class profile::apache {
 }
 ```
 
+###Rule inversion
+Firewall rules may be inverted by prefixing the value of a parameter by "! ". If the value is an array, then every item in the array must be prefixed as iptables does not understand inverting a single value.
+
+Parameters that understand inversion are: connmark, ctstate, destination, dport, dst\_range, dst\_type, port, proto, source, sport, src\_range, src\_type, and state.
+
+Examples:
+
+```puppet
+firewall { '001 disallow esp protocol':
+  action => 'accept',
+  proto  => '! esp',
+}
+firewall { '002 drop NEW external website packets with FIN/RST/ACK set and SYN unset':
+  chain     => 'INPUT',
+  state     => 'NEW',
+  action    => 'drop',
+  proto     => 'tcp',
+  sport     => ['! http', '! 443'],
+  source    => '! 10.0.0.0/8',
+  tcp_flags => '! FIN,SYN,RST,ACK SYN',
+}
+```
 
 ###Additional Uses for the Firewall Module
 

lib/puppet/provider/firewall/iptables.rb

@@ -216,13 +216,13 @@ 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.sub(/--tcp-flags (\S*) (\S*)/, '--tcp-flags "\1 \2"')
+    values = values.gsub(/(!\s+)?--tcp-flags (\S*) (\S*)/, '--tcp-flags "\1\2 \3"')
     # we do a similar thing for negated address masks (source and destination).
-    values = values.sub(/(-\S+) (!)\s?(\S*)/,'\1 "\2 \3"')
+    values = values.gsub(/(-\S+) (!)\s?(\S*)/,'\1 "\2 \3"')
     # the actual rule will have the ! mark before the option.
-    values = values.sub(/(!)\s*(-\S+)\s*(\S*)/, '\2 "\1 \3"')
+    values = values.gsub(/(!)\s*(-\S+)\s*(\S*)/, '\2 "\1 \3"')
     # The match extension for tcp & udp are optional and throws off the @resource_map.
-    values = values.sub(/-m (tcp|udp) (--(s|d)port|-m multiport)/, '\2')
+    values = values.gsub(/-m (tcp|udp) (--(s|d)port|-m multiport)/, '\2')
     # '--pol ipsec' takes many optional arguments; we cheat again by adding " around them
     values = values.sub(/
         --pol\sipsec
@@ -291,14 +291,6 @@ def self.rule_to_hash(line, table, counter)
     # POST PARSE CLUDGING
     #####################
 
-    # Normalise all rules to CIDR notation.
-    [:source, :destination].each do |prop|
-      next if hash[prop].nil?
-      m = hash[prop].match(/(!?)\s?(.*)/)
-      neg = "! " if m[1] == "!"
-      hash[prop] = "#{neg}#{Puppet::Util::IPCidr.new(m[2]).cidr}"
-    end
-
     [:dport, :sport, :port, :state, :ctstate].each do |prop|
       hash[prop] = hash[prop].split(',') if ! hash[prop].nil?
     end
@@ -322,6 +314,43 @@ def self.rule_to_hash(line, table, counter)
       end
     end
 
+    # Invert any rules that are prefixed with a '!'
+    [
+      :connmark,
+      :ctstate,
+      :destination,
+      :dport,
+      :dst_range,
+      :dst_type,
+      :port,
+      :proto,
+      :source,
+      :sport,
+      :src_range,
+      :src_type,
+      :state,
+    ].each do |prop|
+      if hash[prop] and hash[prop].is_a?(Array)
+        # find if any are negated, then negate all if so
+        should_negate = hash[prop].index do |value|
+          value.match(/^(!)\s+/)
+        end
+        hash[prop] = hash[prop].collect { |v|
+          "! #{v.sub(/^!\s+/,'')}"
+        } if should_negate
+      elsif hash[prop]
+        m = hash[prop].match(/^(!?)\s?(.*)/)
+        neg = "! " if m[1] == "!"
+        if [:source,:destination].include?(prop)
+          p hash if hash[prop] == "udp"
+          # Normalise all rules to CIDR notation.
+          hash[prop] = "#{neg}#{Puppet::Util::IPCidr.new(m[2]).cidr}"
+        else
+          hash[prop] = "#{neg}#{m[2]}"
+        end
+      end
+    end
+
     # States should always be sorted. This ensures that the output from
     # iptables-save and user supplied resources is consistent.
     hash[:state]   = hash[:state].sort   unless hash[:state].nil?
@@ -424,12 +453,37 @@ def general_args
       end
 
       args << [resource_map[res]].flatten.first.split(' ')
+      args = args.flatten
 
       # On negations, the '!' has to be before the option (eg: "! -d 1.2.3.4")
       if resource_value.is_a?(String) and resource_value.sub!(/^!\s*/, '') then
         # we do this after adding the 'dash' argument because of ones like "-m multiport --dports", where we want it before the "--dports" but after "-m multiport".
         # so we insert before whatever the last argument is
         args.insert(-2, '!')
+      elsif resource_value.is_a?(Symbol) and resource_value.to_s.match(/^!/) then
+        #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)
+        should_negate = resource_value.index do |value|
+          #ruby 1.8.7 can't .match symbols
+          value.to_s.match(/^(!)\s+/)
+        end
+        if should_negate
+          resource_value, wrong_values = resource_value.collect do |value|
+            if value.is_a?(String)
+              wrong = value if ! value.match(/^!\s+/)
+              [value.sub(/^!\s*/, ''),wrong]
+            else
+              [value,nil]
+            end
+          end.transpose
+          wrong_values = wrong_values.compact
+          if ! wrong_values.empty?
+            fail "All values of the '#{res}' property must be prefixed with a '!' when inverting, but '#{wrong_values.join("', '")}' #{wrong_values.length>1?"are":"is"} not prefixed; aborting"
+          end
+          args.insert(-2, '!')
+        end
       end
 
 

lib/puppet/type/firewall.rb

@@ -53,6 +53,7 @@
   feature :isfirstfrag, "Match the first fragment of a fragmented ipv6 packet"
   feature :ipsec_policy, "Match IPsec policy"
   feature :ipsec_dir, "Match IPsec policy direction"
+  feature :mask, "Ability to match recent rules based on the ipv4 mask"
 
   # provider specific features
   feature :iptables, "The provider provides iptables features."
@@ -323,7 +324,9 @@ def should_to_s(value)
       *tcp*.
     EOS
 
-    newvalues(:tcp, :udp, :icmp, :"ipv6-icmp", :esp, :ah, :vrrp, :igmp, :ipencap, :ospf, :gre, :cbt, :all)
+    newvalues(*[:tcp, :udp, :icmp, :"ipv6-icmp", :esp, :ah, :vrrp, :igmp, :ipencap, :ospf, :gre, :cbt, :all].collect do |proto|
+      [proto, "! #{proto}".to_sym]
+    end.flatten)
     defaultto "tcp"
   end
 

lib/puppet/util/firewall.rb

@@ -77,14 +77,11 @@ def string_to_port(value, proto)
       proto = 'tcp'
     end
 
-    if value.kind_of?(String)
-      if value.match(/^\d+(-\d+)?$/)
-        return value
-      else
-        return Socket.getservbyname(value, proto).to_s
-      end
+    m = value.to_s.match(/^(!\s+)?(\S+)/)
+    if m[2].match(/^\d+(-\d+)?$/)
+      return "#{m[1]}#{m[2]}"
     else
-      Socket.getservbyname(value.to_s, proto).to_s
+      return "#{m[1]}#{Socket.getservbyname(m[2], proto).to_s}"
     end
   end
 

spec/acceptance/invert_spec.rb

@@ -0,0 +1,55 @@
+require 'spec_helper_acceptance'
+
+describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do
+  before(:all) do
+    iptables_flush_all_tables
+  end
+
+  context "inverting rules" do
+    it 'applies' do
+      pp = <<-EOS
+        class { '::firewall': }
+        firewall { '601 disallow esp protocol':
+          action => 'accept',
+          proto  => '! esp',
+        }
+        firewall { '602 drop NEW external website packets with FIN/RST/ACK set and SYN unset':
+          chain     => 'INPUT',
+          state     => 'NEW',
+          action    => 'drop',
+          proto     => 'tcp',
+          sport     => ['! http', '! 443'],
+          source    => '! 10.0.0.0/8',
+          tcp_flags => '! FIN,SYN,RST,ACK SYN',
+        }
+      EOS
+
+      apply_manifest(pp, :catch_failures => true)
+      apply_manifest(pp, :catch_changes  => true)
+    end
+
+    it 'should contain the rules' do
+      shell('iptables-save') do |r|
+        expect(r.stdout).to match(/-A INPUT ! -p esp -m comment --comment "601 disallow esp protocol" -j ACCEPT/)
+        expect(r.stdout).to match(/-A INPUT ! -s 10\.0\.0\.0\/8 -p tcp -m tcp ! --tcp-flags FIN,SYN,RST,ACK SYN -m multiport ! --sports 80,443 -m comment --comment "602 drop NEW external website packets with FIN\/RST\/ACK set and SYN unset" -m state --state NEW -j DROP/)
+      end
+    end
+  end
+  context "inverting partial array rules" do
+    it 'raises a failure' do
+      pp = <<-EOS
+        class { '::firewall': }
+        firewall { '603 drop 80,443 traffic':
+          chain     => 'INPUT',
+          action    => 'drop',
+          proto     => 'tcp',
+          sport     => ['! http', '443'],
+        }
+      EOS
+
+      apply_manifest(pp, :expect_failures => true) do |r|
+        expect(r.stderr).to match(/is not prefixed/)
+      end
+    end
+  end
+end

spec/fixtures/iptables/conversion_hash.rb

@@ -473,6 +473,40 @@
       :action => 'reject',
     },
   },
+  'disallow_esp_protocol' => {
+    :line   => '-t filter ! -p esp -m comment --comment "063 disallow esp protocol" -j ACCEPT',
+    :table  => 'filter',
+    :params => {
+      :name   => '063 disallow esp protocol',
+      :action => 'accept',
+      :proto  => '! esp',
+    },
+  },
+  'drop_new_packets_without_syn' => {
+    :line   => '-t filter ! -s 10.0.0.0/8 ! -p tcp -m tcp ! --tcp-flags FIN,SYN,RST,ACK SYN -m comment --comment "064 drop NEW non-tcp external packets with FIN/RST/ACK set and SYN unset" -m state --state NEW -j DROP',
+    :table  => 'filter',
+    :params => {
+      :name      => '064 drop NEW non-tcp external packets with FIN/RST/ACK set and SYN unset',
+      :state     => ['NEW'],
+      :action    => 'drop',
+      :proto     => '! tcp',
+      :source    => '! 10.0.0.0/8',
+      :tcp_flags => '! FIN,SYN,RST,ACK SYN',
+    },
+  },
+  'negate_dport_and_sport' => {
+    :line => '-A nova-compute-FORWARD -s 0.0.0.0/32 -d 255.255.255.255/32 -p udp -m udp ! --sport 68,69 ! --dport 67,66 -j ACCEPT',
+    :table => 'filter',
+    :params => {
+      :action => 'accept',
+      :chain => 'nova-compute-FORWARD',
+      :source => '0.0.0.0/32',
+      :destination => '255.255.255.255/32',
+      :sport => ['! 68','! 69'],
+      :dport => ['! 67','! 66'],
+      :proto => 'udp',
+    },
+  },
 }
 
 # This hash is for testing converting a hash to an argument line.
@@ -940,4 +974,40 @@
     },
     :args => ["-t", :filter, "-p", :all, "-m", "comment", "--comment", "062 REJECT connmark", "-j", "REJECT", "-m", "connmark", "--mark", "0x1"],
   },
+  'disallow_esp_protocol' => {
+    :params => {
+      :name   => '063 disallow esp protocol',
+      :table  => 'filter',
+      :action => 'accept',
+      :proto  => '! esp',
+    },
+    :args => ["-t", :filter, "!", "-p", :esp, "-m", "comment", "--comment", "063 disallow esp protocol", "-j", "ACCEPT"],
+  },
+  'drop_new_packets_without_syn' => {
+    :params => {
+      :name      => '064 drop NEW non-tcp external packets with FIN/RST/ACK set and SYN unset',
+      :table     => 'filter',
+      :chain     => 'INPUT',
+      :state     => ['NEW'],
+      :action    => 'drop',
+      :proto     => '! tcp',
+      :source    => '! 10.0.0.0/8',
+      :tcp_flags => '! FIN,SYN,RST,ACK SYN',
+    },
+    :args => ["-t", :filter, "!", "-s", "10.0.0.0/8", "!", "-p", :tcp, "-m", "tcp", "!", "--tcp-flags", "FIN,SYN,RST,ACK", "SYN", "-m", "comment", "--comment", "064 drop NEW non-tcp external packets with FIN/RST/ACK set and SYN unset", "-m", "state", "--state", "NEW", "-j", "DROP"]
+  },
+  'negate_dport_and_sport' => {
+    :params => {
+      :name        => '065 negate dport and sport',
+      :table       => 'filter',
+      :action      => 'accept',
+      :chain       => 'nova-compute-FORWARD',
+      :source      => '0.0.0.0/32',
+      :destination => '255.255.255.255/32',
+      :sport       => ['! 68','! 69'],
+      :dport       => ['! 67','! 66'],
+      :proto       => 'udp',
+    },
+    :args => ["-t", :filter, "-s", "0.0.0.0/32", "-d", "255.255.255.255/32", "-p", :udp, "-m", "multiport", "!", "--sports", "68,69", "-m", "multiport", "!", "--dports", "67,66", "-m", "comment", "--comment", "065 negate dport and sport", "-j", "ACCEPT"],
+  },
 }

spec/unit/puppet/provider/iptables_spec.rb

@@ -315,6 +315,27 @@
     end
   end
 
+  describe 'when inverting rules' do
+    let(:resource) {
+      Puppet::Type.type(:firewall).new({
+        :name  => '040 partial invert',
+        :table       => 'filter',
+        :action      => 'accept',
+        :chain       => 'nova-compute-FORWARD',
+        :source      => '0.0.0.0/32',
+        :destination => '255.255.255.255/32',
+        :sport       => ['! 78','79','http'],
+        :dport       => ['77','! 76'],
+        :proto       => 'udp',
+      })
+    }
+    let(:instance) { provider.new(resource) }
+
+    it 'fails when not all array items are inverted' do
+      expect { instance.insert }.to raise_error Puppet::Error, /but '79', '80' are not prefixed/
+    end
+  end
+
   describe 'when deleting resources' do
     let(:sample_rule) {
       '-A INPUT -s 1.1.1.1 -d 1.1.1.1 -p tcp -m multiport --dports 7061,7062 -m multiport --sports 7061,7062 -j ACCEPT'