18 changes: 18 additions & 0 deletions lib/puppet/provider/firewall/iptables.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@ def self.rule_to_hash(line, table, counter)
hash[prop] = hash[prop].split(',') if ! hash[prop].nil?
end

# Our type prefers hyphens over colons for ranges so ...
# Iterate across all ports replacing colons with hyphens so that ranges match
# the types expectations.
[:dport, :sport].each do |prop|
next unless hash[prop]
hash[prop] = hash[prop].collect do |elem|
elem.gsub(/:/,'-')
end
end

# This forces all existing, commentless rules to be moved to the bottom of the stack.
# Puppet-firewall requires that all rules have comments (resource names) and will fail if
# a rule in iptables does not have a comment. We get around this by appending a high level
Expand Down Expand Up @@ -207,6 +217,14 @@ def general_args

args << resource_map[res].split(' ')

# For sport and dport, convert hyphens to colons since the type
# expects hyphens for ranges of ports.
if [:sport, :dport].include?(res) then
resource_value = resource_value.collect do |elem|
elem.gsub(/-/, ':')
end
end

if resource_value.is_a?(Array)
args << resource_value.join(',')
else
Expand Down
265 changes: 206 additions & 59 deletions lib/puppet/type/firewall.rb

Large diffs are not rendered by default.

16 changes: 11 additions & 5 deletions lib/puppet/util/firewall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,19 @@ def icmp_name_to_number(value_icmp)
end
end

# This method takes a string and attempts to convert it to a port number
# if valid.
#
# If the string already contains a port number or perhaps a range of ports
# in the format 22:1000 for example, it simply returns the string and does
# nothing.
def string_to_port(value)
if value.kind_of?(Array)
ports = []
value.each do |port|
ports << Socket.getservbyname(port) unless port.kind_of?(Integer)
if value.kind_of?(String)
if value.match(/^\d+(-\d+)?$/)
return value
else
return Socket.getservbyname(value).to_s
end
ports
else
Socket.getservbyname(value)
end
Expand Down
62 changes: 61 additions & 1 deletion spec/fixtures/iptables/conversion_hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,34 @@
:action => nil,
},
},
'dport_range_1' => {
:line => '-A INPUT -m multiport --dports 1:1024 -m comment --comment "000 allow foo"',
:table => 'filter',
:params => {
:dport => ["1-1024"],
},
},
'dport_range_2' => {
:line => '-A INPUT -m multiport --dports 15,512:1024 -m comment --comment "000 allow foo"',
:table => 'filter',
:params => {
:dport => ["15","512-1024"],
},
},
'sport_range_1' => {
:line => '-A INPUT -m multiport --sports 1:1024 -m comment --comment "000 allow foo"',
:table => 'filter',
:params => {
:sport => ["1-1024"],
},
},
'sport_range_2' => {
:line => '-A INPUT -m multiport --sports 15,512:1024 -m comment --comment "000 allow foo"',
:table => 'filter',
:params => {
:sport => ["15","512-1024"],
},
},
}

# This hash is for testing converting a hash to an argument line.
Expand Down Expand Up @@ -98,5 +126,37 @@
},
:args => ["-t", :filter, "-p", :tcp, "-m", "comment", "--comment",
"100 no action"],
}
},
'sport_range_1' => {
:params => {
:name => "100 sport range",
:sport => ["1-1024"],
:table => "filter",
},
:args => ["-t", :filter, "-p", :tcp, "-m", "multiport", "--sports", "1:1024", "-m", "comment", "--comment", "100 sport range"],
},
'sport_range_2' => {
:params => {
:name => "100 sport range",
:sport => ["15","512-1024"],
:table => "filter",
},
:args => ["-t", :filter, "-p", :tcp, "-m", "multiport", "--sports", "15,512:1024", "-m", "comment", "--comment", "100 sport range"],
},
'dport_range_1' => {
:params => {
:name => "100 sport range",
:dport => ["1-1024"],
:table => "filter",
},
:args => ["-t", :filter, "-p", :tcp, "-m", "multiport", "--dports", "1:1024", "-m", "comment", "--comment", "100 sport range"],
},
'dport_range_2' => {
:params => {
:name => "100 sport range",
:dport => ["15","512-1024"],
:table => "filter",
},
:args => ["-t", :filter, "-p", :tcp, "-m", "multiport", "--dports", "15,512:1024", "-m", "comment", "--comment", "100 sport range"],
},
}
8 changes: 7 additions & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,13 @@ module PuppetSpec
Puppet.settings[:bindaddress] = "127.0.0.1"

@logs = []
Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(@logs))
# This tests allows the spec_helper to be >2.6.7 and >2.7.1 compatible
# as the Puppet::Test::LogCollector facility wasn't available until 2.7.x
if Puppet.const_defined?("Test") and Puppet::Test.const_defined?("LogCollector")
Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(@logs))
else
Puppet::Util::Log.newdestination(@logs)
end

@log_level = Puppet::Util::Log.level
end
Expand Down
29 changes: 27 additions & 2 deletions spec/unit/puppet/type/firewall_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,37 @@
describe port do
it "should accept a #{port} as string" do
@resource[port] = '22'
@resource[port].should == [22]
@resource[port].should == ['22']
end

it "should accept a #{port} as an array" do
@resource[port] = ['22','23']
@resource[port].should == [22,23]
@resource[port].should == ['22','23']
end

it "should accept a #{port} as a hyphen separated range" do
@resource[port] = ['22-1000']
@resource[port].should == ['22-1000']
end

it "should accept a #{port} as a combination of arrays of single and " \
"hyphen separated ranges" do

@resource[port] = ['22-1000','33','3000-4000']
@resource[port].should == ['22-1000','33','3000-4000']
end

it "should convert a port name for #{port} to its number" do
@resource[port] = 'ssh'
@resource[port].should == ['22']
end

it "should not accept something invalid for #{port}" do
expect { @resource[port] = 'something odd' }.should raise_error(Puppet::Error, /^Parameter .+ failed: Munging failed for value ".+" in class .+: no such service/)
end

it "should not accept something invalid in an array for #{port}" do
expect { @resource[port] = ['something odd','something even odder'] }.should raise_error(Puppet::Error, /^Parameter .+ failed: Munging failed for value ".+" in class .+: no such service/)
end
end
end
Expand Down