Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ansible.posix.firewalld permanent rule issue. #451

Closed
ysakkk opened this issue Apr 18, 2023 · 5 comments · Fixed by #454
Closed

ansible.posix.firewalld permanent rule issue. #451

ysakkk opened this issue Apr 18, 2023 · 5 comments · Fixed by #454

Comments

@ysakkk
Copy link

ysakkk commented Apr 18, 2023

When I add a "permanent" rule in the following environment, all tcp communication permission rules are added with no change in the execution result.

OS: Rocky Linux release 9.1 (Blue Onyx)
ansible: ansible-core-2.13.3-2.el9_1.x86_64

* requirements.yml
collections:
  - name: community.general
  - name: ansible.posix

* install 
$ ansible-galaxy  install -r requirements.yml

* roles/firewall/tasks/main.yml
- name: "allow /etc/firewalld/zones/public.xml"
  ansible.posix.firewalld:
    port: "1111/tcp"
    permanent: true    ### here
    state: enabled 

$ ansible-playbook firewall.yml
localhost                  : ok=2    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0  


# diff  /etc/firewalld/zones/public.xml{,.old}
10d9
<   <protocol value="tcp"/>
@rekup
Copy link
Contributor

rekup commented May 3, 2023

This is a huge issue! The line <protocol value="tcp"/> in the zone file results in all tcp connections being accepted. This is reproducable using a simple task:

    - name: open ports
      ansible.posix.firewalld:
        port: "25/tcp"
        permanent: yes
        immediate: yes
        state: enabled
        zone: "public"

Afterwards test using nc:

on the server:

[user@server]# nc -l 1234

On the client:

echo "this is a huge problem" | nc <ip> 1234

Back on the server:

[user@server]# nc -l 1234
this is a huge problem

This potentially leaves many servers exposed.

CC @felixfontein, @maxamillion

@nerrehmit
Copy link
Contributor

To expand on the above message, I have identified the problematic commit during testing.

It is ee9df94 where apparently a protocol gets added to the config without the restriction to a single port.

Using the above example by @rekup the file /etc/firewalld/zones/public.xml goes from

<zone>
  <short>Public</short>
  <description>For use in public areas. You do not trust the other computers on networks to not harm your computer. Only selected incoming connections are accepted.</description>
  <service name="ssh"/>
  <service name="mdns"/>
  <service name="dhcpv6-client"/>
  <forward/>
</zone>

before the ansible run to

<zone>
  <short>Public</short>
  <description>For use in public areas. You do not trust the other computers on networks to not harm your computer. Only selected incoming connections are accepted.</description>
  <service name="ssh"/>
  <service name="mdns"/>
  <service name="dhcpv6-client"/>
  <port port="25" protocol="tcp"/>
  <protocol value="tcp"/>
  <forward/>
</zone>

after the run.

<protocol value="tcp"/> then allows all incoming tcp connections.

What makes this even more problematic, is that this does not show up as a change during the ansible run if the initial port is already open.

@rekup
Copy link
Contributor

rekup commented May 3, 2023

The problem might be that in

if module.params['port'] is not None:
if '/' in module.params['port']:
port, protocol = module.params['port'].strip().split('/')
the variable protocol is used to save the protocol extracted from the port parameter. This is the same variable used by the newly introduced protocol parameter:
if protocol is not None:
transaction = ProtocolTransaction(
module,
action_args=(protocol, timeout),
zone=zone,
desired_state=desired_state,
permanent=permanent,
immediate=immediate,
)
changed, transaction_msgs = transaction.run()
msgs = msgs + transaction_msgs
if changed is True:
msgs.append("Changed protocol %s to %s" % (protocol, desired_state))

softwarefactory-project-zuul bot added a commit that referenced this issue May 4, 2023
fix firewalld protocol

SUMMARY
This PR resolves an issue where opening a port (e.g. 25/tcp) resulted in opening all ports for the specified protocol (e.g. tcp)
Fixes #451
ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME
ansible.posix.firewalld
ADDITIONAL INFORMATION
Many thanks to @nerrehmit and every one else who helped troubleshooting this!
@nliechti
Copy link

Is there a release planned in the near future? I would strongly suggest to do one as soon as possible. This issue seems very urgent to me, as it de facto disabled the firewall on my systems completely without even showing a change in the playbook run.

@maxamillion
Copy link
Collaborator

release 1.5.4 is out now and has the fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants