saltstack-formulas / ufw-formula

Manages your firewall using ufw with pillar configured rules
Apache License 2.0
7 stars 37 forks source link

[BUG] UFW Deny rule does not create CIDR based filters correctly. #20

Closed r-pufky closed 4 years ago

r-pufky commented 4 years ago

Your setup

Formula commit hash / release tag

5b88c1b / 0.5.3 / 2019-10-19

Versions reports (master & minion)

Minion

$ salt-minion --versions
Salt Version:
           Salt: 3000.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: 2.0.5
      gitpython: 2.1.11
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: 3.6.1
         pygit2: Not Installed
         Python: 3.7.3 (default, Dec 20 2019, 18:57:59)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 17.1.2
          smmap: 2.0.5
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1

System Versions:
           dist: debian 10.4
         locale: UTF-8
        machine: x86_64
        release: 4.19.0-9-amd64
         system: Linux
        version: debian 10.4

Master

$ salt-master --versions
Salt Version:
           Salt: 3000.3

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: 2.0.5
      gitpython: 2.1.11
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: 3.6.1
         pygit2: Not Installed
         Python: 3.7.3 (default, Dec 20 2019, 18:57:59)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 17.1.2
          smmap: 2.0.5
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1

System Versions:
           dist: debian 10.4
         locale: UTF-8
        machine: x86_64
        release: 4.19.0-9-amd64
         system: Linux
        version: debian 10.4

Pillar / config used

ufw:
  enabled: True
  services:
    22:
      protocol:  tcp
      deny:      True
      from_addr: 172.31.255.0/24
      comment:   Block SSH over wireguard.
    ssh:
      protocol: tcp
      comment:  SSH host services.

Bug details

Describe the bug

When applying the denial rule deny: True, the rule is applied but matched for ANY instead of the specified subnet.

$ sudo ufw status
Status: active

To       Action   From
--       -------  -----
22/tcp   DENY     Anywhere      # Block SSH over wireguard.
22/tcp   ALLOW    Anywhere      # SSH Services.
...

This results in all SSH connections being blocked.

Expected behaviour

Using the same configuration from above, the expected results are:

$ sudo ufw status
Status: active

To       Action   From
--       -------  -----
22/tcp   DENY     172.31.255.0/24   # Block SSH over wireguard.
22/tcp   ALLOW    Anywhere          # SSH Services.
...

Which will appropriately deny SSH traffic from 172.31.255.0/24 and allow normal SSH traffic.

This can be independently verified by manually inserting the filter via the CLI, which works:

sudo ufw insert 1 deny proto tcp from 172.31.255.0/24 to any port 22 comment Block SSH over wireguard

Attempts to fix the bug

I've dug into the code, and think I've identified the problem line. wherein the from_addr is somehow being interpreted as None, and any is being returned as one of the command tokens instead of 172.31.255.0/24.

myii commented 4 years ago

Appreciate the detailed report, @r-pufky. I'll try to have a look at this within the next day, if someone else doesn't get there first.

r-pufky commented 4 years ago

It looks like this can be closed. It appears the 3000.2->3000.3 upgrade had some wonky pillar caching issues. I had to stop master/minion, manually clear caches and reboot for this state to apply correctly.

I've verified on two separate machines now.

myii commented 4 years ago

@r-pufky OK, thanks for confirming that there aren't any problems -- closing this issue.