From: Colin Shea Date: Wed, 16 Oct 2013 01:37:26 +0000 (-0700) Subject: Support conntrack stateful firewall matching X-Git-Tag: 0.5.0~25^2 X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=13457a4ade45f4a46d64ceb4da9d2b9582c39fcd;p=puppet-modules%2Fpuppetlabs-firewall.git 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 467e835..827ef6f 100644 --- 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 1a7b16c..56319b1 100644 --- 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 7caf952..93d7edc 100644 --- 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 0e4bb79..f887194 100644 --- 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 cf4d868..580dc48 100644 --- 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 497a042..f6d6356 100644 --- 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 5db7e02..49d4ce3 100644 --- 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 1f9dc96..ed1cbbc 100755 --- 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