Skip to content

Commit 8da7c5a

Browse files
richmrichm
committed
fix: ensure rich rule handling is idempotent
Cause: The in-memory backend was using a key 'rich_rule' for handling the rich rules, but the code in get_config was using 'rules_str'. Consequence: The firewall module added the rich rule every time to the 'rich_rule' key, ignoring the 'rules_str' key, and was not idempotent. Fix: Ensure that the key 'rich_rule' is used everywhere. When reading the information from the firewall api, ensure that the data under the key 'rules_str' is normalized and added to the 'rich_rule' key. Result: The firewall role can manage rich rules idempotently. Signed-off-by: Rich Megginson <rmeggins@redhat.com> Fixes: #357
1 parent b62a393 commit 8da7c5a

3 files changed

Lines changed: 53 additions & 25 deletions

File tree

‎library/firewall_lib.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1948,6 +1948,8 @@ def set_masquerade(self, masquerade):
19481948
zone_config["masquerade"] = masquerade
19491949
self.changed = True
19501950

1951+
# note: rich_rule is a single string, normalized using str(Rich_Rule(rule_str=original_string))
1952+
# the zone config field 'rich_rule' is a list of normalized strings
19511953
def set_rich_rule(self, rich_rule):
19521954
"""Configure rich rules in a zone."""
19531955
for config_type, zone_config in (
@@ -1957,12 +1959,12 @@ def set_rich_rule(self, rich_rule):
19571959
if config_type and zone_config is not None:
19581960
for item in rich_rule:
19591961
if self.state == "enabled":
1960-
if item not in zone_config.get("rich_rules", []):
1961-
zone_config.setdefault("rich_rules", []).append(item)
1962+
if item not in zone_config.get("rich_rule", []):
1963+
zone_config.setdefault("rich_rule", []).append(item)
19621964
self.changed = True
19631965
elif self.state == "disabled":
1964-
if item in zone_config.get("rich_rules", []):
1965-
zone_config["rich_rules"].remove(item)
1966+
if item in zone_config.get("rich_rule", []):
1967+
zone_config["rich_rule"].remove(item)
19661968
self.changed = True
19671969

19681970
def set_source(self, source):
@@ -2481,7 +2483,7 @@ def set_rich_rule(self, rich_rule):
24812483
enable = self.check_state(["enabled", "disabled"], "rich_rule")
24822484

24832485
for item in rich_rule:
2484-
# note: item is a Rich_Rule object, but its __str__() does the right thing
2486+
# note: item is a string, normalized using str(Rich_Rule(rule_str=original_string))
24852487
cur = self.query("--zone", self.zone, "--query-rich-rule=" + item)
24862488

24872489
if cur != enable:

‎module_utils/firewall_lsr/get_config.py‎

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
try:
3434
import firewall.config
3535

36-
from firewall.client import FirewallClient
36+
from firewall.client import FirewallClient, Rich_Rule
3737

3838
# firewall.core.io modules needed for xml file reading
3939
from firewall.core.io.zone import zone_reader
@@ -97,6 +97,11 @@ def export_config_dict(io_object):
9797
object_dict["forward_ports"] = normalize_forward_ports(
9898
object_dict["forward_ports"]
9999
)
100+
if object_dict.get("rules_str"):
101+
object_dict["rich_rule"] = [
102+
str(Rich_Rule(rule_str=rule)) for rule in object_dict["rules_str"]
103+
]
104+
del object_dict["rules_str"]
100105
return object_dict
101106
else:
102107
return {}
@@ -176,15 +181,13 @@ def fetch_settings_using_offline_cmd(module, setting_name):
176181
element_settings["sources"] = offline_cmd(
177182
module, ["--zone=" + item, "--list-sources"], defaults=True
178183
).split()
179-
element_settings["rules_str"] = (
180-
offline_cmd(
184+
rich_rules = [
185+
str(Rich_Rule(rule_str=rule))
186+
for rule in offline_cmd(
181187
module, ["--zone=" + item, "--list-rich-rules"], defaults=True
182188
).split("\n")
183-
if offline_cmd(
184-
module, ["--zone=" + item, "--list-rich-rules"], defaults=True
185-
)
186-
else []
187-
)
189+
]
190+
element_settings["rich_rule"] = rich_rules
188191

189192
# Query masquerade (returns yes/no or error)
190193
try:
@@ -386,15 +389,13 @@ def fetch_settings_using_offline_cmd(module, setting_name):
386389
module, ["--policy=" + item, "--list-forward-ports"], defaults=True
387390
).split()
388391

389-
element_settings["rules_str"] = (
390-
offline_cmd(
392+
rich_rules = [
393+
str(Rich_Rule(rule_str=rule))
394+
for rule in offline_cmd(
391395
module, ["--policy=" + item, "--list-rich-rules"], defaults=True
392396
).split("\n")
393-
if offline_cmd(
394-
module, ["--policy=" + item, "--list-rich-rules"], defaults=True
395-
)
396-
else []
397-
)
397+
]
398+
element_settings["rich_rule"] = rich_rules
398399

399400
# Get ingress/egress zones
400401
try:
@@ -905,7 +906,10 @@ def fetch_online_settings(fw, setting_list, detailed=False):
905906
element_settings["forward_ports"] = element.getForwardPorts()
906907
element_settings["interfaces"] = element.getInterfaces()
907908
element_settings["sources"] = element.getSources()
908-
element_settings["rules_str"] = element.getRichRules()
909+
element_settings["rich_rule"] = [
910+
str(Rich_Rule(rule_str=rule))
911+
for rule in element.getRichRules()
912+
]
909913
element_settings["protocols"] = element.getProtocols()
910914
element_settings["source_ports"] = element.getSourcePorts()
911915
element_settings["icmp_block_inversion"] = (
@@ -916,6 +920,12 @@ def fetch_online_settings(fw, setting_list, detailed=False):
916920
element_settings["forward_ports"] = normalize_forward_ports(
917921
element_settings["forward_ports"]
918922
)
923+
if "rules_str" in element_settings:
924+
element_settings["rich_rule"] = [
925+
str(Rich_Rule(rule_str=rule))
926+
for rule in element_settings["rules_str"]
927+
]
928+
del element_settings["rules_str"]
919929
elif setting_name == "services":
920930
element = fw.getServiceSettings(_item)
921931
try:
@@ -1058,6 +1068,7 @@ def fetch_settings_from_xml_files(module, setting_list, defaults=False):
10581068
return all_settings
10591069

10601070

1071+
# currently unused
10611072
def fetch_settings_from_dir(directory, detailed=False, fw=None):
10621073
setting_options = [
10631074
_file[:-4] for _file in os.listdir(directory) if _file.endswith(".xml")
@@ -1085,7 +1096,9 @@ def fetch_settings_from_dir(directory, detailed=False, fw=None):
10851096
element_settings["forward_ports"] = element.getForwardPorts()
10861097
element_settings["interfaces"] = element.getInterfaces()
10871098
element_settings["sources"] = element.getSources()
1088-
element_settings["rules_str"] = element.getRichRules()
1099+
element_settings["rich_rule"] = [
1100+
str(Rich_Rule(rule_str=rule)) for rule in element.getRichRules()
1101+
]
10891102
element_settings["protocols"] = element.getProtocols()
10901103
element_settings["source_ports"] = element.getSourcePorts()
10911104
element_settings["icmp_block_inversion"] = (

‎tests/tests_zone.yml‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,14 @@
4545
forward_port: ['447/tcp;;1.2.3.4',
4646
'448/tcp;;1.2.3.5']
4747
state: 'enabled'
48+
rich_rule: 'rule family="ipv4" source address="192.0.2.0/24" reject'
4849
- zone: 'internal'
4950
service: ['tftp', 'ftp']
5051
port: ['443/tcp', '443/udp']
5152
forward_port: ['447/tcp;;1.2.3.4',
5253
'448/tcp;;1.2.3.5']
5354
state: 'enabled'
55+
rich_rule: 'rule family="ipv4" source address="192.0.2.0/24" reject'
5456
- zone: customzone
5557
state: present
5658
- zone: customzone
@@ -94,12 +96,14 @@
9496
forward_port: ['447/tcp;;1.2.3.4',
9597
'448/tcp;;1.2.3.5']
9698
state: 'enabled'
99+
rich_rule: 'rule family="ipv4" source address="192.0.2.0/24" reject'
97100
- zone: 'internal'
98101
service: ['tftp', 'ftp']
99102
port: ['443/tcp', '443/udp']
100103
forward_port: ['447/tcp;;1.2.3.4',
101104
'448/tcp;;1.2.3.5']
102105
state: 'enabled'
106+
rich_rule: 'rule family="ipv4" source address="192.0.2.0/24" reject'
103107
- zone: customzone
104108
state: present
105109
- zone: customzone
@@ -146,12 +150,14 @@
146150
forward_port: ['447/tcp;;1.2.3.4',
147151
'448/tcp;;1.2.3.5']
148152
state: 'enabled'
153+
rich_rule: 'rule family="ipv4" source address="192.0.2.0/24" reject'
149154
- zone: 'internal'
150155
service: ['tftp', 'ftp']
151156
port: ['443/tcp', '443/udp']
152157
forward_port: ['447/tcp;;1.2.3.4',
153158
'448/tcp;;1.2.3.5']
154159
state: 'enabled'
160+
rich_rule: 'rule family="ipv4" source address="192.0.2.0/24" reject'
155161
- zone: customzone
156162
state: present
157163
- zone: customzone
@@ -198,23 +204,23 @@
198204
command: firewall-offline-cmd --zone=internal --list-services
199205
register: result
200206
changed_when: false
201-
failed_when: result.failed
207+
failed_when: result is failed
202208
or "tftp" not in result.stdout
203209
or "ftp" not in result.stdout
204210

205211
- name: Verify firewalld zone internal ports
206212
command: firewall-offline-cmd --zone=internal --list-ports
207213
register: result
208214
changed_when: false
209-
failed_when: result.failed
215+
failed_when: result is failed
210216
or "443/tcp" not in result.stdout
211217
or "443/udp" not in result.stdout
212218

213219
- name: Verify firewalld zone internal forward ports
214220
command: firewall-offline-cmd --zone=internal --list-forward-ports
215221
register: result
216222
changed_when: false
217-
failed_when: result.failed
223+
failed_when: result is failed
218224
or "port=447:proto=tcp:toport=:toaddr=1.2.3.4"
219225
not in result.stdout
220226
or "port=448:proto=tcp:toport=:toaddr=1.2.3.5"
@@ -241,6 +247,13 @@
241247
failed_when: result is failed
242248
or "yes" not in result.stdout
243249

250+
- name: Verify rich rule has been added to the internal zone
251+
command: firewall-offline-cmd --zone=internal --list-rich-rules
252+
register: result
253+
changed_when: false
254+
failed_when: result is failed
255+
or 'rule family="ipv4" source address="192.0.2.0/24" reject' not in result.stdout
256+
244257
- name: Fail if ipsets not added
245258
shell:
246259
executable: /bin/bash

0 commit comments

Comments
 (0)