From 0dc967788dcb7c05625bce3850be5f453924caac Mon Sep 17 00:00:00 2001
From: Tim Meusel <tim@bastelfreak.de>
Date: Wed, 20 Dec 2023 20:02:17 +0100
Subject: [PATCH] Make "dropping invalid packets" configureable

It doesn't make sense to explicitly drop those pakets when the default
policy is already `DROP`. Also some applications, like ceph, are known
to send packets that might be marked as invalid.
---
 REFERENCE.md                                  | 18 ++++++++++++++++
 manifests/inet_filter/fwd_conntrack.pp        |  5 ++++-
 manifests/inet_filter/in_out_conntrack.pp     |  5 ++++-
 manifests/init.pp                             |  8 +++++++
 .../inet_filter/in_out_conntrack_spec.rb      | 21 ++++++++++++++++---
 5 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/REFERENCE.md b/REFERENCE.md
index ed9c572a..aa76452a 100644
--- a/REFERENCE.md
+++ b/REFERENCE.md
@@ -168,7 +168,9 @@ The following parameters are available in the `nftables` class:
 * [`log_limit`](#-nftables--log_limit)
 * [`reject_with`](#-nftables--reject_with)
 * [`in_out_conntrack`](#-nftables--in_out_conntrack)
+* [`in_out_drop_invalid`](#-nftables--in_out_drop_invalid)
 * [`fwd_conntrack`](#-nftables--fwd_conntrack)
+* [`fwd_drop_invalid`](#-nftables--fwd_drop_invalid)
 * [`firewalld_enable`](#-nftables--firewalld_enable)
 * [`noflush_tables`](#-nftables--noflush_tables)
 * [`rules`](#-nftables--rules)
@@ -324,6 +326,14 @@ established connection and also to drop invalid packets.
 
 Default value: `true`
 
+##### <a name="-nftables--in_out_drop_invalid"></a>`in_out_drop_invalid`
+
+Data type: `Boolean`
+
+Drops invalid packets in INPUT and OUTPUT
+
+Default value: `$in_out_conntrack`
+
 ##### <a name="-nftables--fwd_conntrack"></a>`fwd_conntrack`
 
 Data type: `Boolean`
@@ -333,6 +343,14 @@ established connection and also to drop invalid packets.
 
 Default value: `false`
 
+##### <a name="-nftables--fwd_drop_invalid"></a>`fwd_drop_invalid`
+
+Data type: `Boolean`
+
+Drops invalid packets in FORWARD
+
+Default value: `$fwd_conntrack`
+
 ##### <a name="-nftables--firewalld_enable"></a>`firewalld_enable`
 
 Data type: `Variant[Boolean[false], Enum['mask']]`
diff --git a/manifests/inet_filter/fwd_conntrack.pp b/manifests/inet_filter/fwd_conntrack.pp
index 7f534a27..424bc854 100644
--- a/manifests/inet_filter/fwd_conntrack.pp
+++ b/manifests/inet_filter/fwd_conntrack.pp
@@ -4,8 +4,11 @@
     'FORWARD-accept_established_related':
       order   => '05',
       content => 'ct state established,related accept';
-    'FORWARD-drop_invalid':
+  }
+  if $nftables::fwd_drop_invalid {
+    nftables::rule { 'FORWARD-drop_invalid':
       order   => '06',
       content => 'ct state invalid drop';
+    }
   }
 }
diff --git a/manifests/inet_filter/in_out_conntrack.pp b/manifests/inet_filter/in_out_conntrack.pp
index 212192cf..4d4656db 100644
--- a/manifests/inet_filter/in_out_conntrack.pp
+++ b/manifests/inet_filter/in_out_conntrack.pp
@@ -10,8 +10,11 @@
     'OUTPUT-accept_established_related':
       order   => '05',
       content => 'ct state established,related accept';
-    'OUTPUT-drop_invalid':
+  }
+  if $nftables::in_out_drop_invalid {
+    nftables::rule { 'OUTPUT-drop_invalid':
       order   => '06',
       content => 'ct state invalid drop';
+    }
   }
 }
diff --git a/manifests/init.pp b/manifests/init.pp
index 038bd6c6..94f03f50 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -73,10 +73,16 @@
 #   Adds INPUT and OUTPUT rules to allow traffic that's part of an
 #   established connection and also to drop invalid packets.
 #
+# @param in_out_drop_invalid
+#   Drops invalid packets in INPUT and OUTPUT
+#
 # @param fwd_conntrack
 #   Adds FORWARD rules to allow traffic that's part of an
 #   established connection and also to drop invalid packets.
 #
+# @param fwd_drop_invalid
+#   Drops invalid packets in FORWARD
+#
 # @param firewalld_enable
 #   Configures how the firewalld systemd service unit is enabled. It might be
 #   useful to set this to false if you're externaly removing firewalld from
@@ -117,7 +123,9 @@
   Boolean $out_icmp = true,
   Boolean $out_all = false,
   Boolean $in_out_conntrack = true,
+  Boolean $in_out_drop_invalid = $in_out_conntrack,
   Boolean $fwd_conntrack = false,
+  Boolean $fwd_drop_invalid = $fwd_conntrack,
   Boolean $inet_filter = true,
   Boolean $nat = true,
   Hash $rules = {},
diff --git a/spec/classes/inet_filter/in_out_conntrack_spec.rb b/spec/classes/inet_filter/in_out_conntrack_spec.rb
index d8f6541b..f184c97c 100644
--- a/spec/classes/inet_filter/in_out_conntrack_spec.rb
+++ b/spec/classes/inet_filter/in_out_conntrack_spec.rb
@@ -3,10 +3,15 @@
 require 'spec_helper'
 
 describe 'nftables::inet_filter::in_out_conntrack' do
-  let(:pre_condition) { 'Exec{path => "/bin"}' }
-
-  on_supported_os.each do |os, _os_facts|
+  on_supported_os.each do |os, os_facts|
+    let :pre_condition do
+      'include nftables'
+    end
     context "on #{os}" do
+      let :facts do
+        os_facts
+      end
+      it { is_expected.to compile.with_all_deps }
       it {
         expect(subject).to contain_concat__fragment('nftables-inet-filter-chain-INPUT-rule-accept_established_related').with(
           target: 'nftables-inet-filter-chain-INPUT',
@@ -38,6 +43,16 @@
           order: '06-nftables-inet-filter-chain-OUTPUT-rule-drop_invalid-b'
         )
       }
+      context 'with in_out_drop_invalid=false' do
+        let :pre_condition do
+          'class { "nftables": in_out_drop_invalid => false}'
+        end
+        it { is_expected.not_to contain_concat__fragment('nftables-inet-filter-chain-OUTPUT-rule-drop_invalid').with(
+            target: 'nftables-inet-filter-chain-OUTPUT',
+            content: %r{^  ct state invalid drop$},
+          )
+        }
+      end
     end
   end
 end