From 00e19278c9644f457624beb0fb65ed007ca3de2a Mon Sep 17 00:00:00 2001 From: Jonathan Tripathy Date: Wed, 4 Mar 2015 12:20:00 +0000 Subject: [PATCH] MODULES-1808 - Implemented code for resource map munging to allow a single ipt module to be used multiple times in a single rule on older versions of iptables --- lib/puppet/provider/firewall/ip6tables.rb | 41 +++- lib/puppet/provider/firewall/iptables.rb | 116 +++++++---- spec/acceptance/firewall_iptmodules_spec.rb | 218 ++++++++++++++++++++ 3 files changed, 326 insertions(+), 49 deletions(-) create mode 100644 spec/acceptance/firewall_iptmodules_spec.rb diff --git a/lib/puppet/provider/firewall/ip6tables.rb b/lib/puppet/provider/firewall/ip6tables.rb index b0ebbcd..b1e3902 100644 --- a/lib/puppet/provider/firewall/ip6tables.rb +++ b/lib/puppet/provider/firewall/ip6tables.rb @@ -71,9 +71,9 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source = :ctstate => "-m conntrack --ctstate", :destination => "-d", :dport => ["-m multiport --dports", "--dport"], - :dst_range => '-m iprange --dst-range', - :dst_type => "-m addrtype --dst-type", - :gid => "-m owner --gid-owner", + :dst_range => '--dst-range', + :dst_type => "--dst-type", + :gid => "--gid-owner", :hop_limit => "-m hl --hl-eq", :icmp => "-m icmp6 --icmpv6-type", :iniface => "-i", @@ -107,8 +107,8 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source = :socket => "-m socket", :source => "-s", :sport => ["-m multiport --sports", "--sport"], - :src_range => '-m iprange --src-range', - :src_type => "-m addrtype --src-type", + :src_range => '--src-range', + :src_type => "--src-type", :stat_every => '--every', :stat_mode => "-m statistic --mode", :stat_packet => '--packet', @@ -119,10 +119,10 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source = :todest => "--to-destination", :toports => "--to-ports", :tosource => "--to-source", - :uid => "-m owner --uid-owner", - :physdev_in => "-m physdev --physdev-in", - :physdev_out => "-m physdev --physdev-out", - :physdev_is_bridged => "-m physdev --physdev-is-bridged" + :uid => "--uid-owner", + :physdev_in => "--physdev-in", + :physdev_out => "--physdev-out", + :physdev_is_bridged => "--physdev-is-bridged" } # These are known booleans that do not take a value, but we want to munge @@ -139,6 +139,25 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source = :physdev_is_bridged ] + # Properties that use "-m " (with the potential to have multiple + # arguments against the same IPT module) must be in this hash. The keys in this + # hash are the IPT module names, with the values being an array of the respective + # supported arguments for this IPT module. + # + # ** IPT Module arguments must be in order as they would appear in iptables-save ** + # + # Exceptions: + # => multiport: (For some reason, the multiport arguments can't be) + # specified within the same "-m multiport", but works in seperate + # ones. + # + @module_to_argument_mapping = { + :physdev => [:physdev_in, :physdev_out, :physdev_is_bridged], + :addrtype => [:src_type, :dst_type], + :iprange => [:src_range, :dst_range], + :owner => [:uid, :gid], + } + # Create property methods dynamically (@resource_map.keys << :chain << :table << :action).each do |property| if @known_booleans.include?(property) then @@ -175,8 +194,8 @@ Puppet::Type.type(:firewall).provide :ip6tables, :parent => :iptables, :source = # not provided with current parser [georg.koester]) @resource_list = [:table, :source, :destination, :iniface, :outiface, :physdev_in, :physdev_out, :physdev_is_bridged, :proto, :ishasmorefrags, :islastfrag, :isfirstfrag, :src_range, :dst_range, - :tcp_flags, :gid, :uid, :mac_source, :sport, :dport, :port, :dst_type, - :src_type, :socket, :pkttype, :name, :ipsec_dir, :ipsec_policy, :state, + :tcp_flags, :uid, :gid, :mac_source, :sport, :dport, :port, :src_type, + :dst_type, :socket, :pkttype, :name, :ipsec_dir, :ipsec_policy, :state, :ctstate, :icmp, :hop_limit, :limit, :burst, :recent, :rseconds, :reap, :rhitcount, :rttl, :rname, :mask, :rsource, :rdest, :ipset, :jump, :todest, :tosource, :toports, :log_level, :log_prefix, :reject, :set_mark, diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb index 0aa5d71..b4713ee 100644 --- a/lib/puppet/provider/firewall/iptables.rb +++ b/lib/puppet/provider/firewall/iptables.rb @@ -57,9 +57,9 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir :ctstate => "-m conntrack --ctstate", :destination => "-d", :dport => ["-m multiport --dports", "--dport"], - :dst_range => "-m iprange --dst-range", - :dst_type => "-m addrtype --dst-type", - :gid => "-m owner --gid-owner", + :dst_range => "--dst-range", + :dst_type => "--dst-type", + :gid => "--gid-owner", :icmp => "-m icmp --icmp-type", :iniface => "-i", :ipsec_dir => "-m policy --dir", @@ -91,8 +91,8 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir :socket => "-m socket", :source => "-s", :sport => ["-m multiport --sports", "--sport"], - :src_range => "-m iprange --src-range", - :src_type => "-m addrtype --src-type", + :src_range => "--src-range", + :src_type => "--src-type", :stat_every => '--every', :stat_mode => "-m statistic --mode", :stat_packet => '--packet', @@ -104,10 +104,10 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir :toports => "--to-ports", :tosource => "--to-source", :to => "--to", - :uid => "-m owner --uid-owner", - :physdev_in => "-m physdev --physdev-in", - :physdev_out => "-m physdev --physdev-out", - :physdev_is_bridged => "-m physdev --physdev-is-bridged" + :uid => "--uid-owner", + :physdev_in => "--physdev-in", + :physdev_out => "--physdev-out", + :physdev_is_bridged => "--physdev-is-bridged" } # These are known booleans that do not take a value, but we want to munge @@ -123,6 +123,67 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir :physdev_is_bridged ] + # Properties that use "-m " (with the potential to have multiple + # arguments against the same IPT module) must be in this hash. The keys in this + # hash are the IPT module names, with the values being an array of the respective + # supported arguments for this IPT module. + # + # ** IPT Module arguments must be in order as they would appear in iptables-save ** + # + # Exceptions: + # => multiport: (For some reason, the multiport arguments can't be) + # specified within the same "-m multiport", but works in seperate + # ones. + # + @module_to_argument_mapping = { + :physdev => [:physdev_in, :physdev_out, :physdev_is_bridged], + :addrtype => [:src_type, :dst_type], + :iprange => [:src_range, :dst_range], + :owner => [:uid, :gid], + } + + def self.munge_resource_map_from_existing_values(resource_map_original, compare) + resource_map_new = resource_map_original.clone + + @module_to_argument_mapping.each do |ipt_module, arg_array| + arg_array.each do |argument| + if resource_map_original[argument].is_a?(Array) + if compare.include?(resource_map_original[argument].first) + resource_map_new[argument] = resource_map_original[argument].clone + resource_map_new[argument][0] = "-m #{ipt_module.to_s} #{resource_map_original[argument].first}" + break + end + else + if compare.include?(resource_map_original[argument]) + resource_map_new[argument] = "-m #{ipt_module.to_s} #{resource_map_original[argument]}" + break + end + end + end + end + resource_map_new + end + + def munge_resource_map_from_resource(resource_map_original, compare) + resource_map_new = resource_map_original.clone + module_to_argument_mapping = self.class.instance_variable_get('@module_to_argument_mapping') + + module_to_argument_mapping.each do |ipt_module, arg_array| + arg_array.each do |argument| + if compare[argument] + if resource_map_original[argument].is_a?(Array) + resource_map_new[argument] = resource_map_original[argument].clone + resource_map_new[argument][0] = "-m #{ipt_module.to_s} #{resource_map_original[argument].first}" + else + resource_map_new[argument] = "-m #{ipt_module.to_s} #{resource_map_original[argument]}" + end + break + end + end + end + resource_map_new + end + # Create property methods dynamically (@resource_map.keys << :chain << :table << :action).each do |property| @@ -158,8 +219,8 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir @resource_list = [ :table, :source, :destination, :iniface, :outiface, :physdev_in, :physdev_out, :physdev_is_bridged, :proto, :isfragment, :stat_mode, :stat_every, :stat_packet, :stat_probability, - :src_range, :dst_range, :tcp_flags, :gid, :uid, :mac_source, :sport, :dport, :port, - :dst_type, :src_type, :socket, :pkttype, :name, :ipsec_dir, :ipsec_policy, + :src_range, :dst_range, :tcp_flags, :uid, :gid, :mac_source, :sport, :dport, :port, + :src_type, :dst_type, :socket, :pkttype, :name, :ipsec_dir, :ipsec_policy, :state, :ctstate, :icmp, :limit, :burst, :recent, :rseconds, :reap, :rhitcount, :rttl, :rname, :mask, :rsource, :rdest, :ipset, :jump, :todest, :tosource, :toports, :to, :random, :log_prefix, :log_level, :reject, :set_mark, @@ -252,17 +313,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir '--pol "ipsec\1\2\3\4\5\6\7\8" ' ) - # Handle resource_map values depending on whether physdev-in, physdev-out, ,physdev-is-bridged, or all three are specified - if values.include? "--physdev-in" - @resource_map[:physdev_in] = "-m physdev --physdev-in" - @resource_map[:physdev_out] = "--physdev-out" - @resource_map[:physdev_is_bridged] = "--physdev-is-bridged" - elsif values.include? "--physdev-out" - @resource_map[:physdev_out] = "-m physdev --physdev-out" - @resource_map[:physdev_is_bridged] = "--physdev-is-bridged" - else - @resource_map[:physdev_is_bridged] = "-m physdev --physdev-is-bridged" - end + resource_map = munge_resource_map_from_existing_values(@resource_map, values) # Trick the system for booleans @known_booleans.each do |bool| @@ -273,7 +324,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir # distinguish between -f and the '-f' inside of --tcp-flags. values = values.sub(/-f(?!l)(?=.*--comment)/, '-f true') else - values = values.sub(/#{@resource_map[bool]}/, "#{@resource_map[bool]} true") + values = values.sub(/#{resource_map[bool]}/, "#{resource_map[bool]} true") end end @@ -281,7 +332,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir # Populate parser_list with used value, in the correct order ############ map_index={} - @resource_map.each_pair do |map_k,map_v| + resource_map.each_pair do |map_k,map_v| [map_v].flatten.each do |v| ind=values.index(/\s#{v}\s/) next unless ind @@ -298,7 +349,7 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir # Here we iterate across our values to generate an array of keys parser_list.reverse.each do |k| - resource_map_key = @resource_map[k] + resource_map_key = resource_map[k] [resource_map_key].flatten.each do |opt| if values.slice!(/\s#{opt}/) keys << k @@ -455,20 +506,9 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir args = [] resource_list = self.class.instance_variable_get('@resource_list') - resource_map = self.class.instance_variable_get('@resource_map') known_booleans = self.class.instance_variable_get('@known_booleans') - - # Handle physdev args depending on whether physdev-in, physdev-out, physdev-is-bridged, or all three are specified - if (resource[:physdev_in]) - resource_map[:physdev_in] = "-m physdev --physdev-in" - resource_map[:physdev_out] = "--physdev-out" - resource_map[:physdev_is_bridged] = "--physdev-is-bridged" - elsif (resource[:physdev_out]) - resource_map[:physdev_out] = "-m physdev --physdev-out" - resource_map[:physdev_is_bridged] = "--physdev-is-bridged" - else - resource_map[:physdev_is_bridged] = "-m physdev --physdev-is-bridged" - end + resource_map = self.class.instance_variable_get('@resource_map') + resource_map = munge_resource_map_from_resource(resource_map, resource) resource_list.each do |res| resource_value = nil diff --git a/spec/acceptance/firewall_iptmodules_spec.rb b/spec/acceptance/firewall_iptmodules_spec.rb new file mode 100644 index 0000000..427e851 --- /dev/null +++ b/spec/acceptance/firewall_iptmodules_spec.rb @@ -0,0 +1,218 @@ + require 'spec_helper_acceptance' + +describe 'firewall type', :unless => UNSUPPORTED_PLATFORMS.include?(fact('osfamily')) do + + describe 'reset' do + it 'deletes all iptables rules' do + shell('iptables --flush; iptables -t nat --flush; iptables -t mangle --flush') + end + it 'deletes all ip6tables rules' do + shell('ip6tables --flush; ip6tables -t nat --flush; ip6tables -t mangle --flush') + end + end + + describe 'iptables ipt_modules tests' do + context 'all the modules with multiple args' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '801 - ipt_modules tests': + proto => tcp, + dport => '8080', + action => reject, + chain => 'OUTPUT', + uid => 500, + gid => 404, + src_range => "90.0.0.1-90.0.0.2", + dst_range => "100.0.0.1-100.0.0.2", + src_type => 'LOCAL', + dst_type => 'UNICAST', + physdev_in => "eth0", + physdev_out => "eth1", + physdev_is_bridged => true, + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('iptables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -p tcp -m physdev\s+--physdev-in eth0 --physdev-out eth1 --physdev-is-bridged -m iprange --src-range 90.0.0.1-90.0.0.2\s+--dst-range 100.0.0.1-100.0.0.2 -m owner --uid-owner 500 --gid-owner 404 -m multiport --dports 8080 -m addrtype --src-type LOCAL --dst-type UNICAST -m comment --comment "801 - ipt_modules tests" -j REJECT --reject-with icmp-port-unreachable/) + end + end + end + + context 'all the modules with single args' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '802 - ipt_modules tests': + proto => tcp, + dport => '8080', + action => reject, + chain => 'OUTPUT', + gid => 404, + dst_range => "100.0.0.1-100.0.0.2", + dst_type => 'UNICAST', + physdev_out => "eth1", + physdev_is_bridged => true, + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('iptables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -p tcp -m physdev\s+--physdev-out eth1 --physdev-is-bridged -m iprange --dst-range 100.0.0.1-100.0.0.2 -m owner --gid-owner 404 -m multiport --dports 8080 -m addrtype --dst-type UNICAST -m comment --comment "802 - ipt_modules tests" -j REJECT --reject-with icmp-port-unreachable/) + end + end + end + end + + #iptables version 1.3.5 is not suppored by the ip6tables provider + if default['platform'] =~ /debian-7/ or default['platform'] =~ /ubuntu-14\.04/ + describe 'ip6tables ipt_modules tests' do + context 'all the modules with multiple args' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '801 - ipt_modules tests': + proto => tcp, + dport => '8080', + action => reject, + chain => 'OUTPUT', + provider => 'ip6tables', + uid => 500, + gid => 404, + src_range => "2001::-2002::", + dst_range => "2003::-2004::", + src_type => 'LOCAL', + dst_type => 'UNICAST', + physdev_in => "eth0", + physdev_out => "eth1", + physdev_is_bridged => true, + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('ip6tables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -p tcp -m physdev\s+--physdev-in eth0 --physdev-out eth1 --physdev-is-bridged -m iprange --src-range 2001::-2002::\s+--dst-range 2003::-2004:: -m owner --uid-owner 500 --gid-owner 404 -m multiport --dports 8080 -m addrtype --src-type LOCAL --dst-type UNICAST -m comment --comment "801 - ipt_modules tests" -j REJECT --reject-with icmp6-port-unreachable/) + end + end + end + + context 'all the modules with single args' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '802 - ipt_modules tests': + proto => tcp, + dport => '8080', + action => reject, + chain => 'OUTPUT', + provider => 'ip6tables', + gid => 404, + dst_range => "2003::-2004::", + dst_type => 'UNICAST', + physdev_out => "eth1", + physdev_is_bridged => true, + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('ip6tables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -p tcp -m physdev\s+--physdev-out eth1 --physdev-is-bridged -m iprange --dst-range 2003::-2004:: -m owner --gid-owner 404 -m multiport --dports 8080 -m addrtype --dst-type UNICAST -m comment --comment "802 - ipt_modules tests" -j REJECT --reject-with icmp6-port-unreachable/) + end + end + end + end + # Older OSes don't have addrtype so we leave those properties out. + # el-5 doesn't support ipv6 by default + elsif default['platform'] !~ /el-5/ + describe 'ip6tables ipt_modules tests' do + context 'all the modules with multiple args' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '801 - ipt_modules tests': + proto => tcp, + dport => '8080', + action => reject, + chain => 'OUTPUT', + provider => 'ip6tables', + uid => 500, + gid => 404, + src_range => "2001::-2002::", + dst_range => "2003::-2004::", + physdev_in => "eth0", + physdev_out => "eth1", + physdev_is_bridged => true, + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('ip6tables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -p tcp -m physdev\s+--physdev-in eth0 --physdev-out eth1 --physdev-is-bridged -m iprange --src-range 2001::-2002::\s+--dst-range 2003::-2004:: -m owner --uid-owner 500 --gid-owner 404 -m multiport --dports 8080 -m comment --comment "801 - ipt_modules tests" -j REJECT --reject-with icmp6-port-unreachable/) + end + end + end + + context 'all the modules with single args' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '802 - ipt_modules tests': + proto => tcp, + dport => '8080', + action => reject, + chain => 'OUTPUT', + provider => 'ip6tables', + gid => 404, + dst_range => "2003::-2004::", + physdev_out => "eth1", + physdev_is_bridged => true, + } + EOS + + apply_manifest(pp, :catch_failures => true) + unless fact('selinux') == 'true' + apply_manifest(pp, :catch_changes => true) + end + end + + it 'should contain the rule' do + shell('ip6tables-save') do |r| + expect(r.stdout).to match(/-A OUTPUT -p tcp -m physdev\s+--physdev-out eth1 --physdev-is-bridged -m iprange --dst-range 2003::-2004:: -m owner --gid-owner 404 -m multiport --dports 8080 -m comment --comment "802 - ipt_modules tests" -j REJECT --reject-with icmp6-port-unreachable/) + end + end + end + end + end + +end \ No newline at end of file -- 2.45.2