Skip to content

Commit 3e8717d

Browse files
malikparvezmalikparvez
authored
Merge pull request #1210 from 2fa/fix-rule-parsing
2 parents 69aac41 + 4a57b75 commit 3e8717d

File tree

2 files changed

+360
-3
lines changed

2 files changed

+360
-3
lines changed

‎lib/puppet/provider/firewall/firewall.rb‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class Puppet::Provider::Firewall::Firewall
1919
# Regex used to retrieve table name
2020
$table_name_regex = %r{^\*(nat|mangle|filter|raw|rawpost|broute|security)}
2121
# Regex used to retrieve Rules
22-
$rules_regex = %r{(-A.*)\n}
22+
$rules_regex = %r{^(-A.*)\n}
2323
# Base command
2424
$base_command = {
2525
'IPv4' => 'iptables -t',
@@ -466,6 +466,9 @@ def self.get_rules(context, basic, protocols = ['IPv4', 'IPv6'])
466466
iptables_list.scan($table_regex).each do |table|
467467
table_name = table[0].scan($table_name_regex)[0][0]
468468
table[0].scan($rules_regex).each do |rule|
469+
# iptables-save escapes ' symbol in it's output for some reason which leads to an incorrect command
470+
# We need to manually replace \' to '
471+
rule[0].gsub!("\\'", "'")
469472
raw_rules = if basic
470473
Puppet::Provider::Firewall::Firewall.rule_to_name(context, rule[0], table_name, protocol)
471474
else
@@ -489,7 +492,7 @@ def self.rule_to_name(_context, rule, table_name, protocol)
489492
rule_hash[:table] = table_name
490493
rule_hash[:protocol] = protocol
491494

492-
name_regex = Regexp.new("#{$resource_map[:name]}\\s(?:\"([^\"]*)|([^\"\\s]*))")
495+
name_regex = Regexp.new("#{$resource_map[:name]}\\s+(?:\"(.+?(?<!\\\\))\"|([^\"\\s]+)\\b)(?:\\s|$)")
493496
name_value = rule.scan(name_regex)[0]
494497
# Combine the returned values and remove and trailing or leading whitespace
495498
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)
527530
# When only a single word comment is returned no quotes are given, so we must check for this as well
528531
# First find if flag is present, add a space to ensure accuracy with the more simplistic flags; i.e. `-i`
529532
if rule.match(Regexp.new("#{value}\\s"))
530-
value_regex = Regexp.new("(?:(!\\s))?#{value}\\s(?:\"([^\"]*)|([^\"\\s]*))")
533+
value_regex = Regexp.new("(?:(!\\s))?#{value}\\s+(?:\"(.+?(?<!\\\\))\"|([^\"\\s]+)\\b)(?:\\s|$)")
531534
key_value = rule.scan(value_regex)[0]
532535
# Combine the returned values and remove and trailing or leading whitespace
533536
key_value[1] = [key_value[0], key_value[1], key_value[2]].join

‎spec/unit/puppet/provider/firewall/firewall_output_parsing_spec.rb‎

Lines changed: 354 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,354 @@
1+
# frozen_string_literal: true
2+
3+
require 'spec_helper'
4+
require 'puppet/resource_api'
5+
6+
ensure_module_defined('Puppet::Provider::Firewall')
7+
require 'puppet/provider/firewall/firewall'
8+
9+
RSpec.describe Puppet::Provider::Firewall::Firewall do
10+
describe 'iptables-save output parsing' do
11+
subject(:provider) { described_class.new }
12+
13+
let(:type) { Puppet::Type.type('firewall') }
14+
let(:context) { Puppet::ResourceApi::BaseContext.new(type.type_definition.definition) }
15+
16+
describe 'get(_context)' do
17+
let(:iptables) do
18+
'
19+
# Generated by iptables-save v1.8.4 on Thu Aug 10 10:15:14 2023
20+
*filter
21+
:INPUT ACCEPT [62:3308]
22+
:FORWARD ACCEPT [0:0]
23+
:OUTPUT ACCEPT [39:3092]
24+
:TEST_ONE - [0:0]
25+
:TEST-ANOTHER - [0:0]
26+
COMMIT
27+
-A TEST_ONE -p tcp -m comment --comment "001 custom chain test rule"
28+
-A INPUT -p tcp -m comment --comment "002 \"double-quotes\" test rule"
29+
-A INPUT -p tcp -m comment --comment "007 \'single-quotes\' test rule"
30+
-A TEST-ANOTHER -p tcp -m comment --comment "003 test -A in chain name"
31+
-A TEST_ONE -p tcp -m comment --comment "foreign rule test"
32+
# Completed on Thu Aug 10 10:15:14 2023
33+
# Generated by iptables-save v1.8.4 on Thu Aug 10 10:15:14 2023
34+
*raw
35+
:PREROUTING ACCEPT [13222:23455532]
36+
:OUTPUT ACCEPT [12523:852730]
37+
COMMIT
38+
-A OUTPUT -p tcp -m comment --comment "004 test raw table rule"
39+
# Completed on Thu Aug 10 10:15:14 2023
40+
'
41+
end
42+
let(:ip6tables) do
43+
'
44+
# Generated by ip6tables-save v1.8.4 on Thu Aug 10 10:21:55 2023
45+
*filter
46+
:INPUT ACCEPT [0:0]
47+
:FORWARD ACCEPT [0:0]
48+
:OUTPUT ACCEPT [13:824]
49+
:TEST_TWO - [0:0]
50+
COMMIT
51+
-A OUTPUT -p tcp -m comment --comment "005 test ipv6 rule"
52+
# Completed on Thu Aug 10 10:21:55 2023
53+
*raw
54+
:PREROUTING ACCEPT [13222:23455532]
55+
:OUTPUT ACCEPT [12523:852730]
56+
COMMIT
57+
-A TEST_TWO -p tcp -m comment --comment "006 test ipv6 rule in different table"
58+
# Completed on Thu Aug 10 10:21:55 2023
59+
'
60+
end
61+
let(:returned_data) do
62+
[
63+
{
64+
chain: 'TEST_ONE',
65+
checksum_fill: false,
66+
clamp_mss_to_pmtu: false,
67+
clusterip_new: false,
68+
ensure: 'present',
69+
ipvs: false,
70+
isfirstfrag: false,
71+
isfragment: false,
72+
ishasmorefrags: false,
73+
islastfrag: false,
74+
kernel_timezone: false,
75+
line: '-A TEST_ONE -p tcp -m comment --comment "001 custom chain test rule"',
76+
log_ip_options: false,
77+
log_tcp_options: false,
78+
log_tcp_sequence: false,
79+
log_uid: false,
80+
name: '001 custom chain test rule',
81+
notrack: false,
82+
physdev_is_bridged: false,
83+
physdev_is_in: false,
84+
physdev_is_out: false,
85+
proto: 'tcp',
86+
protocol: 'IPv4',
87+
queue_bypass: false,
88+
random: false,
89+
random_fully: false,
90+
rdest: false,
91+
reap: false,
92+
rsource: false,
93+
rttl: false,
94+
socket: false,
95+
table: 'filter',
96+
time_contiguous: false
97+
},
98+
{
99+
chain: 'INPUT',
100+
checksum_fill: false,
101+
clamp_mss_to_pmtu: false,
102+
clusterip_new: false,
103+
ensure: 'present',
104+
ipvs: false,
105+
isfirstfrag: false,
106+
isfragment: false,
107+
ishasmorefrags: false,
108+
islastfrag: false,
109+
kernel_timezone: false,
110+
line: '-A INPUT -p tcp -m comment --comment "002 \"double-quotes\" test rule"',
111+
log_ip_options: false,
112+
log_tcp_options: false,
113+
log_tcp_sequence: false,
114+
log_uid: false,
115+
name: '002 \"double-quotes\" test rule',
116+
notrack: false,
117+
physdev_is_bridged: false,
118+
physdev_is_in: false,
119+
physdev_is_out: false,
120+
proto: 'tcp',
121+
protocol: 'IPv4',
122+
queue_bypass: false,
123+
random: false,
124+
random_fully: false,
125+
rdest: false,
126+
reap: false,
127+
rsource: false,
128+
rttl: false,
129+
socket: false,
130+
table: 'filter',
131+
time_contiguous: false
132+
},
133+
{
134+
chain: 'INPUT',
135+
checksum_fill: false,
136+
clamp_mss_to_pmtu: false,
137+
clusterip_new: false,
138+
ensure: 'present',
139+
ipvs: false,
140+
isfirstfrag: false,
141+
isfragment: false,
142+
ishasmorefrags: false,
143+
islastfrag: false,
144+
kernel_timezone: false,
145+
line: '-A INPUT -p tcp -m comment --comment "007 \'single-quotes\' test rule"',
146+
log_ip_options: false,
147+
log_tcp_options: false,
148+
log_tcp_sequence: false,
149+
log_uid: false,
150+
name: '007 \'single-quotes\' test rule',
151+
notrack: false,
152+
physdev_is_bridged: false,
153+
physdev_is_in: false,
154+
physdev_is_out: false,
155+
proto: 'tcp',
156+
protocol: 'IPv4',
157+
queue_bypass: false,
158+
random: false,
159+
random_fully: false,
160+
rdest: false,
161+
reap: false,
162+
rsource: false,
163+
rttl: false,
164+
socket: false,
165+
table: 'filter',
166+
time_contiguous: false
167+
},
168+
{
169+
chain: 'TEST-ANOTHER',
170+
checksum_fill: false,
171+
clamp_mss_to_pmtu: false,
172+
clusterip_new: false,
173+
ensure: 'present',
174+
ipvs: false,
175+
isfirstfrag: false,
176+
isfragment: false,
177+
ishasmorefrags: false,
178+
islastfrag: false,
179+
kernel_timezone: false,
180+
line: '-A TEST-ANOTHER -p tcp -m comment --comment "003 test -A in chain name"',
181+
log_ip_options: false,
182+
log_tcp_options: false,
183+
log_tcp_sequence: false,
184+
log_uid: false,
185+
name: '003 test -A in chain name',
186+
notrack: false,
187+
physdev_is_bridged: false,
188+
physdev_is_in: false,
189+
physdev_is_out: false,
190+
proto: 'tcp',
191+
protocol: 'IPv4',
192+
queue_bypass: false,
193+
random: false,
194+
random_fully: false,
195+
rdest: false,
196+
reap: false,
197+
rsource: false,
198+
rttl: false,
199+
socket: false,
200+
table: 'filter',
201+
time_contiguous: false
202+
},
203+
{
204+
chain: 'TEST_ONE',
205+
checksum_fill: false,
206+
clamp_mss_to_pmtu: false,
207+
clusterip_new: false,
208+
ensure: 'present',
209+
ipvs: false,
210+
isfirstfrag: false,
211+
isfragment: false,
212+
ishasmorefrags: false,
213+
islastfrag: false,
214+
kernel_timezone: false,
215+
line: '-A TEST_ONE -p tcp -m comment --comment "foreign rule test"',
216+
log_ip_options: false,
217+
log_tcp_options: false,
218+
log_tcp_sequence: false,
219+
log_uid: false,
220+
name: '9005 foreign rule test',
221+
notrack: false,
222+
physdev_is_bridged: false,
223+
physdev_is_in: false,
224+
physdev_is_out: false,
225+
proto: 'tcp',
226+
protocol: 'IPv4',
227+
queue_bypass: false,
228+
random: false,
229+
random_fully: false,
230+
rdest: false,
231+
reap: false,
232+
rsource: false,
233+
rttl: false,
234+
socket: false,
235+
table: 'filter',
236+
time_contiguous: false
237+
},
238+
{
239+
chain: 'OUTPUT',
240+
checksum_fill: false,
241+
clamp_mss_to_pmtu: false,
242+
clusterip_new: false,
243+
ensure: 'present',
244+
ipvs: false,
245+
isfirstfrag: false,
246+
isfragment: false,
247+
ishasmorefrags: false,
248+
islastfrag: false,
249+
kernel_timezone: false,
250+
line: '-A OUTPUT -p tcp -m comment --comment "004 test raw table rule"',
251+
log_ip_options: false,
252+
log_tcp_options: false,
253+
log_tcp_sequence: false,
254+
log_uid: false,
255+
name: '004 test raw table rule',
256+
notrack: false,
257+
physdev_is_bridged: false,
258+
physdev_is_in: false,
259+
physdev_is_out: false,
260+
proto: 'tcp',
261+
protocol: 'IPv4',
262+
queue_bypass: false,
263+
random: false,
264+
random_fully: false,
265+
rdest: false,
266+
reap: false,
267+
rsource: false,
268+
rttl: false,
269+
socket: false,
270+
table: 'raw',
271+
time_contiguous: false
272+
},
273+
{
274+
chain: 'OUTPUT',
275+
checksum_fill: false,
276+
clamp_mss_to_pmtu: false,
277+
clusterip_new: false,
278+
ensure: 'present',
279+
ipvs: false,
280+
isfirstfrag: false,
281+
isfragment: false,
282+
ishasmorefrags: false,
283+
islastfrag: false,
284+
kernel_timezone: false,
285+
line: '-A OUTPUT -p tcp -m comment --comment "005 test ipv6 rule"',
286+
log_ip_options: false,
287+
log_tcp_options: false,
288+
log_tcp_sequence: false,
289+
log_uid: false,
290+
name: '005 test ipv6 rule',
291+
notrack: false,
292+
physdev_is_bridged: false,
293+
physdev_is_in: false,
294+
physdev_is_out: false,
295+
proto: 'tcp',
296+
protocol: 'IPv6',
297+
queue_bypass: false,
298+
random: false,
299+
random_fully: false,
300+
rdest: false,
301+
reap: false,
302+
rsource: false,
303+
rttl: false,
304+
socket: false,
305+
table: 'filter',
306+
time_contiguous: false
307+
},
308+
{
309+
chain: 'TEST_TWO',
310+
checksum_fill: false,
311+
clamp_mss_to_pmtu: false,
312+
clusterip_new: false,
313+
ensure: 'present',
314+
ipvs: false,
315+
isfirstfrag: false,
316+
isfragment: false,
317+
ishasmorefrags: false,
318+
islastfrag: false,
319+
kernel_timezone: false,
320+
line: '-A TEST_TWO -p tcp -m comment --comment "006 test ipv6 rule in different table"',
321+
log_ip_options: false,
322+
log_tcp_options: false,
323+
log_tcp_sequence: false,
324+
log_uid: false,
325+
name: '006 test ipv6 rule in different table',
326+
notrack: false,
327+
physdev_is_bridged: false,
328+
physdev_is_in: false,
329+
physdev_is_out: false,
330+
proto: 'tcp',
331+
protocol: 'IPv6',
332+
queue_bypass: false,
333+
random: false,
334+
random_fully: false,
335+
rdest: false,
336+
reap: false,
337+
rsource: false,
338+
rttl: false,
339+
socket: false,
340+
table: 'raw',
341+
time_contiguous: false
342+
},
343+
]
344+
end
345+
346+
it 'processes the resource' do
347+
allow(Puppet::Util::Execution).to receive(:execute).with('iptables-save').and_return(iptables)
348+
allow(Puppet::Util::Execution).to receive(:execute).with('ip6tables-save').and_return(ip6tables)
349+
350+
expect(provider.get(context)).to eq(returned_data)
351+
end
352+
end
353+
end
354+
end

0 commit comments

Comments
 (0)