linux-system-roles / firewall

Configure firewalld and system-config-firewall
https://linux-system-roles.github.io/firewall/
GNU General Public License v2.0
57 stars 32 forks source link

/etc/firewalld/firewalld.conf is resetted, and all default comment are deleted #127

Closed bdouxx closed 1 year ago

bdouxx commented 1 year ago

What happened:

/etc/firewalld/firewalld.conf is resetted, and all default comment are deleted

What you expected to happen: only parameters are modified

How to reproduce it (as minimally and precisely as possible):

---

- name: "[@@ firewalld @@]"
  ansible.builtin.include_role:
    name: linux-system-roles.firewall
  vars:
    firewall:
      - previous: replaced

      - zone: myownzone
        state: present

      - zone: myownzone
        service: [ssh]
        icmp_block_inversion: true
        icmp_block: [ echo-reply, echo-request ]
        rich_rule: rule family="ipv4" source address="192.168.1.95" service name="zabbix-agent" accept
        state: enabled
        target: "%%REJECT%%"
        permanent: true

      - set_default_zone: myownzone
        permanent: true
# cat /etc/firewalld/firewalld.conf

DefaultZone=myownzone
CleanupOnExit=yes
CleanupModulesOnExit=yes
Lockdown=no
IPv6_rpfilter=yes
IndividualCalls=no
LogDenied=off
FirewallBackend=nftables
FlushAllOnReload=yes
RFC3964_IPv4=yes
AllowZoneDrifting=yes
# cat /etc/firewalld/firewalld.conf.rpmnew
# firewalld config file

# default zone
# The default zone used if an empty zone string is used.
# Default: public
DefaultZone=public

# Clean up on exit
# If set to no or false the firewall configuration will not get cleaned up
# on exit or stop of firewalld.
# Default: yes
CleanupOnExit=yes

# Clean up kernel modules on exit
# If set to yes or true the firewall related kernel modules will be
# unloaded on exit or stop of firewalld. This might attempt to unload
# modules not originally loaded by firewalld.
# Default: yes
CleanupModulesOnExit=yes
....

why don't you use something like this:

- name: "[firewalld] AllowZoneDrifting=no"
  ansible.builtin.lineinfile:
    path: "/etc/firewalld/firewalld.conf"
    regexp: '^(AllowZoneDrifting=)(.*)'
    line: 'AllowZoneDrifting=no'
    insertafter: '#AllowZoneDrifting='
  notify:
    - reload firewalld

in the current ansible playbook, how to force AllowZoneDrifting=no?

richm commented 1 year ago

Is it a problem that the comments are removed?

myllynen commented 1 year ago

The problem is that this line is added back:

AllowZoneDrifting=yes

Even if zone drifting was disabled earlier then it gets re-enabled when the configuration is reset.

On RHEL 8 if AllowZoneDrifting is enabled this is the warning in the firewalld log:

WARNING: AllowZoneDrifting is enabled. This is considered an insecure configuration option. It will be removed in a future release. Please consider disabling it now.

Thanks.

BrennanPaciorek commented 1 year ago

On RHEL 8 if AllowZoneDrifting is enabled this is the warning in the firewalld log:

WARNING: AllowZoneDrifting is enabled. This is considered an insecure configuration option. It will be removed in a future release. Please consider disabling it now.

Some releases of firewalld (such as the rpm firewalld-0.9.3-13.el8.noarch.rpm, which is the most recent version available to both RHEL 8 and CentOS 8) set AllowZoneDrifting=yes by default.

While potentially insecure defaults are an issue, I do not think it is an issue that should be addressed by the system role, aside from warning the user of this default, or providing a front-end for configuring firewalld.conf directives themselves.

We do not have any feature in the system role to directly modify /etc/firewalld/firewalld.conf at present, so a workaround that I would recommend for RHEL 8 would be to modify the value of AllowZoneDrifting via dbus then reload firewalld after each call to the role that resets firewalld to its default configuration, one way to implement this with terminal commands would be:

# dbus-send --system --dest=org.fedoraproject.FirewallD1 --print-reply --type=method_call /org/fedoraproject/FirewallD1/config org.freedesktop.DBus.Properties.Set string:"org.fedoraproject.FirewallD1.config" string:"AllowZoneDrifting" string:"no"
# firewall-cmd --reload

This workaround in the context of an ansible play:

---
- name: Reset firewalld to defaults
  include_role:
    name: linux-system-roles.firewall
  vars:
    firewall:
      previous: replaced

# This should always have a zero return code, even in more recent versions of firewalld where this AllowZoneDrifting does not exist and is never allowed
- name: Ensure AllowZoneDrifting disabled
  command: >-
    dbus-send --system --dest=org.fedoraproject.FirewallD1 --print-reply
    --type=method_call /org/fedoraproject/FirewallD1/config
    org.freedesktop.DBus.Properties.Set
    string:"org.fedoraproject.FirewallD1.config"
    string:"AllowZoneDrifting"
    string:"no"

- name: reload firewalld
  command: firewall-cmd --reload

This should update /etc/firewalld/firewalld.conf to not allow zone drifting.

While potentially insecure defaults are an issue, I do not think it is an issue that should be addressed by the system role, aside from warning the user of this default, or providing a front-end for the user to modify firewalld.conf directives themselves.

Your thoughts on this @richm?

richm commented 1 year ago

The problem with requiring the user to implement workarounds like this is that, for RHEL customers, this is not a supported use of the ansible provided with/for RHEL - users may only use constructs like roles:, include_role: and import_role: to "call" system roles, anything else, like calling modules like command, shell, etc., is unsupported - so in that sense, the firewall role would somehow need to handle this case.

We do not have any feature in the system role to directly modify /etc/firewalld/firewalld.conf at present

Then we may need to add something e.g.

richm commented 1 year ago

@BrennanPaciorek can we close this issue since #162 is merged?

BrennanPaciorek commented 1 year ago

Sure, we can close this issue.

The issue described in the title is currently part of a larger issue (firewalld.conf being deleted as a result of previous: replaced), so we'll close this issue and open a new issue regarding the deletion of firewalld.conf when running previous: replaced.

BrennanPaciorek commented 1 year ago

AllowZoneDrifting is now able to be disabled by calling the role with the following variables

firewall:
  - firewalld_conf:
      allow_zone_drifting: false

Regarding the deletion of comments when using previous: replaced, see the comment below:

The issue described in the title is currently part of a larger issue (firewalld.conf being deleted as a result of previous: replaced), so we'll close this issue and open a new issue regarding the deletion of firewalld.conf when running previous: replaced.

Thank you for submitting this issue.

richm commented 1 year ago

Sure, we can close this issue.

The issue described in the title is currently part of a larger issue (firewalld.conf being deleted as a result of previous: replaced), so we'll close this issue and open a new issue regarding the deletion of firewalld.conf when running previous: replaced.

That has been mitigated somewhat by https://github.com/linux-system-roles/firewall/pull/163 - the way it works now is if firewalld.conf is the same as provided by the rpm package (verified by rpm -V), then it is not removed when using previous: replaced