From 2685964715a07285598e9d4b675ecf4e1f2fb000 Mon Sep 17 00:00:00 2001
From: Saurabh Pandit <saurabh.pandit@perforce.com>
Date: Thu, 21 Nov 2024 17:10:37 +0530
Subject: [PATCH] (MODULE-11463): Fix rule parsing when iptables chains with
 '-A' in the name

---
 lib/puppet/provider/firewall/firewall.rb      |   9 +-
 .../firewall/firewall_output_parsing_spec.rb  | 354 ++++++++++++++++++
 2 files changed, 360 insertions(+), 3 deletions(-)
 create mode 100644 spec/unit/puppet/provider/firewall/firewall_output_parsing_spec.rb

diff --git a/lib/puppet/provider/firewall/firewall.rb b/lib/puppet/provider/firewall/firewall.rb
index 503a1b57e..61bfc1acc 100644
--- a/lib/puppet/provider/firewall/firewall.rb
+++ b/lib/puppet/provider/firewall/firewall.rb
@@ -19,7 +19,7 @@ class Puppet::Provider::Firewall::Firewall
   # Regex used to retrieve table name
   $table_name_regex = %r{^\*(nat|mangle|filter|raw|rawpost|broute|security)}
   # Regex used to retrieve Rules
-  $rules_regex = %r{(-A.*)\n}
+  $rules_regex = %r{^(-A.*)\n}
   # Base command
   $base_command = {
     'IPv4' => 'iptables -t',
@@ -466,6 +466,9 @@ def self.get_rules(context, basic, protocols = ['IPv4', 'IPv6'])
       iptables_list.scan($table_regex).each do |table|
         table_name = table[0].scan($table_name_regex)[0][0]
         table[0].scan($rules_regex).each do |rule|
+          # iptables-save escapes ' symbol in it's output for some reason which leads to an incorrect command
+          # We need to manually replace \' to '
+          rule[0].gsub!("\\'", "'")
           raw_rules = if basic
                         Puppet::Provider::Firewall::Firewall.rule_to_name(context, rule[0], table_name, protocol)
                       else
@@ -489,7 +492,7 @@ def self.rule_to_name(_context, rule, table_name, protocol)
     rule_hash[:table] = table_name
     rule_hash[:protocol] = protocol
 
-    name_regex = Regexp.new("#{$resource_map[:name]}\\s(?:\"([^\"]*)|([^\"\\s]*))")
+    name_regex = Regexp.new("#{$resource_map[:name]}\\s+(?:\"(.+?(?<!\\\\))\"|([^\"\\s]+)\\b)(?:\\s|$)")
     name_value = rule.scan(name_regex)[0]
     # Combine the returned values and remove and trailing or leading whitespace
     rule_hash[:name] = [name_value[0], name_value[1]].join(' ').strip if name_value
@@ -527,7 +530,7 @@ def self.rule_to_hash(_context, rule, table_name, protocol)
         # When only a single word comment is returned no quotes are given, so we must check for this as well
         # First find if flag is present, add a space to ensure accuracy with the more simplistic flags; i.e. `-i`
         if rule.match(Regexp.new("#{value}\\s"))
-          value_regex = Regexp.new("(?:(!\\s))?#{value}\\s(?:\"([^\"]*)|([^\"\\s]*))")
+          value_regex = Regexp.new("(?:(!\\s))?#{value}\\s+(?:\"(.+?(?<!\\\\))\"|([^\"\\s]+)\\b)(?:\\s|$)")
           key_value = rule.scan(value_regex)[0]
           # Combine the returned values and remove and trailing or leading whitespace
           key_value[1] = [key_value[0], key_value[1], key_value[2]].join
diff --git a/spec/unit/puppet/provider/firewall/firewall_output_parsing_spec.rb b/spec/unit/puppet/provider/firewall/firewall_output_parsing_spec.rb
new file mode 100644
index 000000000..6b582c344
--- /dev/null
+++ b/spec/unit/puppet/provider/firewall/firewall_output_parsing_spec.rb
@@ -0,0 +1,354 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'puppet/resource_api'
+
+ensure_module_defined('Puppet::Provider::Firewall')
+require 'puppet/provider/firewall/firewall'
+
+RSpec.describe Puppet::Provider::Firewall::Firewall do
+  describe 'iptables-save output parsing' do
+    subject(:provider) { described_class.new }
+
+    let(:type) { Puppet::Type.type('firewall') }
+    let(:context) { Puppet::ResourceApi::BaseContext.new(type.type_definition.definition) }
+
+    describe 'get(_context)' do
+      let(:iptables) do
+        '
+# Generated by iptables-save v1.8.4 on Thu Aug 10 10:15:14 2023
+*filter
+:INPUT ACCEPT [62:3308]
+:FORWARD ACCEPT [0:0]
+:OUTPUT ACCEPT [39:3092]
+:TEST_ONE - [0:0]
+:TEST-ANOTHER - [0:0]
+COMMIT
+-A TEST_ONE -p tcp -m comment --comment "001 custom chain test rule"
+-A INPUT -p tcp -m comment --comment "002 \"double-quotes\" test rule"
+-A INPUT -p tcp -m comment --comment "007 \'single-quotes\' test rule"
+-A TEST-ANOTHER -p tcp -m comment --comment "003 test -A in chain name"
+-A TEST_ONE -p tcp -m comment --comment "foreign rule test"
+# Completed on Thu Aug 10 10:15:14 2023
+# Generated by iptables-save v1.8.4 on Thu Aug 10 10:15:14 2023
+*raw
+:PREROUTING ACCEPT [13222:23455532]
+:OUTPUT ACCEPT [12523:852730]
+COMMIT
+-A OUTPUT -p tcp -m comment --comment "004 test raw table rule"
+# Completed on Thu Aug 10 10:15:14 2023
+        '
+      end
+      let(:ip6tables) do
+        '
+# Generated by ip6tables-save v1.8.4 on Thu Aug 10 10:21:55 2023
+*filter
+:INPUT ACCEPT [0:0]
+:FORWARD ACCEPT [0:0]
+:OUTPUT ACCEPT [13:824]
+:TEST_TWO - [0:0]
+COMMIT
+-A OUTPUT -p tcp -m comment --comment "005 test ipv6 rule"
+# Completed on Thu Aug 10 10:21:55 2023
+*raw
+:PREROUTING ACCEPT [13222:23455532]
+:OUTPUT ACCEPT [12523:852730]
+COMMIT
+-A TEST_TWO -p tcp -m comment --comment "006 test ipv6 rule in different table"
+# Completed on Thu Aug 10 10:21:55 2023
+        '
+      end
+      let(:returned_data) do
+        [
+          {
+            chain: 'TEST_ONE',
+            checksum_fill: false,
+            clamp_mss_to_pmtu: false,
+            clusterip_new: false,
+            ensure: 'present',
+            ipvs: false,
+            isfirstfrag: false,
+            isfragment: false,
+            ishasmorefrags: false,
+            islastfrag: false,
+            kernel_timezone: false,
+            line: '-A TEST_ONE -p tcp -m comment --comment "001 custom chain test rule"',
+            log_ip_options: false,
+            log_tcp_options: false,
+            log_tcp_sequence: false,
+            log_uid: false,
+            name: '001 custom chain test rule',
+            notrack: false,
+            physdev_is_bridged: false,
+            physdev_is_in: false,
+            physdev_is_out: false,
+            proto: 'tcp',
+            protocol: 'IPv4',
+            queue_bypass: false,
+            random: false,
+            random_fully: false,
+            rdest: false,
+            reap: false,
+            rsource: false,
+            rttl: false,
+            socket: false,
+            table: 'filter',
+            time_contiguous: false
+          },
+          {
+            chain: 'INPUT',
+            checksum_fill: false,
+            clamp_mss_to_pmtu: false,
+            clusterip_new: false,
+            ensure: 'present',
+            ipvs: false,
+            isfirstfrag: false,
+            isfragment: false,
+            ishasmorefrags: false,
+            islastfrag: false,
+            kernel_timezone: false,
+            line: '-A INPUT -p tcp -m comment --comment "002 \"double-quotes\" test rule"',
+            log_ip_options: false,
+            log_tcp_options: false,
+            log_tcp_sequence: false,
+            log_uid: false,
+            name: '002 \"double-quotes\" test rule',
+            notrack: false,
+            physdev_is_bridged: false,
+            physdev_is_in: false,
+            physdev_is_out: false,
+            proto: 'tcp',
+            protocol: 'IPv4',
+            queue_bypass: false,
+            random: false,
+            random_fully: false,
+            rdest: false,
+            reap: false,
+            rsource: false,
+            rttl: false,
+            socket: false,
+            table: 'filter',
+            time_contiguous: false
+          },
+          {
+            chain: 'INPUT',
+            checksum_fill: false,
+            clamp_mss_to_pmtu: false,
+            clusterip_new: false,
+            ensure: 'present',
+            ipvs: false,
+            isfirstfrag: false,
+            isfragment: false,
+            ishasmorefrags: false,
+            islastfrag: false,
+            kernel_timezone: false,
+            line: '-A INPUT -p tcp -m comment --comment "007 \'single-quotes\' test rule"',
+            log_ip_options: false,
+            log_tcp_options: false,
+            log_tcp_sequence: false,
+            log_uid: false,
+            name: '007 \'single-quotes\' test rule',
+            notrack: false,
+            physdev_is_bridged: false,
+            physdev_is_in: false,
+            physdev_is_out: false,
+            proto: 'tcp',
+            protocol: 'IPv4',
+            queue_bypass: false,
+            random: false,
+            random_fully: false,
+            rdest: false,
+            reap: false,
+            rsource: false,
+            rttl: false,
+            socket: false,
+            table: 'filter',
+            time_contiguous: false
+          },
+          {
+            chain: 'TEST-ANOTHER',
+            checksum_fill: false,
+            clamp_mss_to_pmtu: false,
+            clusterip_new: false,
+            ensure: 'present',
+            ipvs: false,
+            isfirstfrag: false,
+            isfragment: false,
+            ishasmorefrags: false,
+            islastfrag: false,
+            kernel_timezone: false,
+            line: '-A TEST-ANOTHER -p tcp -m comment --comment "003 test -A in chain name"',
+            log_ip_options: false,
+            log_tcp_options: false,
+            log_tcp_sequence: false,
+            log_uid: false,
+            name: '003 test -A in chain name',
+            notrack: false,
+            physdev_is_bridged: false,
+            physdev_is_in: false,
+            physdev_is_out: false,
+            proto: 'tcp',
+            protocol: 'IPv4',
+            queue_bypass: false,
+            random: false,
+            random_fully: false,
+            rdest: false,
+            reap: false,
+            rsource: false,
+            rttl: false,
+            socket: false,
+            table: 'filter',
+            time_contiguous: false
+          },
+          {
+            chain: 'TEST_ONE',
+            checksum_fill: false,
+            clamp_mss_to_pmtu: false,
+            clusterip_new: false,
+            ensure: 'present',
+            ipvs: false,
+            isfirstfrag: false,
+            isfragment: false,
+            ishasmorefrags: false,
+            islastfrag: false,
+            kernel_timezone: false,
+            line: '-A TEST_ONE -p tcp -m comment --comment "foreign rule test"',
+            log_ip_options: false,
+            log_tcp_options: false,
+            log_tcp_sequence: false,
+            log_uid: false,
+            name: '9005 foreign rule test',
+            notrack: false,
+            physdev_is_bridged: false,
+            physdev_is_in: false,
+            physdev_is_out: false,
+            proto: 'tcp',
+            protocol: 'IPv4',
+            queue_bypass: false,
+            random: false,
+            random_fully: false,
+            rdest: false,
+            reap: false,
+            rsource: false,
+            rttl: false,
+            socket: false,
+            table: 'filter',
+            time_contiguous: false
+          },
+          {
+            chain: 'OUTPUT',
+            checksum_fill: false,
+            clamp_mss_to_pmtu: false,
+            clusterip_new: false,
+            ensure: 'present',
+            ipvs: false,
+            isfirstfrag: false,
+            isfragment: false,
+            ishasmorefrags: false,
+            islastfrag: false,
+            kernel_timezone: false,
+            line: '-A OUTPUT -p tcp -m comment --comment "004 test raw table rule"',
+            log_ip_options: false,
+            log_tcp_options: false,
+            log_tcp_sequence: false,
+            log_uid: false,
+            name: '004 test raw table rule',
+            notrack: false,
+            physdev_is_bridged: false,
+            physdev_is_in: false,
+            physdev_is_out: false,
+            proto: 'tcp',
+            protocol: 'IPv4',
+            queue_bypass: false,
+            random: false,
+            random_fully: false,
+            rdest: false,
+            reap: false,
+            rsource: false,
+            rttl: false,
+            socket: false,
+            table: 'raw',
+            time_contiguous: false
+          },
+          {
+            chain: 'OUTPUT',
+            checksum_fill: false,
+            clamp_mss_to_pmtu: false,
+            clusterip_new: false,
+            ensure: 'present',
+            ipvs: false,
+            isfirstfrag: false,
+            isfragment: false,
+            ishasmorefrags: false,
+            islastfrag: false,
+            kernel_timezone: false,
+            line: '-A OUTPUT -p tcp -m comment --comment "005 test ipv6 rule"',
+            log_ip_options: false,
+            log_tcp_options: false,
+            log_tcp_sequence: false,
+            log_uid: false,
+            name: '005 test ipv6 rule',
+            notrack: false,
+            physdev_is_bridged: false,
+            physdev_is_in: false,
+            physdev_is_out: false,
+            proto: 'tcp',
+            protocol: 'IPv6',
+            queue_bypass: false,
+            random: false,
+            random_fully: false,
+            rdest: false,
+            reap: false,
+            rsource: false,
+            rttl: false,
+            socket: false,
+            table: 'filter',
+            time_contiguous: false
+          },
+          {
+            chain: 'TEST_TWO',
+            checksum_fill: false,
+            clamp_mss_to_pmtu: false,
+            clusterip_new: false,
+            ensure: 'present',
+            ipvs: false,
+            isfirstfrag: false,
+            isfragment: false,
+            ishasmorefrags: false,
+            islastfrag: false,
+            kernel_timezone: false,
+            line: '-A TEST_TWO -p tcp -m comment --comment "006 test ipv6 rule in different table"',
+            log_ip_options: false,
+            log_tcp_options: false,
+            log_tcp_sequence: false,
+            log_uid: false,
+            name: '006 test ipv6 rule in different table',
+            notrack: false,
+            physdev_is_bridged: false,
+            physdev_is_in: false,
+            physdev_is_out: false,
+            proto: 'tcp',
+            protocol: 'IPv6',
+            queue_bypass: false,
+            random: false,
+            random_fully: false,
+            rdest: false,
+            reap: false,
+            rsource: false,
+            rttl: false,
+            socket: false,
+            table: 'raw',
+            time_contiguous: false
+          },
+        ]
+      end
+
+      it 'processes the resource' do
+        allow(Puppet::Util::Execution).to receive(:execute).with('iptables-save').and_return(iptables)
+        allow(Puppet::Util::Execution).to receive(:execute).with('ip6tables-save').and_return(ip6tables)
+
+        expect(provider.get(context)).to eq(returned_data)
+      end
+    end
+  end
+end