From 2ab8ba3ecbd52e452cc554d515e0782801dcb4b6 Mon Sep 17 00:00:00 2001 From: Pavel Moravec 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 --- 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 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 --- 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 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 --- 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 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 --- 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