From 628a53eb1a73e14f65ab3d5b1a66fbad209b1042 Mon Sep 17 00:00:00 2001 From: Gregory Furlong Date: Wed, 14 Dec 2022 16:59:19 -0500 Subject: [PATCH 1/2] Add new feature to firewalld module allowing the default zone to be set. --- plugins/modules/firewalld.py | 60 ++++++++- .../targets/firewalld/tasks/main.yml | 2 + .../firewalld/tasks/source_test_cases.yml | 2 +- .../tasks/zone_default_test_cases.yml | 115 ++++++++++++++++++ 4 files changed, 176 insertions(+), 3 deletions(-) create mode 100644 tests/integration/targets/firewalld/tasks/zone_default_test_cases.yml diff --git a/plugins/modules/firewalld.py b/plugins/modules/firewalld.py index 39a3b18c1f..27ef6ea74f 100644 --- a/plugins/modules/firewalld.py +++ b/plugins/modules/firewalld.py @@ -222,6 +222,7 @@ try: from firewall.client import Rich_Rule from firewall.client import FirewallClientZoneSettings + from firewall.config import FALLBACK_ZONE except ImportError: # The import errors are handled via FirewallTransaction, don't need to # duplicate that here @@ -695,6 +696,45 @@ def set_disabled_permanent(self): zone_obj = self.fw.config().getZoneByName(self.zone) zone_obj.remove() +class DefaultZoneTransaction(FirewallTransaction): + """ + DefaultZoneTransaction + """ + + def __init__(self, module, action_args=None, zone=None, desired_state=None, permanent=False, immediate=False): + super(DefaultZoneTransaction, self).__init__( + module, action_args=action_args, desired_state=desired_state, zone=zone, permanent=permanent, immediate=immediate + ) + self.upstream_default_zone = FALLBACK_ZONE + self.enabled_msg = "Updated default zone to %s" % self.zone + self.disabled_msg = "Reverted default zone from %s to upstream default %s" % (self.zone, self.upstream_default_zone) + self.tx_not_permanent_error_msg = "Zone operations must be permanent. " \ + "Make sure you didn't set the 'permanent' flag to 'false' or the 'immediate' flag to 'true'." + + def get_enabled_immediate(self): + self.module.fail_json(msg=self.tx_not_permanent_error_msg) + + def get_enabled_permanent(self): + default_zone = self.fw.get_default_zone() if fw_offline else self.fw.getDefaultZone() + return self.zone == default_zone + + def set_enabled_immediate(self): + self.module.fail_json(msg=self.tx_not_permanent_error_msg) + + def set_enabled_permanent(self): + if fw_offline: + self.fw.set_default_zone(self.zone) + else: + self.fw.setDefaultZone(self.zone) + + def set_disabled_immediate(self): + self.module.fail_json(msg=self.tx_not_permanent_error_msg) + + def set_disabled_permanent(self): + if fw_offline: + self.fw.set_default_zone(self.upstream_default_zone) + else: + self.fw.setDefaultZone(self.upstream_default_zone) class ForwardPortTransaction(FirewallTransaction): """ @@ -732,7 +772,6 @@ def set_disabled_permanent(self, port, proto, toport, toaddr, timeout): fw_settings.removeForwardPort(port, proto, toport, toaddr) self.update_fw_settings(fw_zone, fw_settings) - def main(): module = AnsibleModule( @@ -751,6 +790,7 @@ def main(): timeout=dict(type='int', default=0), interface=dict(type='str'), masquerade=dict(type='str'), + default=dict(type='bool'), offline=dict(type='bool'), target=dict(type='str', choices=['default', 'ACCEPT', 'DROP', '%%REJECT%%']), ), @@ -758,11 +798,12 @@ def main(): required_by=dict( interface=('zone',), target=('zone',), + default=('zone',), source=('permanent',), ), mutually_exclusive=[ ['icmp_block', 'icmp_block_inversion', 'service', 'port', 'port_forward', 'rich_rule', - 'interface', 'masquerade', 'source', 'target'] + 'interface', 'masquerade', 'source', 'target','default'] ], ) @@ -772,6 +813,7 @@ def main(): timeout = module.params['timeout'] interface = module.params['interface'] masquerade = module.params['masquerade'] + default = module.params['default'] # Sanity checks FirewallTransaction.sanity_check(module) @@ -1008,6 +1050,20 @@ def main(): changed, transaction_msgs = transaction.run() msgs = msgs + transaction_msgs + if default is not None: + expected_state = 'enabled' if (desired_state == 'enabled') == default else 'disabled' + transaction = DefaultZoneTransaction( + module, + action_args=(), + zone=zone, + desired_state=expected_state, + permanent=permanent, + immediate=immediate, + ) + + changed, transaction_msgs = transaction.run() + msgs = msgs + transaction_msgs + ''' If there are no changes within the zone we are operating on the zone itself ''' if not modification and desired_state in ['absent', 'present']: diff --git a/tests/integration/targets/firewalld/tasks/main.yml b/tests/integration/targets/firewalld/tasks/main.yml index 17f14c2b56..6aa8dfec1c 100644 --- a/tests/integration/targets/firewalld/tasks/main.yml +++ b/tests/integration/targets/firewalld/tasks/main.yml @@ -40,6 +40,8 @@ state: stopped - import_tasks: run_all_tests.yml + # FIXME currently fails when the daemon is running AND executed inside a container + - import_tasks: zone_default_test_cases.yml when: check_output.rc == 0 when: diff --git a/tests/integration/targets/firewalld/tasks/source_test_cases.yml b/tests/integration/targets/firewalld/tasks/source_test_cases.yml index 172a47e02b..2ab2f2544c 100644 --- a/tests/integration/targets/firewalld/tasks/source_test_cases.yml +++ b/tests/integration/targets/firewalld/tasks/source_test_cases.yml @@ -82,4 +82,4 @@ assert: that: - result is not changed - - "result.msg == 'parameters are mutually exclusive: icmp_block|icmp_block_inversion|service|port|port_forward|rich_rule|interface|masquerade|source|target'" + - "result.msg == 'parameters are mutually exclusive: icmp_block|icmp_block_inversion|service|port|port_forward|rich_rule|interface|masquerade|source|target|default'" diff --git a/tests/integration/targets/firewalld/tasks/zone_default_test_cases.yml b/tests/integration/targets/firewalld/tasks/zone_default_test_cases.yml new file mode 100644 index 0000000000..18a207826c --- /dev/null +++ b/tests/integration/targets/firewalld/tasks/zone_default_test_cases.yml @@ -0,0 +1,115 @@ +# Test playbook for the firewalld module - zone default operations +# (c) 2022, Gregory Furlong +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +- name: Set default zone to trusted (default - true / state - enabled) + block: + - name: Update default zone to trusted + ansible.posix.firewalld: + zone: trusted + default: True + permanent: True + state: enabled + register: result + + - name: Default zone is trusted + ansible.builtin.assert: + that: + - result is changed + + - name: Update default zone to trusted (verify not changed) + ansible.posix.firewalld: + zone: trusted + default: True + permanent: True + state: enabled + register: result + + - name: Default zone is trusted (verify not changed) + ansible.builtin.assert: + that: + - result is not changed + +- name: Revert default zone to upstream default (default - false / state - enabled) + block: + - name: Revert default zone to upstream default + ansible.posix.firewalld: + zone: trusted + default: False + permanent: True + state: enabled + register: result + + - name: Default zone is reverted + ansible.builtin.assert: + that: + - result is changed + + - name: Revert default zone to upstream default (verify not changed) + ansible.posix.firewalld: + zone: trusted + default: False + permanent: True + state: enabled + register: result + + - name: Default zone is reverted (verify not changed) + ansible.builtin.assert: + that: + - result is not changed + +- name: Set default zone to trusted (default - false / state - disabled) + block: + - name: Update default zone to trusted + ansible.posix.firewalld: + zone: trusted + default: False + permanent: True + state: disabled + register: result + + - name: Default zone is trusted + ansible.builtin.assert: + that: + - result is changed + + - name: Update default zone to trusted (verify not changed) + ansible.posix.firewalld: + zone: trusted + default: False + permanent: True + state: disabled + register: result + + - name: Default zone is trusted (verify not changed) + ansible.builtin.assert: + that: + - result is not changed + +- name: Revert default zone to upstream default (default - true / state - disabled) + block: + - name: Revert default zone to upstream default + ansible.posix.firewalld: + zone: trusted + default: True + permanent: True + state: disabled + register: result + + - name: Default zone is reverted + ansible.builtin.assert: + that: + - result is changed + + - name: Revert default zone to upstream default (verify not changed) + ansible.posix.firewalld: + zone: trusted + default: True + permanent: True + state: disabled + register: result + + - name: Default zone is reverted (verify not changed) + ansible.builtin.assert: + that: + - result is not changed From 0438630004d2f44d1515fd92bc2a28831b244dea Mon Sep 17 00:00:00 2001 From: Gregory Furlong Date: Thu, 15 Dec 2022 10:48:52 -0500 Subject: [PATCH 2/2] Add missing documentation and fix linting errors introducted with firewalld default parameter. Update to fail if not explicitly both immedate AND permanent when the firewall daemon is online. --- plugins/modules/firewalld.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/plugins/modules/firewalld.py b/plugins/modules/firewalld.py index 27ef6ea74f..a0dc8342b0 100644 --- a/plugins/modules/firewalld.py +++ b/plugins/modules/firewalld.py @@ -106,6 +106,11 @@ description: - The masquerade setting you would like to enable/disable to/from zones within firewalld. type: str + default: + description: + - Indicates that the targeted zone should be set as firewalld's default zone. + - This change must always be both immediate (when firewalld is running) and permanent. + type: bool offline: description: - Whether to run this module even when firewalld is offline. @@ -213,6 +218,13 @@ permanent: yes immediate: yes state: enabled + +- name: Set the default zone to 'trusted' + ansible.builtin.firewalld: + zone: trusted + permanent: true + default: true + state: enabled ''' from ansible.module_utils.basic import AnsibleModule @@ -696,6 +708,7 @@ def set_disabled_permanent(self): zone_obj = self.fw.config().getZoneByName(self.zone) zone_obj.remove() + class DefaultZoneTransaction(FirewallTransaction): """ DefaultZoneTransaction @@ -708,18 +721,18 @@ def __init__(self, module, action_args=None, zone=None, desired_state=None, perm self.upstream_default_zone = FALLBACK_ZONE self.enabled_msg = "Updated default zone to %s" % self.zone self.disabled_msg = "Reverted default zone from %s to upstream default %s" % (self.zone, self.upstream_default_zone) - self.tx_not_permanent_error_msg = "Zone operations must be permanent. " \ - "Make sure you didn't set the 'permanent' flag to 'false' or the 'immediate' flag to 'true'." + if (not permanent) or not (fw_offline or immediate): + self.module.fail_json(msg="Default zone changes must be permanent and when daemon is online must also be immediate") def get_enabled_immediate(self): - self.module.fail_json(msg=self.tx_not_permanent_error_msg) + return self.fw.getDefaultZone() == self.zone def get_enabled_permanent(self): default_zone = self.fw.get_default_zone() if fw_offline else self.fw.getDefaultZone() return self.zone == default_zone def set_enabled_immediate(self): - self.module.fail_json(msg=self.tx_not_permanent_error_msg) + pass # permanent default zone change will also apply immediately to a running daemon def set_enabled_permanent(self): if fw_offline: @@ -728,7 +741,7 @@ def set_enabled_permanent(self): self.fw.setDefaultZone(self.zone) def set_disabled_immediate(self): - self.module.fail_json(msg=self.tx_not_permanent_error_msg) + pass # permanent default zone change will also apply immediately to a running daemon def set_disabled_permanent(self): if fw_offline: @@ -736,6 +749,7 @@ def set_disabled_permanent(self): else: self.fw.setDefaultZone(self.upstream_default_zone) + class ForwardPortTransaction(FirewallTransaction): """ ForwardPortTransaction @@ -772,6 +786,7 @@ def set_disabled_permanent(self, port, proto, toport, toaddr, timeout): fw_settings.removeForwardPort(port, proto, toport, toaddr) self.update_fw_settings(fw_zone, fw_settings) + def main(): module = AnsibleModule( @@ -803,7 +818,7 @@ def main(): ), mutually_exclusive=[ ['icmp_block', 'icmp_block_inversion', 'service', 'port', 'port_forward', 'rich_rule', - 'interface', 'masquerade', 'source', 'target','default'] + 'interface', 'masquerade', 'source', 'target', 'default'] ], )