From 948a76c790783a8fae571d64382fb90884260605 Mon Sep 17 00:00:00 2001 From: Jonathan Tripathy Date: Sun, 18 Jan 2015 14:11:58 -0800 Subject: [PATCH] Added support for seperate physdev-in and physdev-out parameters. --- README.markdown | 4 +- lib/puppet/provider/firewall/ip6tables.rb | 7 +- lib/puppet/provider/firewall/iptables.rb | 20 ++- lib/puppet/type/firewall.rb | 25 ++- spec/acceptance/firewall_bridging_spec.rb | 182 ++++++++++++++++++++++ 5 files changed, 216 insertions(+), 22 deletions(-) create mode 100644 spec/acceptance/firewall_bridging_spec.rb diff --git a/README.markdown b/README.markdown index 5e60325f8..e21937c65 100644 --- a/README.markdown +++ b/README.markdown @@ -339,12 +339,12 @@ This type enables you to manage firewall rules within Puppet. * `ip6tables`: Ip6tables type provider * Required binaries: `ip6tables-save`, `ip6tables`. - * Supported features: `connection_limiting`, `dnat`, `hop_limiting`, `icmp_match`, `interface_match`, `ipsec_dir`, `ipsec_policy`, `ipset`, `iptables`, `isfirstfrag`, `ishasmorefrags`, `islastfrag`, `log_level`, `log_prefix`, `mark`, `mask`, `owner`, `pkttype`, `rate_limiting`, `recent_limiting`, `reject_type`, `snat`, `socket`, `state_match`, `tcp_flags`. + * Supported features: `connection_limiting`, `dnat`, `hop_limiting`, `icmp_match`, `interface_match`, `ipsec_dir`, `ipsec_policy`, `ipset`, `iptables`, `isfirstfrag`, `ishasmorefrags`, `islastfrag`, `log_level`, `log_prefix`, `mark`, `mask`, `owner`, `physdev_in`, `physdev_out`, `pkttype`, `rate_limiting`, `recent_limiting`, `reject_type`, `snat`, `socket`, `state_match`, `tcp_flags`. * `iptables`: Iptables type provider * Required binaries: `iptables-save`, `iptables`. * Default for `kernel` == `linux`. - * Supported features: `address_type`, `connection_limiting`, `dnat`, `icmp_match`, `interface_match`, `iprange`, `ipsec_dir`, `ipsec_policy`, `ipset`, `iptables`, `isfragment`, `log_level`, `log_prefix`, `mark`, `mask`, `owner`, `pkttype`, `rate_limiting`, `recent_limiting`, `reject_type`, `snat`, `socket`, `state_match`, `tcp_flags`, `netmap`. + * Supported features: `address_type`, `connection_limiting`, `dnat`, `icmp_match`, `interface_match`, `iprange`, `ipsec_dir`, `ipsec_policy`, `ipset`, `iptables`, `isfragment`, `log_level`, `log_prefix`, `mark`, `mask`, `owner`, `physdev_in`, `physdev_out`, `pkttype`, `rate_limiting`, `recent_limiting`, `reject_type`, `snat`, `socket`, `state_match`, `tcp_flags`, `netmap`. **Autorequires:** diff --git a/lib/puppet/provider/firewall/ip6tables.rb b/lib/puppet/provider/firewall/ip6tables.rb index 9139045a8..b5df9a7a4 100644 --- a/lib/puppet/provider/firewall/ip6tables.rb +++ b/lib/puppet/provider/firewall/ip6tables.rb @@ -119,7 +119,8 @@ def self.iptables_save(*args) :toports => "--to-ports", :tosource => "--to-source", :uid => "-m owner --uid-owner", - :bridge => "-m physdev", + :physdev_in => "-m physdev --physdev-in", + :physdev_out => "-m physdev --physdev-out", } # These are known booleans that do not take a value, but we want to munge @@ -169,8 +170,8 @@ def self.iptables_save(*args) # (Note: on my CentOS 6.4 ip6tables-save returns -m frag on the place # I put it when calling the command. So compability with manual changes # not provided with current parser [georg.koester]) - @resource_list = [:table, :source, :destination, :iniface, :outiface, - :proto, :ishasmorefrags, :islastfrag, :isfirstfrag, :src_range, :dst_range, + @resource_list = [:table, :source, :destination, :iniface, :outiface, :physdev_in, + :physdev_out, :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, :ctstate, :icmp, :hop_limit, :limit, :burst, :recent, :rseconds, :reap, diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb index 56c869a83..2a1b7d59c 100644 --- a/lib/puppet/provider/firewall/iptables.rb +++ b/lib/puppet/provider/firewall/iptables.rb @@ -105,7 +105,8 @@ :tosource => "--to-source", :to => "--to", :uid => "-m owner --uid-owner", - :bridge => "-m physdev", + :physdev_in => "-m physdev --physdev-in", + :physdev_out => "-m physdev --physdev-out", } # These are known booleans that do not take a value, but we want to munge @@ -153,7 +154,7 @@ # changes between puppet runs, the changed rules will be re-applied again. # This order can be determined by going through iptables source code or just tweaking and trying manually @resource_list = [ - :table, :source, :destination, :iniface, :outiface, :proto, :isfragment, + :table, :source, :destination, :iniface, :outiface, :physdev_in, :physdev_out, :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, @@ -262,6 +263,14 @@ def self.rule_to_hash(line, table, counter) end end + # Handle resource_map values depending on whether physdev-in, physdev-out, or both are specified + if values.include? "--physdev-in" and values.include? "--physdev-out" then + #values = values.sub("--physdev-out","-m physdev --physdev-out") + @resource_map[:physdev_out] = "--physdev-out" + else + @resource_map[:physdev_out] = "-m physdev --physdev-out" + end + ############ # Populate parser_list with used value, in the correct order ############ @@ -443,6 +452,13 @@ def general_args 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, or both are specified + if (resource[:physdev_in]) + resource_map[:physdev_out] = "--physdev-out" + else + resource_map[:physdev_out] = "-m physdev --physdev-out" + end + resource_list.each do |res| resource_value = nil if (resource[res]) then diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb index ebfc3b862..e7cb04b71 100644 --- a/lib/puppet/type/firewall.rb +++ b/lib/puppet/type/firewall.rb @@ -1033,17 +1033,18 @@ def should_to_s(value) newvalues(/^([0-9a-f]{2}[:]){5}([0-9a-f]{2})$/i) end - newproperty(:bridge, :required_features => :iptables) do + newproperty(:physdev_in, :required_features => :iptables) do desc <<-EOS - Match if the packet is being bridged. + Match if the packet is entering a bridge from the given interface. EOS - munge do |value| - if ! value.to_s.start_with?("--") - "--" + value.to_s - else - value - end - end + newvalues(/^[a-zA-Z0-9\-\._\+]+$/) + end + + newproperty(:physdev_out, :required_features => :iptables) do + desc <<-EOS + Match if the packet is leaving a bridge via the given interface. + EOS + newvalues(/^[a-zA-Z0-9\-\._\+]+$/) end autorequire(:firewallchain) do @@ -1204,11 +1205,5 @@ def should_to_s(value) self.fail "Parameter 'stat_probability' requires 'stat_mode' to be set to 'random'" end - if value(:bridge) - unless value(:chain).to_s =~ /FORWARD/ - self.fail "Parameter bridge only applies to the FORWARD chain" - end - end - end end diff --git a/spec/acceptance/firewall_bridging_spec.rb b/spec/acceptance/firewall_bridging_spec.rb new file mode 100644 index 000000000..109806a42 --- /dev/null +++ b/spec/acceptance/firewall_bridging_spec.rb @@ -0,0 +1,182 @@ + 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 physdev tests' do + context 'physdev_in eth0' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '701 - test': + chain => 'FORWARD', + proto => tcp, + port => '701', + action => accept, + physdev_in => 'eth0', + } + 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 FORWARD -p tcp -m physdev\s+--physdev-in eth0 -m multiport --ports 701 -m comment --comment "701 - test" -j ACCEPT/) + end + end + end + + context 'physdev_out eth1' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '702 - test': + chain => 'FORWARD', + proto => tcp, + port => '702', + action => accept, + physdev_out => 'eth1', + } + 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 FORWARD -p tcp -m physdev\s+--physdev-out eth1 -m multiport --ports 702 -m comment --comment "702 - test" -j ACCEPT/) + end + end + end + + context 'physdev_in eth0 and physdev_out eth1' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '703 - test': + chain => 'FORWARD', + proto => tcp, + port => '703', + action => accept, + physdev_in => 'eth0', + physdev_out => 'eth1', + } + 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 FORWARD -p tcp -m physdev\s+--physdev-in eth0 --physdev-out eth1 -m multiport --ports 703 -m comment --comment "703 - test" -j ACCEPT/) + end + end + end + end + + #iptables version 1.3.5 is not suppored by the ip6tables provider + if default['platform'] !~ /el-5/ + describe 'ip6tables physdev tests' do + context 'physdev_in eth0' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '701 - test': + provider => 'ip6tables', + chain => 'FORWARD', + proto => tcp, + port => '701', + action => accept, + physdev_in => 'eth0', + } + 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 FORWARD -p tcp -m physdev\s+--physdev-in eth0 -m multiport --ports 701 -m comment --comment "701 - test" -j ACCEPT/) + end + end + end + + context 'physdev_out eth1' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '702 - test': + provider => 'ip6tables', + chain => 'FORWARD', + proto => tcp, + port => '702', + action => accept, + physdev_out => 'eth1', + } + 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 FORWARD -p tcp -m physdev\s+--physdev-out eth1 -m multiport --ports 702 -m comment --comment "702 - test" -j ACCEPT/) + end + end + end + + context 'physdev_in eth0 and physdev_out eth1' do + it 'applies' do + pp = <<-EOS + class { '::firewall': } + firewall { '703 - test': + provider => 'ip6tables', + chain => 'FORWARD', + proto => tcp, + port => '703', + action => accept, + physdev_in => 'eth0', + physdev_out => 'eth1', + } + 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 FORWARD -p tcp -m physdev\s+--physdev-in eth0 --physdev-out eth1 -m multiport --ports 703 -m comment --comment "703 - test" -j ACCEPT/) + end + end + end + end + end + +end \ No newline at end of file