commit 79e9a23fb7386c5921c3547f4377664c340584d6
Author: Nacho Barrientos <nacho.barrientos@cern.ch>
Date:   Fri Nov 20 12:02:32 2020 +0100

    Move ICMP stuff to separate classes

diff --git a/files/config/puppet-inet-filter.nft b/files/config/puppet-inet-filter.nft
index 881850b..50681d2 100644
--- a/files/config/puppet-inet-filter.nft
+++ b/files/config/puppet-inet-filter.nft
@@ -2,8 +2,4 @@
 
   # something we want for all
   chain global {
-    ip protocol icmp icmp type { destination-unreachable, time-exceeded, parameter-problem } accept
-    ip6 nexthdr ipv6-icmp icmpv6 type { destination-unreachable, packet-too-big, time-exceeded, parameter-problem, mld-listener-query, mld-listener-report, mld-listener-done, nd-router-advert, nd-neighbor-solicit, nd-neighbor-advert, ind-neighbor-solicit, ind-neighbor-advert, mld2-listener-report } accept
-    ip protocol icmp icmp type echo-request limit rate 4/second accept
-    ip6 nexthdr ipv6-icmp icmpv6 type echo-request limit rate 4/second accept
   }
diff --git a/manifests/inet_filter.pp b/manifests/inet_filter.pp
index 0b2f274..c17d467 100644
--- a/manifests/inet_filter.pp
+++ b/manifests/inet_filter.pp
@@ -136,6 +136,9 @@ class nftables::inet_filter inherits nftables {
     if $nftables::out_https {
       include nftables::rules::out::https
     }
+    if $nftables::out_icmp {
+      include nftables::rules::out::icmp
+    }
   }
 
   # allow forwarding traffic on bridges
@@ -145,4 +148,7 @@ class nftables::inet_filter inherits nftables {
   if $nftables::in_ssh {
     include nftables::rules::ssh
   }
+  if $nftables::in_icmp {
+    include nftables::rules::icmp
+  }
 }
diff --git a/manifests/init.pp b/manifests/init.pp
index 6d89691..029c430 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -23,9 +23,15 @@
 # @param out_https
 #   Allow outbound to https servers.
 #
+# @param out_icmp
+#   Allow outbound ICMPv4/v6 traffic.
+#
 # @param in_ssh
 #   Allow inbound to ssh servers.
 #
+# @param in_icmp
+#   Allow inbound ICMPv4/v6 traffic.
+#
 # @param log_prefix
 #   String that will be used as prefix when logging packets. It can contain
 #   two variables using standard sprintf() string-formatting:
@@ -44,10 +50,12 @@
 #
 class nftables (
   Boolean $in_ssh                = true,
+  Boolean $in_icmp               = true,
   Boolean $out_ntp               = true,
   Boolean $out_dns               = true,
   Boolean $out_http              = true,
   Boolean $out_https             = true,
+  Boolean $out_icmp              = true,
   Boolean $out_all               = false,
   Boolean $in_out_conntrack      = true,
   Hash $rules                    = {},
diff --git a/manifests/rules/icmp.pp b/manifests/rules/icmp.pp
new file mode 100644
index 0000000..a699ff8
--- /dev/null
+++ b/manifests/rules/icmp.pp
@@ -0,0 +1,37 @@
+class nftables::rules::icmp (
+  Optional[Array[String]] $v4_types = undef,
+  Optional[Array[String]] $v6_types = undef,
+  String $order                     = '10',
+) {
+  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,
+      }
+    }
+  } else {
+    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,
+      }
+    }
+  } else {
+    nftables::rule{
+      'default_in-accept_icmpv6':
+        content => 'ip6 nexthdr ipv6-icmp accept',
+        order   => $order,
+      }
+  }
+}
diff --git a/manifests/rules/out/icmp.pp b/manifests/rules/out/icmp.pp
new file mode 100644
index 0000000..a67304a
--- /dev/null
+++ b/manifests/rules/out/icmp.pp
@@ -0,0 +1,37 @@
+class nftables::rules::out::icmp (
+  Optional[Array[String]] $v4_types = undef,
+  Optional[Array[String]] $v6_types = undef,
+  String $order                     = '10',
+) {
+  if $v4_types {
+    $v4_types.each | String $icmp_type | {
+      nftables::rule{
+        'default_out-accept_icmpv4':
+          content => "ip protocol icmp icmp type ${icmp_type} accept",
+          order   => $order,
+      }
+    }
+  } else {
+    nftables::rule{
+      'default_out-accept_icmpv4':
+        content => 'ip protocol icmp accept',
+        order   => $order,
+      }
+  }
+
+  if $v6_types {
+    $v6_types.each | String $icmp_type | {
+      nftables::rule{
+        'default_out-accept_icmpv6':
+          content => "ip6 nexthdr ipv6-icmp icmpv6 type ${icmp_type} accept",
+          order   => $order,
+      }
+    }
+  } else {
+    nftables::rule{
+      'default_out-accept_icmpv6':
+        content => 'ip6 nexthdr ipv6-icmp accept',
+        order   => $order,
+      }
+  }
+}
diff --git a/spec/classes/inet_filter_spec.rb b/spec/classes/inet_filter_spec.rb
index 098be46..187da7d 100644
--- a/spec/classes/inet_filter_spec.rb
+++ b/spec/classes/inet_filter_spec.rb
@@ -160,6 +160,9 @@ describe 'nftables' do
             order:   '50-nftables-inet-filter-chain-default_in-rule-ssh-b',
           )
         }
+        it {
+          is_expected.to contain_class('nftables::rules::icmp')
+        }
       end
 
       context 'chain output' do
@@ -308,6 +311,9 @@ describe 'nftables' do
             order:   '50-nftables-inet-filter-chain-default_out-rule-https-b',
           )
         }
+        it {
+          is_expected.to contain_class('nftables::rules::out::icmp')
+        }
       end
 
       context 'chain forward' do
@@ -553,6 +559,22 @@ describe 'nftables' do
           is_expected.not_to contain_concat__fragment('nftables-inet-filter-chain-FORWARD-rule-drop_invalid')
         }
       end
+
+      context 'without ICMP configuration' do
+        let(:params) do
+          {
+            'in_icmp' => false,
+            'out_icmp' => false,
+          }
+        end
+
+        it {
+          is_expected.not_to contain_class('nftables::rules::icmp')
+        }
+        it {
+          is_expected.not_to contain_class('nftables::rules::out::icmp')
+        }
+      end
     end
   end
 end
diff --git a/spec/classes/rules/icmp_spec.rb b/spec/classes/rules/icmp_spec.rb
new file mode 100644
index 0000000..bb132c4
--- /dev/null
+++ b/spec/classes/rules/icmp_spec.rb
@@ -0,0 +1,88 @@
+require 'spec_helper'
+
+describe 'nftables::rules::icmp' do
+  on_supported_os.each do |os, os_facts|
+    context "on #{os}" do
+      let(:facts) { os_facts }
+
+      context 'default options' do
+        it { is_expected.to compile }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv4').with(
+            content: 'ip protocol icmp accept',
+            order: '10',
+          )
+        }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv6').with(
+            content: 'ip6 nexthdr ipv6-icmp accept',
+            order: '10',
+          )
+        }
+      end
+
+      context 'with custom ICMP types (v4 only)' do
+        let(:params) do
+          {
+            v4_types: ['echo-request limit rate 4/second', 'echo-reply'],
+          }
+        end
+
+        it { is_expected.to compile }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv4_echo_request').with(
+            content: 'ip protocol icmp icmp type echo-request limit rate 4/second accept',
+            order: '10',
+          )
+        }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv4_echo_reply').with(
+            content: 'ip protocol icmp icmp type echo-reply accept',
+            order: '10',
+          )
+        }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv6').with(
+            content: 'ip6 nexthdr ipv6-icmp accept',
+            order: '10',
+          )
+        }
+      end
+
+      context 'with custom ICMP types (both v4 and v6)' do
+        let(:params) do
+          {
+            v4_types: ['echo-request limit rate 4/second', 'echo-reply'],
+            v6_types: ['echo-reply', 'nd-router-advert'],
+          }
+        end
+
+        it { is_expected.to compile }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv4_echo_request').with(
+            content: 'ip protocol icmp icmp type echo-request limit rate 4/second accept',
+            order: '10',
+          )
+        }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv4_echo_reply').with(
+            content: 'ip protocol icmp icmp type echo-reply accept',
+            order: '10',
+          )
+        }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv6_echo_reply').with(
+            content: 'ip6 nexthdr ipv6-icmp icmpv6 type echo-reply accept',
+            order: '10',
+          )
+        }
+        it {
+          is_expected.to contain_nftables__rule('default_in-accept_icmpv6_nd_router_advert').with(
+            content: 'ip6 nexthdr ipv6-icmp icmpv6 type nd-router-advert accept',
+            order: '10',
+          )
+        }
+      end
+    end
+  end
+end
