Support conntrack stateful firewall matching

Since Linux 3.7+ the "state" module has been removed from the kernel, leaving
only the "conntrack" module. This patch adds support for the conntrack module in
iptables by adding a new parameter to the firewall type, 'ctstate'.

Updates the README to demonstrate using the ctstate parameter instead of state
to nudge people to use it instead. This is safe as far as back to Linux kernel
2.6.18, so long as CONFIG_NF_CONNTRACK is enabled.

diff --git a/README.markdown b/README.markdown
index 467e8359ce84818a2f4386d3f0a9248e4aa2ec55..827ef6fa66faf925869396dcc3611c7e9fc29d53 100644 (file)
--- a/README.markdown
+++ b/README.markdown
@@ -92,7 +92,7 @@ The `pre` class should be located in `my_fw/manifests/pre.pp` and should contain
       }->
       firewall { '002 accept related established rules':
         proto   => 'all',
-        state   => ['RELATED', 'ESTABLISHED'],
+        ctstate => ['RELATED', 'ESTABLISHED'],
         action  => 'accept',
       }
     }

diff --git a/lib/puppet/provider/firewall/ip6tables.rb b/lib/puppet/provider/firewall/ip6tables.rb
index 1a7b16c71ab1057198ef78fc782ae6d8944d7944..56319b1fa8cc5cd51090d5a41721ea8831f7ca70 100644 (file)
--- a/lib/puppet/provider/firewall/ip6tables.rb
+++ b/lib/puppet/provider/firewall/ip6tables.rb
@@ -37,6 +37,7 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source =
 
   @resource_map = {
     :burst => "--limit-burst",
+    :ctstate => "-m conntrack --ctstate",
     :destination => "-d",
     :dport => "-m multiport --dports",
     :gid => "-m owner --gid-owner",
@@ -86,8 +87,8 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source =
   # not provided with current parser [georg.koester])
   @resource_list = [:table, :source, :destination, :iniface, :outiface,
     :proto, :ishasmorefrags, :islastfrag, :isfirstfrag, :gid, :uid, :sport, :dport,
-    :port, :pkttype, :name, :state, :icmp, :hop_limit, :limit, :burst, :jump,
-    :todest, :tosource, :toports, :log_level, :log_prefix, :reject]
+    :port, :pkttype, :name, :state, :ctstate, :icmp, :hop_limit, :limit, :burst,
+    :jump, :todest, :tosource, :toports, :log_level, :log_prefix, :reject]
 
   # These are known booleans that do not take a value, but we want to munge
   # to true if they exist.

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index 7caf9526ae5022a016c943b2fc36f03b6441a9c9..93d7edcaedc439994ddc9433d6c537b7dac85fe0 100644 (file)
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -43,6 +43,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
 
   @resource_map = {
     :burst => "--limit-burst",
+    :ctstate => "-m conntrack --ctstate",
     :destination => "-d",
     :dst_type => "-m addrtype --dst-type",
     :dst_range => "-m iprange --dst-range",
@@ -98,7 +99,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
   # This order can be determined by going through iptables source code or just tweaking and trying manually
   @resource_list = [:table, :source, :src_range, :destination, :dst_range, :iniface, :outiface,
     :proto, :isfragment, :tcp_flags, :gid, :uid, :sport, :dport, :port,
-    :dst_type, :src_type, :socket, :pkttype, :name, :state, :icmp,
+    :dst_type, :src_type, :socket, :pkttype, :name, :state, :ctstate, :icmp,
     :limit, :burst, :jump, :todest, :tosource, :toports, :log_prefix,
     :log_level, :reject, :set_mark]
 
@@ -211,7 +212,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
       hash[prop] = Puppet::Util::IPCidr.new(hash[prop]).cidr unless hash[prop].nil?
     end
 
-    [:dport, :sport, :port, :state].each do |prop|
+    [:dport, :sport, :port, :state, :ctstate].each do |prop|
       hash[prop] = hash[prop].split(',') if ! hash[prop].nil?
     end
 
@@ -236,7 +237,8 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
 
     # 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?
+    hash[:state]   = hash[:state].sort   unless hash[:state].nil?
+    hash[:ctstate] = hash[:ctstate].sort unless hash[:ctstate].nil?
 
     # This forces all existing, commentless rules or rules with invalid comments to be moved 
     # to the bottom of the stack.

diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index 0e4bb792e77f126094e76c143fd11f094deda56f..f88719438402919b39179db67ead5f0717b1cb25 100644 (file)
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -553,6 +553,38 @@ Puppet::Type.newtype(:firewall) do
     end
   end
 
+  newproperty(:ctstate, :array_matching => :all, :required_features =>
+    :state_match) do
+
+    desc <<-EOS
+      Matches a packet based on its state in the firewall stateful inspection
+      table, using the conntrack module. Values can be:
+
+      * INVALID
+      * ESTABLISHED
+      * NEW
+      * RELATED
+    EOS
+
+    newvalues(:INVALID,:ESTABLISHED,:NEW,:RELATED)
+
+    # States should always be sorted. This normalizes the resource states to
+    # keep it consistent with the sorted result from iptables-save.
+    def should=(values)
+      @should = super(values).sort_by {|sym| sym.to_s}
+    end
+
+    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
+
+
   # Hop limiting properties
   newproperty(:hop_limit, :required_features => :hop_limiting) do
     desc <<-EOS

diff --git a/spec/fixtures/iptables/conversion_hash.rb b/spec/fixtures/iptables/conversion_hash.rb
index cf4d868feac12737b232e1175de7c34d65bf6011..580dc48a804dc2cfb42d26ffae78d3178b03c176 100644 (file)
--- a/spec/fixtures/iptables/conversion_hash.rb
+++ b/spec/fixtures/iptables/conversion_hash.rb
@@ -170,6 +170,14 @@ ARGS_TO_HASH = {
       :action => nil,
     },
   },
+  'ctstate_returns_sorted_values' => {
+    :line => '-A INPUT -m conntrack --ctstate INVALID,RELATED,ESTABLISHED',
+    :table => 'filter',
+    :params => {
+      :ctstate => ['ESTABLISHED', 'INVALID', 'RELATED'],
+      :action => nil,
+    },
+  },
   'comment_string_character_validation' => {
     :line => '-A INPUT -s 192.168.0.1/32 -m comment --comment "000 allow from 192.168.0.1, please"',
     :table => 'filter',
@@ -567,6 +575,15 @@ HASH_TO_ARGS = {
     :args => ["-t", :filter, "-p", :tcp, "-m", "comment", "--comment", "100 states_set_from_array",
       "-m", "state", "--state", "ESTABLISHED,INVALID"],
   },
+  'ctstates_set_from_array' => {
+    :params => {
+      :name => "100 ctstates_set_from_array",
+      :table => "filter",
+      :ctstate => ['ESTABLISHED', 'INVALID']
+    },
+    :args => ["-t", :filter, "-p", :tcp, "-m", "comment", "--comment", "100 ctstates_set_from_array",
+      "-m", "conntrack", "--ctstate", "ESTABLISHED,INVALID"],
+  },
   'comment_string_character_validation' => {
     :params => {
       :name => "000 allow from 192.168.0.1, please",

diff --git a/spec/system/params_spec.rb b/spec/system/params_spec.rb
index 497a042b55618a62b09d90cb95463978dc18ddb0..f6d6356c5bc10224d85f8234d1bb219a2cfd0e1d 100644 (file)
--- a/spec/system/params_spec.rb
+++ b/spec/system/params_spec.rb
@@ -71,7 +71,7 @@ firewall { '#{name}':
       'name' => '004 log all INVALID packets',
       'chain' => 'INPUT',
       'proto' => 'all',
-      'state' => 'INVALID',
+      'ctstate' => 'INVALID',
       'jump' => 'LOG',
       'log_level' => '3',
       'log_prefix' => '"IPTABLES dropped invalid: "',
@@ -81,7 +81,7 @@ firewall { '#{name}':
       'name' => '003 log all INVALID packets',
       'chain' => 'INPUT',
       'proto' => 'all',
-      'state' => 'INVALID',
+      'ctstate' => 'INVALID',
       'jump' => 'LOG',
       'log_level' => '3',
       'log_prefix' => '"IPTABLES dropped invalid: "',
@@ -110,7 +110,7 @@ firewall { '#{name}':
       'name' => '004 log all INVALID packets',
       'chain' => 'INPUT',
       'proto' => 'all',
-      'state' => 'INVALID',
+      'ctstate' => 'INVALID',
       'jump' => 'LOG',
       'log_level' => '3',
       'log_prefix' => '"IPTABLES dropped invalid: "',

diff --git a/spec/system/standard_usage_spec.rb b/spec/system/standard_usage_spec.rb
index 5db7e021a6c05f55ff9302515e2bc1e9493d89ec..49d4ce302ad0b6b508fafe33c11381856e6eb251 100644 (file)
--- a/spec/system/standard_usage_spec.rb
+++ b/spec/system/standard_usage_spec.rb
@@ -21,7 +21,7 @@ describe 'standard usage tests:' do
         }->
         firewall { '002 accept related established rules':
           proto   => 'all',
-          state   => ['RELATED', 'ESTABLISHED'],
+          ctstate => ['RELATED', 'ESTABLISHED'],
           action  => 'accept',
         }
       }

diff --git a/spec/unit/puppet/type/firewall_spec.rb b/spec/unit/puppet/type/firewall_spec.rb
index 1f9dc9614cc6075b01d73a039daa3305c6ad7eab..ed1cbbcfb030af23779472bc919591baa02ac936 100755 (executable)
--- a/spec/unit/puppet/type/firewall_spec.rb
+++ b/spec/unit/puppet/type/firewall_spec.rb
@@ -316,6 +316,23 @@ describe firewall do
     end
   end
 
+  describe ':ctstate' do
+    it 'should accept value as a string' do
+      @resource[:ctstate] = :INVALID
+      @resource[:ctstate].should == [:INVALID]
+    end
+
+    it 'should accept value as an array' do
+      @resource[:ctstate] = [:INVALID, :NEW]
+      @resource[:ctstate].should == [:INVALID, :NEW]
+    end
+
+    it 'should sort values alphabetically' do
+      @resource[:ctstate] = [:NEW, :ESTABLISHED]
+      @resource[:ctstate].should == [:ESTABLISHED, :NEW]
+    end
+  end
+
   describe ':burst' do
     it 'should accept numeric values' do
       @resource[:burst] = 12