Skip to content

Commit a2b5e71

Browse files
foxxx0foxxx0
authored
Merge pull request #114 from foxxx0/fix-portrange-regression
implement proper sport/dport types, validate port ranges, fix some minor regressions
2 parents 840e99f + 1fc9834 commit a2b5e71

File tree

8 files changed

+348
-9
lines changed

8 files changed

+348
-9
lines changed

‎manifests/rule.pp‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@
5959
String $comment = $name,
6060
Optional[Ferm::Actions] $action = undef,
6161
Optional[Ferm::Policies] $policy = undef,
62-
Optional[Variant[Stdlib::Port,Array[Stdlib::Port]]] $dport = undef,
63-
Optional[Variant[Stdlib::Port,Array[Stdlib::Port]]] $sport = undef,
62+
Optional[Ferm::Port] $dport = undef,
63+
Optional[Ferm::Port] $sport = undef,
6464
Optional[Variant[Array, String[1]]] $saddr = undef,
6565
Optional[Variant[Array, String[1]]] $daddr = undef,
6666
Optional[String[1]] $proto_options = undef,
@@ -95,26 +95,60 @@
9595
String => "proto ${proto}",
9696
}
9797

98-
# ferm supports implicit multiport using the "dports" shortcut
98+
9999
if $dport =~ Array {
100100
$dports = join($dport, ' ')
101-
$dport_real = "dports (${dports})"
101+
$dport_real = "mod multiport destination-ports (${dports})"
102102
} elsif $dport =~ Integer {
103103
$dport_real = "dport ${dport}"
104-
} else {
104+
} elsif String($dport) =~ /^\d*:\d+$/ {
105+
$portrange = split($dport, /:/)
106+
$lower = $portrange[0] ? {
107+
'' => 0,
108+
default => Integer($portrange[0]),
109+
}
110+
$upper = Integer($portrange[1])
111+
assert_type(Tuple[Stdlib::Port, Stdlib::Port], [$lower, $upper]) |$expected, $actual| {
112+
fail("The data type should be \'${expected}\', not \'${actual}\'. The data is [${lower}, ${upper}])}.")
113+
''
114+
}
115+
if $lower > $upper {
116+
fail("Lower port number of the port range is larger than upper. ${lower}:${upper}")
117+
}
118+
$dport_real = "dport ${lower}:${upper}"
119+
} elsif String($dport) == '' {
105120
$dport_real = ''
121+
} else {
122+
fail("invalid destination-port: ${dport}")
106123
}
107124

108-
# ferm supports implicit multiport using the "sports" shortcut
109125
if $sport =~ Array {
110126
$sports = join($sport, ' ')
111-
$sport_real = "sports (${sports})"
127+
$sport_real = "mod multiport source-ports (${sports})"
112128
} elsif $sport =~ Integer {
113129
$sport_real = "sport ${sport}"
114-
} else {
130+
} elsif String($sport) =~ /^\d*:\d+$/ {
131+
$portrange = split($sport, /:/)
132+
$lower = $portrange[0] ? {
133+
'' => 0,
134+
default => Integer($portrange[0]),
135+
}
136+
$upper = Integer($portrange[1])
137+
assert_type(Tuple[Stdlib::Port, Stdlib::Port], [$lower, $upper]) |$expected, $actual| {
138+
fail("The data type should be \'${expected}\', not \'${actual}\'. The data is [${lower}, ${upper}])}.")
139+
''
140+
}
141+
if $lower > $upper {
142+
fail("Lower port number of the port range is larger than upper. ${lower}:${upper}")
143+
}
144+
$sport_real = "sport ${lower}:${upper}"
145+
} elsif String($sport) == '' {
115146
$sport_real = ''
147+
} else {
148+
fail("invalid source-port: ${sport}")
116149
}
117150

151+
118152
if $saddr =~ Array {
119153
assert_type(Array[Stdlib::IP::Address], flatten($saddr)) |$expected, $actual| {
120154
fail( "The data type should be \'${expected}\', not \'${actual}\'. The data is ${flatten($saddr)}." )

‎spec/defines/rule_spec.rb‎

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,91 @@
127127
end
128128

129129
it { is_expected.to compile.with_all_deps }
130-
it { is_expected.to contain_concat__fragment('INPUT-filter-consul').with_content("mod comment comment 'filter-consul' proto (tcp udp) dports (8301 8302) saddr @ipfilter((127.0.0.1)) ACCEPT;\n") }
130+
it { is_expected.to contain_concat__fragment('INPUT-filter-consul').with_content("mod comment comment 'filter-consul' proto (tcp udp) mod multiport destination-ports (8301 8302) saddr @ipfilter((127.0.0.1)) ACCEPT;\n") }
131131
it { is_expected.to contain_concat__fragment('filter-INPUT-config-include') }
132132
it { is_expected.to contain_concat__fragment('filter-FORWARD-config-include') }
133133
it { is_expected.to contain_concat__fragment('filter-OUTPUT-config-include') }
134134
end
135135

136+
context 'with a valid destination-port range' do
137+
let(:title) { 'filter-portrange' }
138+
let :params do
139+
{
140+
chain: 'INPUT',
141+
action: 'ACCEPT',
142+
proto: 'tcp',
143+
dport: '20000:25000',
144+
saddr: '127.0.0.1'
145+
}
146+
end
147+
148+
it { is_expected.to compile.with_all_deps }
149+
it { is_expected.to contain_concat__fragment('INPUT-filter-portrange').with_content("mod comment comment 'filter-portrange' proto tcp dport 20000:25000 saddr @ipfilter((127.0.0.1)) ACCEPT;\n") }
150+
it { is_expected.to contain_concat__fragment('filter-INPUT-config-include') }
151+
it { is_expected.to contain_concat__fragment('filter-FORWARD-config-include') }
152+
it { is_expected.to contain_concat__fragment('filter-OUTPUT-config-include') }
153+
end
154+
155+
context 'with a malformed source-port range' do
156+
let(:title) { 'filter-malformed-portrange' }
157+
let :params do
158+
{
159+
chain: 'INPUT',
160+
action: 'ACCEPT',
161+
proto: 'tcp',
162+
sport: '25000:20000',
163+
saddr: '127.0.0.1'
164+
}
165+
end
166+
167+
it { is_expected.to compile.and_raise_error(%r{Lower port number of the port range is larger than upper. 25000:20000}) }
168+
end
169+
170+
context 'with an invalid destination-port range' do
171+
let(:title) { 'filter-invalid-portrange' }
172+
let :params do
173+
{
174+
chain: 'INPUT',
175+
action: 'ACCEPT',
176+
proto: 'tcp',
177+
dport: '50000:65538',
178+
saddr: '127.0.0.1'
179+
}
180+
end
181+
182+
it { is_expected.to compile.and_raise_error(%r{The data type should be 'Tuple\[Stdlib::Port, Stdlib::Port\]', not 'Tuple\[Integer\[50000, 50000\], Integer\[65538, 65538\]\]'. The data is \[50000, 65538\]}) }
183+
end
184+
185+
context 'with an invalid destination-port string' do
186+
let(:title) { 'filter-invalid-portnumber' }
187+
let :params do
188+
{
189+
chain: 'INPUT',
190+
action: 'ACCEPT',
191+
proto: 'tcp',
192+
dport: '65538',
193+
saddr: '127.0.0.1'
194+
}
195+
end
196+
197+
it { is_expected.to compile.and_raise_error(%r{parameter 'dport' expects a Ferm::Port .* value, got String}) }
198+
end
199+
200+
context 'with an invalid source-port number' do
201+
let(:title) { 'filter-invalid-portnumber' }
202+
let :params do
203+
{
204+
chain: 'INPUT',
205+
action: 'ACCEPT',
206+
proto: 'tcp',
207+
sport: 65_538,
208+
saddr: '127.0.0.1'
209+
}
210+
end
211+
212+
it { is_expected.to compile.and_raise_error(%r{parameter 'sport' expects a Ferm::Port .* value, got Integer}) }
213+
end
214+
136215
context 'with jumping to custom chains' do
137216
# create custom chain
138217
let(:pre_condition) do

‎spec/type_aliases/actions_spec.rb‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# rubocop:disable Style/WordArray, Style/TrailingCommaInLiteral
2+
require 'spec_helper'
3+
4+
describe 'Ferm::Actions' do
5+
describe 'valid values' do
6+
[
7+
'RETURN',
8+
'ACCEPT',
9+
'DROP',
10+
'REJECT',
11+
'NOTRACK',
12+
'LOG',
13+
'MARK',
14+
'DNAT',
15+
'SNAT',
16+
'MASQUERADE',
17+
'REDIRECT',
18+
'MYFANCYCUSTOMCHAINNAMEISALSOVALID',
19+
].each do |value|
20+
describe value.inspect do
21+
it { is_expected.to allow_value(value) }
22+
end
23+
end
24+
end
25+
26+
describe 'invalid values' do
27+
context 'with garbage inputs' do
28+
[
29+
# :symbol, # this should not match but seems liks String[1] allows it?
30+
# nil, # this should not match but seems liks String[1] allows it?
31+
'',
32+
true,
33+
false,
34+
['meep', 'meep'],
35+
65_538,
36+
[95_000, 67_000],
37+
{},
38+
{ 'foo' => 'bar' },
39+
].each do |value|
40+
describe value.inspect do
41+
it { is_expected.not_to allow_value(value) }
42+
end
43+
end
44+
end
45+
end
46+
end

‎spec/type_aliases/policies_spec.rb‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# rubocop:disable Style/WordArray, Style/TrailingCommaInLiteral
2+
require 'spec_helper'
3+
4+
describe 'Ferm::Policies' do
5+
describe 'valid values' do
6+
[
7+
'ACCEPT',
8+
'DROP',
9+
].each do |value|
10+
describe value.inspect do
11+
it { is_expected.to allow_value(value) }
12+
end
13+
end
14+
end
15+
16+
describe 'invalid values' do
17+
context 'with garbage inputs' do
18+
[
19+
'RETURN',
20+
'REJECT',
21+
'foobar',
22+
:symbol,
23+
nil,
24+
'',
25+
true,
26+
false,
27+
['meep', 'meep'],
28+
65_538,
29+
[95_000, 67_000],
30+
{},
31+
{ 'foo' => 'bar' },
32+
].each do |value|
33+
describe value.inspect do
34+
it { is_expected.not_to allow_value(value) }
35+
end
36+
end
37+
end
38+
end
39+
end

‎spec/type_aliases/port_spec.rb‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# rubocop:disable Style/WordArray, Style/TrailingCommaInLiteral
2+
require 'spec_helper'
3+
4+
describe 'Ferm::Port' do
5+
describe 'valid values' do
6+
[
7+
17,
8+
65_535,
9+
'25:30',
10+
':22',
11+
[80, 443, 8080, 8443],
12+
].each do |value|
13+
describe value.inspect do
14+
it { is_expected.to allow_value(value) }
15+
end
16+
end
17+
end
18+
19+
describe 'invalid values' do
20+
context 'with garbage inputs' do
21+
[
22+
'asdf',
23+
true,
24+
false,
25+
:symbol,
26+
['meep', 'meep'],
27+
65_538,
28+
[95_000, 67_000],
29+
'12345',
30+
'20:22:23',
31+
'1024:',
32+
'ネット',
33+
nil,
34+
{},
35+
{ 'foo' => 'bar' },
36+
].each do |value|
37+
describe value.inspect do
38+
it { is_expected.not_to allow_value(value) }
39+
end
40+
end
41+
end
42+
end
43+
end

‎spec/type_aliases/protocols_spec.rb‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# rubocop:disable Style/WordArray, Style/TrailingCommaInLiteral
2+
require 'spec_helper'
3+
4+
describe 'Ferm::Protocols' do
5+
describe 'valid values' do
6+
[
7+
'icmp',
8+
'tcp',
9+
'udp',
10+
'udplite',
11+
'icmpv6',
12+
'esp',
13+
'ah',
14+
'sctp',
15+
'mh',
16+
'all',
17+
['icmp', 'tcp', 'udp'],
18+
].each do |value|
19+
describe value.inspect do
20+
it { is_expected.to allow_value(value) }
21+
end
22+
end
23+
end
24+
25+
describe 'invalid values' do
26+
context 'with garbage inputs' do
27+
[
28+
:symbol,
29+
nil,
30+
'foobar',
31+
'',
32+
true,
33+
false,
34+
['meep', 'meep'],
35+
65_538,
36+
[95_000, 67_000],
37+
{},
38+
{ 'foo' => 'bar' },
39+
].each do |value|
40+
describe value.inspect do
41+
it { is_expected.not_to allow_value(value) }
42+
end
43+
end
44+
end
45+
end
46+
end

‎spec/type_aliases/tables_spec.rb‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# rubocop:disable Style/WordArray, Style/TrailingCommaInLiteral
2+
require 'spec_helper'
3+
4+
describe 'Ferm::Tables' do
5+
describe 'valid values' do
6+
[
7+
'raw',
8+
'mangle',
9+
'nat',
10+
'filter',
11+
].each do |value|
12+
describe value.inspect do
13+
it { is_expected.to allow_value(value) }
14+
end
15+
end
16+
end
17+
18+
describe 'invalid values' do
19+
context 'with garbage inputs' do
20+
[
21+
:symbol,
22+
nil,
23+
'foobar',
24+
'',
25+
true,
26+
false,
27+
['meep', 'meep'],
28+
65_538,
29+
[95_000, 67_000],
30+
{},
31+
{ 'foo' => 'bar' },
32+
].each do |value|
33+
describe value.inspect do
34+
it { is_expected.not_to allow_value(value) }
35+
end
36+
end
37+
end
38+
end
39+
end

0 commit comments

Comments
 (0)