From 856eca997158141e084b9e8c2002d7491a4720a1 Mon Sep 17 00:00:00 2001 From: Thore Bödecker Date: Thu, 25 Jun 2020 17:07:07 +0200 Subject: use proper types and validations for port handling - implement validations for port ranges - add test cases for these scenarios --- manifests/rule.pp | 46 ++++++++++++++++++++---- spec/defines/rule_spec.rb | 79 ++++++++++++++++++++++++++++++++++++++++++ spec/type_aliases/port_spec.rb | 43 +++++++++++++++++++++++ types/port.pp | 13 +++++++ 4 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 spec/type_aliases/port_spec.rb create mode 100644 types/port.pp diff --git a/manifests/rule.pp b/manifests/rule.pp index e44d04a..f239402 100644 --- a/manifests/rule.pp +++ b/manifests/rule.pp @@ -59,8 +59,8 @@ define ferm::rule ( String $comment = $name, Optional[Ferm::Actions] $action = undef, Optional[Ferm::Policies] $policy = undef, - Optional[Variant[Stdlib::Port,Array[Stdlib::Port]]] $dport = undef, - Optional[Variant[Stdlib::Port,Array[Stdlib::Port]]] $sport = undef, + Optional[Ferm::Port] $dport = undef, + Optional[Ferm::Port] $sport = undef, Optional[Variant[Array, String[1]]] $saddr = undef, Optional[Variant[Array, String[1]]] $daddr = undef, Optional[String[1]] $proto_options = undef, @@ -95,26 +95,60 @@ define ferm::rule ( String => "proto ${proto}", } - # ferm supports implicit multiport using the "dports" shortcut + if $dport =~ Array { $dports = join($dport, ' ') $dport_real = "mod multiport destination-ports (${dports})" } elsif $dport =~ Integer { $dport_real = "dport ${dport}" - } else { + } elsif String($dport) =~ /^\d*:\d+$/ { + $portrange = split($dport, /:/) + $lower = $portrange[0] ? { + '' => 0, + default => Integer($portrange[0]), + } + $upper = Integer($portrange[1]) + assert_type(Tuple[Stdlib::Port, Stdlib::Port], [$lower, $upper]) |$expected, $actual| { + fail("The data type should be \'${expected}\', not \'${actual}\'. The data is [${lower}, ${upper}])}.") + '' + } + if $lower > $upper { + fail("Lower port number of the port range is larger than upper. ${lower}:${upper}") + } + $dport_real = "dport ${lower}:${upper}" + } elsif String($dport) == '' { $dport_real = '' + } else { + fail("invalid destination-port: ${dport}") } - # ferm supports implicit multiport using the "sports" shortcut if $sport =~ Array { $sports = join($sport, ' ') $sport_real = "mod multiport source-ports (${sports})" } elsif $sport =~ Integer { $sport_real = "sport ${sport}" - } else { + } elsif String($sport) =~ /^\d*:\d+$/ { + $portrange = split($sport, /:/) + $lower = $portrange[0] ? { + '' => 0, + default => Integer($portrange[0]), + } + $upper = Integer($portrange[1]) + assert_type(Tuple[Stdlib::Port, Stdlib::Port], [$lower, $upper]) |$expected, $actual| { + fail("The data type should be \'${expected}\', not \'${actual}\'. The data is [${lower}, ${upper}])}.") + '' + } + if $lower > $upper { + fail("Lower port number of the port range is larger than upper. ${lower}:${upper}") + } + $sport_real = "sport ${lower}:${upper}" + } elsif String($sport) == '' { $sport_real = '' + } else { + fail("invalid source-port: ${sport}") } + if $saddr =~ Array { assert_type(Array[Stdlib::IP::Address], flatten($saddr)) |$expected, $actual| { fail( "The data type should be \'${expected}\', not \'${actual}\'. The data is ${flatten($saddr)}." ) diff --git a/spec/defines/rule_spec.rb b/spec/defines/rule_spec.rb index b2a2abd..f2601c6 100644 --- a/spec/defines/rule_spec.rb +++ b/spec/defines/rule_spec.rb @@ -133,6 +133,85 @@ describe 'ferm::rule', type: :define do it { is_expected.to contain_concat__fragment('filter-OUTPUT-config-include') } end + context 'with a valid destination-port range' do + let(:title) { 'filter-portrange' } + let :params do + { + chain: 'INPUT', + action: 'ACCEPT', + proto: 'tcp', + dport: '20000:25000', + saddr: '127.0.0.1' + } + end + + it { is_expected.to compile.with_all_deps } + 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") } + it { is_expected.to contain_concat__fragment('filter-INPUT-config-include') } + it { is_expected.to contain_concat__fragment('filter-FORWARD-config-include') } + it { is_expected.to contain_concat__fragment('filter-OUTPUT-config-include') } + end + + context 'with a malformed source-port range' do + let(:title) { 'filter-malformed-portrange' } + let :params do + { + chain: 'INPUT', + action: 'ACCEPT', + proto: 'tcp', + sport: '25000:20000', + saddr: '127.0.0.1' + } + end + + it { is_expected.to compile.and_raise_error(%r{Lower port number of the port range is larger than upper. 25000:20000}) } + end + + context 'with an invalid destination-port range' do + let(:title) { 'filter-invalid-portrange' } + let :params do + { + chain: 'INPUT', + action: 'ACCEPT', + proto: 'tcp', + dport: '50000:65538', + saddr: '127.0.0.1' + } + end + + 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\]}) } + end + + context 'with an invalid destination-port string' do + let(:title) { 'filter-invalid-portnumber' } + let :params do + { + chain: 'INPUT', + action: 'ACCEPT', + proto: 'tcp', + dport: '65538', + saddr: '127.0.0.1' + } + end + + it { is_expected.to compile.and_raise_error(%r{parameter 'dport' expects a Ferm::Port .* value, got String}) } + end + + context 'with an invalid source-port number' do + let(:title) { 'filter-invalid-portnumber' } + let :params do + { + chain: 'INPUT', + action: 'ACCEPT', + proto: 'tcp', + sport: 65_538, + saddr: '127.0.0.1' + } + end + + it { is_expected.to compile.and_raise_error(%r{parameter 'sport' expects a Ferm::Port .* value, got Integer}) } + end + context 'with jumping to custom chains' do # create custom chain let(:pre_condition) do diff --git a/spec/type_aliases/port_spec.rb b/spec/type_aliases/port_spec.rb new file mode 100644 index 0000000..e2b0d43 --- /dev/null +++ b/spec/type_aliases/port_spec.rb @@ -0,0 +1,43 @@ +# rubocop:disable Style/WordArray, Style/TrailingCommaInLiteral +require 'spec_helper' + +describe 'Ferm::Port' do + describe 'valid values' do + [ + 17, + 65_535, + '25:30', + ':22', + [80, 443, 8080, 8443], + ].each do |value| + describe value.inspect do + it { is_expected.to allow_value(value) } + end + end + end + + describe 'invalid values' do + context 'with garbage inputs' do + [ + 'asdf', + true, + false, + :symbol, + ['meep', 'meep'], + 65_538, + [95_000, 67_000], + '12345', + '20:22:23', + '1024:', + 'ネット', + nil, + {}, + { 'foo' => 'bar' }, + ].each do |value| + describe value.inspect do + it { is_expected.not_to allow_value(value) } + end + end + end + end +end diff --git a/types/port.pp b/types/port.pp new file mode 100644 index 0000000..dc2b7e1 --- /dev/null +++ b/types/port.pp @@ -0,0 +1,13 @@ +# @summary ferm port-spec +# +# allowed variants: +# ----------------- +# + single Integer port +# + Array of Integers (creates a multiport matcher) +# + ferm range port-spec (pair of colon-separated integer, assumes 0 if first is omitted) + +type Ferm::Port = Variant[ + Stdlib::Port, + Array[Stdlib::Port], + Pattern['^\d*:\d+$'], +] -- cgit v1.2.3