Merge pull request #231 from voxpupuli/icmp

rules::icmp: Allow ICMP packets with extensions
voxpupuli · Dec 29, 2023 · 483d0a9 · 483d0a9
2 parents add6257 + 8cdd24a
commit 483d0a9
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 40 deletions.
diff --git a/REFERENCE.md b/REFERENCE.md
@@ -24,7 +24,7 @@ Enable this option to support Ceph's Monitor Daemon.
 * [`nftables::rules::http`](#nftables--rules--http): manage in http
 * [`nftables::rules::https`](#nftables--rules--https): manage in https
 * [`nftables::rules::icinga2`](#nftables--rules--icinga2): manage in icinga2
-* [`nftables::rules::icmp`](#nftables--rules--icmp)
+* [`nftables::rules::icmp`](#nftables--rules--icmp): allows incoming ICMP
 * [`nftables::rules::igmp`](#nftables--rules--igmp): allow incoming IGMP messages
 * [`nftables::rules::ldap`](#nftables--rules--ldap): manage in ldap
 * [`nftables::rules::llmnr`](#nftables--rules--llmnr): allow incoming Link-Local Multicast Name Resolution
@@ -683,7 +683,7 @@ Default value: `[5665]`
 
 ### <a name="nftables--rules--icmp"></a>`nftables::rules::icmp`
 
-The nftables::rules::icmp class.
+allows incoming ICMP
 
 #### Parameters
 
@@ -697,23 +697,23 @@ The following parameters are available in the `nftables::rules::icmp` class:
 
 Data type: `Optional[Array[String]]`
 
-
+ICMP v4 types that should be allowed
 
 Default value: `undef`
 
 ##### <a name="-nftables--rules--icmp--v6_types"></a>`v6_types`
 
 Data type: `Optional[Array[String]]`
 
-
+ICMP v6 types that should be allowed
 
 Default value: `undef`
 
 ##### <a name="-nftables--rules--icmp--order"></a>`order`
 
 Data type: `String`
 
-
+the ordering of the rules
 
 Default value: `'10'`
 

diff --git a/manifests/rules/icmp.pp b/manifests/rules/icmp.pp
@@ -1,39 +1,46 @@
+#
+# @summary allows incoming ICMP
+#
+# @param v4_types ICMP v4 types that should be allowed
+# @param v6_types ICMP v6 types that should be allowed
+# @param order the ordering of the rules
+#
 class nftables::rules::icmp (
-  # lint:ignore:parameter_documentation
   Optional[Array[String]] $v4_types = undef,
   Optional[Array[String]] $v6_types = undef,
   String $order                     = '10',
-  # lint:endignore
 ) {
   if $v4_types {
     $v4_types.each | String $icmp_type | {
-      nftables::rule {
-        "default_in-accept_icmpv4_${regsubst(split($icmp_type, ' ')[0], '-', '_', 'G')}":
-          content => "ip protocol icmp icmp type ${icmp_type} accept",
-          order   => $order,
+      nftables::rule { "default_in-accept_icmpv4_${regsubst(split($icmp_type, ' ')[0], '-', '_', 'G')}":
+        content => "ip protocol icmp icmp type ${icmp_type} accept",
+        order   => $order,
       }
     }
-  } else {
-    nftables::rule {
-      'default_in-accept_icmpv4':
-        content => 'ip protocol icmp accept',
-        order   => $order,
+  } elsif $v6_types {
+    nftables::rule { 'default_in-accept_icmpv4':
+      content => 'ip protocol icmp accept',
+      order   => $order,
     }
   }
 
   if $v6_types {
     $v6_types.each | String $icmp_type | {
-      nftables::rule {
-        "default_in-accept_icmpv6_${regsubst(split($icmp_type, ' ')[0], '-', '_', 'G')}":
-          content => "ip6 nexthdr ipv6-icmp icmpv6 type ${icmp_type} accept",
-          order   => $order,
+      nftables::rule { "default_in-accept_icmpv6_${regsubst(split($icmp_type, ' ')[0], '-', '_', 'G')}":
+        content => "ip6 nexthdr ipv6-icmp icmpv6 type ${icmp_type} accept",
+        order   => $order,
       }
     }
-  } else {
-    nftables::rule {
-      'default_in-accept_icmpv6':
-        content => 'ip6 nexthdr ipv6-icmp accept',
-        order   => $order,
+  } elsif $v4_types {
+    nftables::rule { 'default_in-accept_icmpv6':
+      content => 'meta l4proto icmpv6 accept',
+      order   => $order,
+    }
+  }
+  if $v6_types == undef and $v4_types == undef {
+    nftables::rule { 'default_in-accept_icmp':
+      content => 'meta l4proto { icmp, icmpv6} accept',
+      order   => $order,
     }
   }
 }
diff --git a/spec/classes/rules/icmp_spec.rb b/spec/classes/rules/icmp_spec.rb
@@ -8,21 +8,11 @@
       let(:facts) { os_facts }
 
       context 'default options' do
-        it { is_expected.to compile }
-
-        it {
-          expect(subject).to contain_nftables__rule('default_in-accept_icmpv4').with(
-            content: 'ip protocol icmp accept',
-            order: '10'
-          )
-        }
+        it { is_expected.to compile.with_all_deps }
 
-        it {
-          expect(subject).to contain_nftables__rule('default_in-accept_icmpv6').with(
-            content: 'ip6 nexthdr ipv6-icmp accept',
-            order: '10'
-          )
-        }
+        it { is_expected.to contain_nftables__rule('default_in-accept_icmp').with_content('meta l4proto { icmp, icmpv6} accept').with_order('10') }
+        it { is_expected.not_to contain_nftables__rule('default_in-accept_icmpv4') }
+        it { is_expected.not_to contain_nftables__rule('default_in-accept_icmpv6') }
       end
 
       context 'with custom ICMP types (v4 only)' do
@@ -50,7 +40,7 @@
 
         it {
           expect(subject).to contain_nftables__rule('default_in-accept_icmpv6').with(
-            content: 'ip6 nexthdr ipv6-icmp accept',
+            content: 'meta l4proto icmpv6 accept',
             order: '10'
           )
         }