Permalink
Browse files

(#14755) Fix mark to not repeat rules with iptables 1.4.1+.

  • Loading branch information...
1 parent c991c2b commit ff2e35584d6b5565dc80e41d8bb436a2c09a7251 Sharif Nassar committed with kbarber May 30, 2012
View
1 LICENSE
@@ -2,6 +2,7 @@ Puppet Firewall Module - Puppet module for managing Firewalls
Copyright (C) 2011 Puppet Labs, Inc.
Copyright (C) 2011 Jonathan Boyett
+Copyright (C) 2011 Media Temple, Inc.
Some of the iptables code was taken from puppet-iptables which was:
View
12 lib/facter/iptables.rb → lib/facter/ip6tables_version.rb
@@ -1,15 +1,3 @@
-Facter.add(:iptables_version) do
- confine :kernel => :linux
- setcode do
- version = Facter::Util::Resolution.exec('iptables --version')
- if version
- version.match(/\d+\.\d+\.\d+/).to_s
- else
- nil
- end
- end
-end
-
Facter.add(:ip6tables_version) do
confine :kernel => :linux
setcode do
View
11 lib/facter/iptables_version.rb
@@ -0,0 +1,11 @@
+Facter.add(:iptables_version) do
+ confine :kernel => :linux
+ setcode do
+ version = Facter::Util::Resolution.exec('iptables --version')
+ if version
+ version.match(/\d+\.\d+\.\d+/).to_s
+ else
+ nil
+ end
+ end
+end
View
11 lib/puppet/provider/firewall/iptables.rb
@@ -26,6 +26,13 @@
defaultfor :kernel => :linux
+ iptables_version = Facter.fact('iptables_version').value
+ if (iptables_version and Puppet::Util::Package.versioncmp(iptables_version, '1.4.1') < 0)
+ mark_flag = '--set-mark'
+ else
+ mark_flag = '--set-xmark'
+ end
+
@resource_map = {
:burst => "--limit-burst",
:destination => "-d",
@@ -42,16 +49,16 @@
:port => '-m multiport --ports',
:proto => "-p",
:reject => "--reject-with",
+ :set_mark => mark_flag,
:source => "-s",
- :state => "-m state --state",
:sport => "-m multiport --sports",
+ :state => "-m state --state",
:table => "-t",
:tcp_flags => "-m tcp --tcp-flags",
:todest => "--to-destination",
:toports => "--to-ports",
:tosource => "--to-source",
:uid => "-m owner --uid-owner",
- :set_mark => "--set-mark",
:pkttype => "-m pkttype --pkt-type"
}
View
36 lib/puppet/type/firewall.rb
@@ -477,15 +477,43 @@ def should_to_s(value)
newproperty(:set_mark, :required_features => :mark) do
desc <<-EOS
- Set the Netfilter mark value associated with the packet.
+ Set the Netfilter mark value associated with the packet. Accepts either of:
+ mark/mask or mark. These will be converted to hex if they are not already.
EOS
munge do |value|
- if ! value.to_s.include?("0x")
- "0x" + value.to_i.to_s(16)
+ int_or_hex = '[a-fA-F0-9x]'
+ match = value.to_s.match("(#{int_or_hex}+)(/)?(#{int_or_hex}+)?")
+ mark = @resource.to_hex32(match[1])
+
+ # Values that can't be converted to hex.
+ # Or contain a trailing slash with no mask.
+ if mark.nil? or (mark and match[2] and match[3].nil?)
+ raise ArgumentError, "MARK value must be integer or hex between 0 and 0xffffffff"
+ end
+
+ # Old iptables does not support a mask. New iptables will expect one.
+ iptables_version = Facter.fact('iptables_version').value
+ mask_required = (iptables_version and Puppet::Util::Package.versioncmp(iptables_version, '1.4.1') >= 0)
+
+ if mask_required
+ if match[3].nil?
+ value = "#{mark}/0xffffffff"
+ else
+ mask = @resource.to_hex32(match[3])
+ if mask.nil?
+ raise ArgumentError, "MARK mask must be integer or hex between 0 and 0xffffffff"
+ end
+ value = "#{mark}/#{mask}"
+ end
else
- super(value)
+ unless match[3].nil?
+ raise ArgumentError, "iptables version #{iptables_version} does not support masks on MARK rules"
+ end
+ value = mark
end
+
+ value
end
end
View
14 lib/puppet/util/firewall.rb
@@ -96,4 +96,18 @@ def host_to_ip(value)
return nil if value.prefixlen == 0
value.cidr
end
+
+ # Validates the argument is int or hex, and returns valid hex
+ # conversion of the value or nil otherwise.
+ def to_hex32(value)
+ begin
+ value = Integer(value)
+ if value.between?(0, 0xffffffff)
+ return '0x' + value.to_s(16)
+ end
+ rescue ArgumentError
+ # pass
+ end
+ return nil
+ end
end
View
40 spec/fixtures/iptables/conversion_hash.rb
@@ -219,12 +219,12 @@
},
},
'mark_set-mark' => {
- :line => '-t mangle -A PREROUTING -j MARK --set-mark 1000',
+ :line => '-t mangle -A PREROUTING -j MARK --set-xmark 0x3e8/0xffffffff',
:table => 'mangle',
:params => {
:jump => 'MARK',
:chain => 'PREROUTING',
- :set_mark => '1000',
+ :set_mark => '0x3e8/0xffffffff',
}
},
'iniface_1' => {
@@ -532,17 +532,47 @@
},
:args => ['-t', :mangle, '-p', :all, '-m', 'owner', '--gid-owner', 'root', '-m', 'comment', '--comment', '057 POSTROUTING gid root only', '-j', 'ACCEPT'],
},
- 'mark_set-mark' => {
+ 'mark_set-mark_int' => {
:params => {
:name => '058 set-mark 1000',
:table => 'mangle',
:jump => 'MARK',
:chain => 'PREROUTING',
:set_mark => '1000',
},
- :args => ['-t', :mangle, '-p', :tcp, '-m', 'comment', '--comment', '058 set-mark 1000', '-j', 'MARK', '--set-mark', '0x3e8'],
+ :args => ['-t', :mangle, '-p', :tcp, '-m', 'comment', '--comment', '058 set-mark 1000', '-j', 'MARK', '--set-xmark', '0x3e8/0xffffffff'],
},
- 'iniface_1' => {
+ 'mark_set-mark_hex' => {
+ :params => {
+ :name => '058 set-mark 0x32',
+ :table => 'mangle',
+ :jump => 'MARK',
+ :chain => 'PREROUTING',
+ :set_mark => '0x32',
+ },
+ :args => ['-t', :mangle, '-p', :tcp, '-m', 'comment', '--comment', '058 set-mark 0x32', '-j', 'MARK', '--set-xmark', '0x32/0xffffffff'],
+ },
+ 'mark_set-mark_hex_with_hex_mask' => {
+ :params => {
+ :name => '058 set-mark 0x32/0xffffffff',
+ :table => 'mangle',
+ :jump => 'MARK',
+ :chain => 'PREROUTING',
+ :set_mark => '0x32/0xffffffff',
+ },
+ :args => ['-t', :mangle, '-p', :tcp, '-m', 'comment', '--comment', '058 set-mark 0x32/0xffffffff', '-j', 'MARK', '--set-xmark', '0x32/0xffffffff'],
+ },
+ 'mark_set-mark_hex_with_mask' => {
+ :params => {
+ :name => '058 set-mark 0x32/4',
+ :table => 'mangle',
+ :jump => 'MARK',
+ :chain => 'PREROUTING',
+ :set_mark => '0x32/4',
+ },
+ :args => ['-t', :mangle, '-p', :tcp, '-m', 'comment', '--comment', '058 set-mark 0x32/4', '-j', 'MARK', '--set-xmark', '0x32/0x4'],
+ },
+ 'iniface_1' => {
:params => {
:name => '060 iniface',
:table => 'filter',
View
2 spec/unit/facter/iptables_spec.rb
@@ -1,8 +1,8 @@
require 'spec_helper'
-require 'facter/iptables'
describe "Facter::Util::Fact" do
before {
+ Facter.clear
Facter.fact(:kernel).stubs(:value).returns("Linux")
Facter.fact(:kernelrelease).stubs(:value).returns("2.6")
}
View
42 spec/unit/puppet/type/firewall_spec.rb
@@ -34,7 +34,7 @@
res = @class.new(:name => "000 test")
res.parameters[:action].should == nil
end
-
+
[:accept, :drop, :reject].each do |action|
it "should accept value #{action}" do
@resource[:action] = action
@@ -277,10 +277,10 @@
describe ':action and :jump' do
it 'should allow only 1 to be set at a time' do
- expect {
+ expect {
@class.new(
- :name => "001-test",
- :action => "accept",
+ :name => "001-test",
+ :action => "accept",
:jump => "custom_chain"
)
}.should raise_error(Puppet::Error, /^Only one of the parameters 'action' and 'jump' can be set$/)
@@ -307,12 +307,38 @@
describe ':set_mark' do
it 'should allow me to set set-mark' do
- @resource[:set_mark] = '0x3e8'
- @resource[:set_mark].should == '0x3e8'
+ @resource[:set_mark] = '0x3e8/0xffffffff'
+ @resource[:set_mark].should == '0x3e8/0xffffffff'
end
- it 'should convert int to hex' do
+ it 'should convert int to hex and add a 32 bit mask' do
@resource[:set_mark] = '1000'
- @resource[:set_mark].should == '0x3e8'
+ @resource[:set_mark].should == '0x3e8/0xffffffff'
+ end
+ it 'should add a 32 bit mask' do
+ @resource[:set_mark] = '0x32'
+ @resource[:set_mark].should == '0x32/0xffffffff'
+ end
+ it 'should use the mask provided' do
+ @resource[:set_mark] = '0x32/0x4'
+ @resource[:set_mark].should == '0x32/0x4'
+ end
+ it 'should use the mask provided and convert int to hex' do
+ @resource[:set_mark] = '1000/0x4'
+ @resource[:set_mark].should == '0x3e8/0x4'
+ end
+ ['/', '1000/', 'pwnie'].each do |bad_mark|
+ it "should fail with malformed mark '#{bad_mark}'" do
+ lambda { @resource[:set_mark] = bad_mark}.should raise_error(Puppet::Error)
+ end
+ end
+ it 'should fail if mask is malformed' do
+ lambda { @resource[:set_mark] = '1000/0xq4'}.should raise_error(Puppet::Error)
+ end
+ it 'should fail if mark value is more than 32 bits' do
+ lambda { @resource[:set_mark] = '4294967296'}.should raise_error(Puppet::Error)
+ end
+ it 'should fail if mask value is more than 32 bits' do
+ lambda { @resource[:set_mark] = '1/4294967296'}.should raise_error(Puppet::Error)
end
end
View
11 spec/unit/puppet/util/firewall_spec.rb
@@ -44,4 +44,15 @@
specify { subject.string_to_port('80').should == '80' }
specify { subject.string_to_port('http').should == '80' }
end
+
+ describe '#to_hex32' do
+ subject { resource }
+ specify { subject.to_hex32('0').should == '0x0' }
+ specify { subject.to_hex32('0x32').should == '0x32' }
+ specify { subject.to_hex32('42').should == '0x2a' }
+ specify { subject.to_hex32('4294967295').should == '0xffffffff' }
+ specify { subject.to_hex32('4294967296').should == nil }
+ specify { subject.to_hex32('-1').should == nil }
+ specify { subject.to_hex32('bananas').should == nil }
+ end
end

0 comments on commit ff2e355

Please sign in to comment.