From 2ab8ba3ecbd52e452cc554d515e0782801dcb4b6 Mon Sep 17 00:00:00 2001

From: Pavel Moravec <pmoravec@redhat.com>

Date: Wed, 8 Sep 2021 15:31:48 +0200

Subject: [PATCH] [firewalld] collect nft rules in firewall_tables only

We collect 'nft list ruleset' in both plugins, while:

- nft is not shipped by firewalld package, so we should not collect

it in firewalld plugin

- running the command requires both nf_tables and nfnetlink kmods, so

we should use both kmods in the predicate

Resolves: #2679

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>

---

 sos/report/plugins/firewall_tables.py | 9 +++++----

 sos/report/plugins/firewalld.py       | 8 +-------

 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/sos/report/plugins/firewall_tables.py b/sos/report/plugins/firewall_tables.py

index 56058d3bf9..63a7dddeb5 100644

--- a/sos/report/plugins/firewall_tables.py

+++ b/sos/report/plugins/firewall_tables.py

@@ -40,10 +40,11 @@ def collect_nftables(self):

         """ Collects nftables rulesets with 'nft' commands if the modules

         are present """

-        self.add_cmd_output(

-            "nft list ruleset",

-            pred=SoSPredicate(self, kmods=['nf_tables'])

-        )

+        # collect nftables ruleset

+        nft_pred = SoSPredicate(self,

+                                kmods=['nf_tables', 'nfnetlink'],

+                                required={'kmods': 'all'})

+        self.add_cmd_output("nft list ruleset", pred=nft_pred, changes=True)

     def setup(self):

         # collect iptables -t for any existing table, if we can't read the

diff --git a/sos/report/plugins/firewalld.py b/sos/report/plugins/firewalld.py

index ec83527ed7..9401bfd239 100644

--- a/sos/report/plugins/firewalld.py

+++ b/sos/report/plugins/firewalld.py

@@ -9,7 +9,7 @@

 #

 # See the LICENSE file in the source distribution for further information.

-from sos.report.plugins import Plugin, RedHatPlugin, SoSPredicate

+from sos.report.plugins import Plugin, RedHatPlugin

 class FirewallD(Plugin, RedHatPlugin):

@@ -35,12 +35,6 @@ def setup(self):

             "/var/log/firewalld",

         ])

-        # collect nftables ruleset

-        nft_pred = SoSPredicate(self,

-                                kmods=['nf_tables', 'nfnetlink'],

-                                required={'kmods': 'all'})

-        self.add_cmd_output("nft list ruleset", pred=nft_pred, changes=True)

-

         # use a 10s timeout to workaround dbus problems in

         # docker containers.

         self.add_cmd_output([

-- 

2.31.1

From 2a7cf53b61943907dc823cf893530b620a87946c Mon Sep 17 00:00:00 2001

From: Pavel Moravec <pmoravec@redhat.com>

Date: Fri, 15 Oct 2021 22:31:36 +0200

Subject: [PATCH 1/3] [report] Use log_skipped_cmd method inside

 collect_cmd_output

Also, remove obsolete parameters of the log_skipped_cmd method.

Related: #2724

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>

---

 sos/report/plugins/__init__.py | 26 ++++++++------------------

 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/sos/report/plugins/__init__.py b/sos/report/plugins/__init__.py

index ec138f83..b60ab5f6 100644

--- a/sos/report/plugins/__init__.py

+++ b/sos/report/plugins/__init__.py

@@ -876,8 +876,7 @@ class Plugin():

             return bool(pred)

         return False

-    def log_skipped_cmd(self, pred, cmd, kmods=False, services=False,

-                        changes=False):

+    def log_skipped_cmd(self, cmd, pred, changes=False):

         """Log that a command was skipped due to predicate evaluation.

         Emit a warning message indicating that a command was skipped due

@@ -887,21 +886,17 @@ class Plugin():

         message indicating that the missing data can be collected by using

         the "--allow-system-changes" command line option will be included.

-        :param pred:    The predicate that caused the command to be skipped

-        :type pred:     ``SoSPredicate``

-

         :param cmd:     The command that was skipped

         :type cmd:      ``str``

-        :param kmods:   Did kernel modules cause the command to be skipped

-        :type kmods:    ``bool``

-

-        :param services: Did services cause the command to be skipped

-        :type services: ``bool``

+        :param pred:    The predicate that caused the command to be skipped

+        :type pred:     ``SoSPredicate``

         :param changes: Is the `--allow-system-changes` enabled

         :type changes:  ``bool``

         """

+        if pred is None:

+            pred = SoSPredicate(self)

         msg = "skipped command '%s': %s" % (cmd, pred.report_failure())

         if changes:

@@ -1700,9 +1693,7 @@ class Plugin():

             self.collect_cmds.append(soscmd)

             self._log_info("added cmd output '%s'" % soscmd.cmd)

         else:

-            self.log_skipped_cmd(pred, soscmd.cmd, kmods=bool(pred.kmods),

-                                 services=bool(pred.services),

-                                 changes=soscmd.changes)

+            self.log_skipped_cmd(soscmd.cmd, pred, changes=soscmd.changes)

     def add_cmd_output(self, cmds, suggest_filename=None,

                        root_symlink=None, timeout=None, stderr=True,

@@ -2112,7 +2103,7 @@ class Plugin():

                            root_symlink=False, timeout=None,

                            stderr=True, chroot=True, runat=None, env=None,

                            binary=False, sizelimit=None, pred=None,

-                           subdir=None, tags=[]):

+                           changes=False, subdir=None, tags=[]):

         """Execute a command and save the output to a file for inclusion in the

         report, then return the results for further use by the plugin

@@ -2163,8 +2154,7 @@ class Plugin():

         :rtype: ``dict``

         """

         if not self.test_predicate(cmd=True, pred=pred):

-            self._log_info("skipped cmd output '%s' due to predicate (%s)" %

-                           (cmd, self.get_predicate(cmd=True, pred=pred)))

+            self.log_skipped_cmd(cmd, pred, changes=changes)

             return {

                 'status': None,  # don't match on if result['status'] checks

                 'output': '',

-- 

2.31.1

From 6b1bea0ffb1df7f8e5001b06cf25f0741b007ddd Mon Sep 17 00:00:00 2001

From: Pavel Moravec <pmoravec@redhat.com>

Date: Fri, 15 Oct 2021 22:34:01 +0200

Subject: [PATCH 2/3] [firewall_tables] call iptables -t  based on nft

 list

If iptables are not realy in use, calling iptables -t 

would load corresponding nft table.

Therefore, call iptables -t only for the tables from "nft list ruleset"

output.

Example: nft list ruleset contains

table ip mangle {

..

}

so we can collect iptable -t mangle -nvL .

The same applies to ip6tables as well.

Resolves: #2724

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>

---

 sos/report/plugins/firewall_tables.py | 29 ++++++++++++++++++++-------

 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/sos/report/plugins/firewall_tables.py b/sos/report/plugins/firewall_tables.py

index 63a7ddde..ef04d939 100644

--- a/sos/report/plugins/firewall_tables.py

+++ b/sos/report/plugins/firewall_tables.py

@@ -44,26 +44,41 @@ class firewall_tables(Plugin, IndependentPlugin):

         nft_pred = SoSPredicate(self,

                                 kmods=['nf_tables', 'nfnetlink'],

                                 required={'kmods': 'all'})

-        self.add_cmd_output("nft list ruleset", pred=nft_pred, changes=True)

+        return self.collect_cmd_output("nft list ruleset", pred=nft_pred,

+                                       changes=True)

     def setup(self):

+        # first, collect "nft list ruleset" as collecting commands like

+        # ip6tables -t mangle -nvL

+        # depends on its output

+        # store in nft_ip_tables lists of ip[|6] tables from nft list

+        nft_list = self.collect_nftables()

+        nft_ip_tables = {'ip': [], 'ip6': []}

+        nft_lines = nft_list['output'] if nft_list['status'] == 0 else ''

+        for line in nft_lines.splitlines():

+            words = line.split()[0:3]

+            if len(words) == 3 and words[0] == 'table' and \

+                    words[1] in nft_ip_tables.keys():

+                nft_ip_tables[words[1]].append(words[2])

         # collect iptables -t for any existing table, if we can't read the

         # tables, collect 2 default ones (mangle, filter)

+        # do collect them only when relevant nft list ruleset exists

+        default_ip_tables = "mangle\nfilter\n"

         try:

             ip_tables_names = open("/proc/net/ip_tables_names").read()

         except IOError:

-            ip_tables_names = "mangle\nfilter\n"

+            ip_tables_names = default_ip_tables

         for table in ip_tables_names.splitlines():

-            self.collect_iptable(table)

+            if nft_list['status'] == 0 and table in nft_ip_tables['ip']:

+                self.collect_iptable(table)

         # collect the same for ip6tables

         try:

             ip_tables_names = open("/proc/net/ip6_tables_names").read()

         except IOError:

-            ip_tables_names = "mangle\nfilter\n"

+            ip_tables_names = default_ip_tables

         for table in ip_tables_names.splitlines():

-            self.collect_ip6table(table)

-

-        self.collect_nftables()

+            if nft_list['status'] == 0 and table in nft_ip_tables['ip6']:

+                self.collect_ip6table(table)

         # When iptables is called it will load the modules

         # iptables_filter (for kernel <= 3) or

-- 

2.31.1

From 464bd2d2e83f203e369f2ba7671bbb7da53e06f6 Mon Sep 17 00:00:00 2001

From: Pavel Moravec <pmoravec@redhat.com>

Date: Sun, 24 Oct 2021 16:00:31 +0200

Subject: [PATCH 3/3] [firewall_tables] Call iptables only when nft ip filter

 table exists

iptables -vnxL creates nft 'ip filter' table if it does not exist, hence

we must guard iptables execution by presence of the nft table.

An equivalent logic applies to ip6tables.

Resolves: #2724

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>

---

 sos/report/plugins/firewall_tables.py | 26 ++++++++++++++------------

 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/sos/report/plugins/firewall_tables.py b/sos/report/plugins/firewall_tables.py

index ef04d939..7eafd60f 100644

--- a/sos/report/plugins/firewall_tables.py

+++ b/sos/report/plugins/firewall_tables.py

@@ -80,19 +80,21 @@ class firewall_tables(Plugin, IndependentPlugin):

             if nft_list['status'] == 0 and table in nft_ip_tables['ip6']:

                 self.collect_ip6table(table)

-        # When iptables is called it will load the modules

-        # iptables_filter (for kernel <= 3) or

-        # nf_tables (for kernel >= 4) if they are not loaded.

+        # When iptables is called it will load:

+        # 1) the modules iptables_filter (for kernel <= 3) or

+        #    nf_tables (for kernel >= 4) if they are not loaded.

+        # 2) nft 'ip filter' table will be created

         # The same goes for ipv6.

-        self.add_cmd_output(

-            "iptables -vnxL",

-            pred=SoSPredicate(self, kmods=['iptable_filter', 'nf_tables'])

-        )

-

-        self.add_cmd_output(

-            "ip6tables -vnxL",

-            pred=SoSPredicate(self, kmods=['ip6table_filter', 'nf_tables'])

-        )

+        if nft_list['status'] != 0 or 'filter' in nft_ip_tables['ip']:

+            self.add_cmd_output(

+                "iptables -vnxL",

+                pred=SoSPredicate(self, kmods=['iptable_filter', 'nf_tables'])

+            )

+        if nft_list['status'] != 0 or 'filter' in nft_ip_tables['ip6']:

+            self.add_cmd_output(

+                "ip6tables -vnxL",

+                pred=SoSPredicate(self, kmods=['ip6table_filter', 'nf_tables'])

+            )

         self.add_copy_spec([

             "/etc/nftables",

-- 

2.31.1