ufw

ufw delete can confuse protocol-specific rule with otherwise matching 'proto any' rule

Bug #1933117 reported by Derek Ramsey
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ufw
High
Jamie Strandboge
ufw (Ubuntu)
Undecided
Jamie Strandboge
Bionic
Medium
Mauricio Faria de Oliveira
Focal
Medium
Mauricio Faria de Oliveira
Hirsute
Medium
Mauricio Faria de Oliveira

Bug Description

[Impact]

 * The deletion of a rule without the 'proto' field
   that has a similar rule *with* the 'proto' field
   might delete the wrong rule (the latter).

 * This might cause services to be inaccessible or
   incorrectly allowed, depending on rule ordering
   (read original description below for examples.)

[Test Steps]

 * Add rules:
   ufw allow from 1.1.1.1 port 1111 proto tcp
   ufw allow from 2.2.2.2 port 2222 proto tcp
   ufw allow from 1.1.1.1 port 1111

 * Check iptables:
   iptables -L ufw-user-input -n
   ...
   ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111
   ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222
   ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111
   ACCEPT udp -- 1.1.1.1 0.0.0.0/0 udp spt:1111

 * Delete the third rule above
   ufw status numbered
   yes | ufw delete 3

 * Check iptables again:
   iptables -L ufw-user-input -n

   Observed: (deleted tcp line from first rule, and udp line from third rule)
   ...
   ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222
   ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111

   Expected: (deleted both tcp and udp lines from third rule)
   ...
   ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111
   ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222

[Regression Potential]

 * Potential regressions would be observed when deleting rules.

 * This fix has been suggested for SRU by jdstrand [1],
   and has already been available in 21.04 and the snap.

 [1] https://code.launchpad.net/~mfo/ufw/+git/ufw/+merge/410091/comments/1083005

[Original Description]

UFW versions 0.35 (on Ubuntu 16.04 LTS) and 0.36 (on Ubuntu 20.04 LTS)

If a rule is inserted without specifying the protocol, it will default to both udp and tcp. If a second rule is inserted earlier in the order that specifies the protocol but is otherwise identical, UFW will delete the wrong rule if the first rule is deleted.

This is repeatable with the following script:

ufw insert 1 allow from 1.1.1.1/26 to any port 22
ufw insert 2 allow from 1.2.3.4/26 to any port 22
ufw insert 1 allow from 1.2.3.4/26 to any port 22 proto tcp
iptables -L -n | grep -A 6 "Chain ufw-user-input"
yes | ufw delete 3
iptables -L -n | grep -A 4 "Chain ufw-user-input"

The output is as follows:

Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 1.2.3.0/26 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 1.1.1.0/26 0.0.0.0/0 tcp dpt:22
ACCEPT udp -- 1.1.1.0/26 0.0.0.0/0 udp dpt:22
ACCEPT tcp -- 1.2.3.0/26 0.0.0.0/0 tcp dpt:22
ACCEPT udp -- 1.2.3.0/26 0.0.0.0/0 udp dpt:22

Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 1.1.1.0/26 0.0.0.0/0 tcp dpt:22
ACCEPT udp -- 1.1.1.0/26 0.0.0.0/0 udp dpt:22
ACCEPT tcp -- 1.2.3.0/26 0.0.0.0/0 tcp dpt:22

UFW deleted the first rule for 1.2.3.0 and then the last rule for 1.2.3.0, leaving the wrong rule remaining. Here is the ufw status:

To Action From
-- ------ ----
22/tcp ALLOW 1.2.3.0/26
22 ALLOW 1.1.1.0/26

Mixing ALLOW and REJECT/DENY rules can further result in incorrect behavior due to this incorrect reordering. On port 22, this could render SSH remotely inaccessible.

For example, if one had initially set up the following rule to port 22 (TCP and UDP)...

ufw insert 1 allow from 1.2.3.4 to any port 22

...and later wanted to further restrict it to only TCP, while explicitly rejecting any other port 22 connections...

ufw insert 1 allow from 1.2.3.4 to any port 22 proto tcp
ufw insert 2 reject from any to any port 22
yes | ufw delete 3

...this would result in SSH becoming inaccessible.

Instead if one had the initial configuration...

ufw insert 1 reject from 1.0.0.0/8 to any port 22
ufw insert 2 allow from any to any port 22

...which was later updated to be...

ufw insert 1 reject from 1.0.0.0/8 to any port 22 proto tcp
ufw insert 2 allow from any to any port 22 proto tcp
yes | ufw delete 3

...this would result in 1.0.0.0/8 incorrectly being allowed to access port 22. While this is a contrived scenario, it is possible and reproducible.

A reboot is required to fix the issue, as it reloads the configuration to the expected order.

Revision history for this message
Derek Ramsey (ramman3000) wrote (last edit ):

This bug should probably be upgraded to a security issue, as anyone relying on specific ordering combining explicit ALLOW with explicit REJECT/DROP might find either the wrong ports closed or the wrong ports unexpectedly open.

description: updated
description: updated
description: updated
information type: Public → Private Security
Revision history for this message
Jamie Strandboge (jdstrand) wrote (last edit ):
Download full text (3.4 KiB)

Thank you for filing a bug.

Note that 1.1.1.1/26 and 1.2.3.4/26 are normalized to 1.1.1.0/26 and 1.2.3.0/26, respectively. They don't overlap so they are still different from each other.

I can confirm the bug. In reproducing this with ufw disabled (ie, it will update rules files but not the firewall itself), I see that,

# ufw insert 1 allow from 1.1.1.1/26 to any port 22 # res. rule 2
# ufw insert 2 allow from 1.2.3.4/26 to any port 22 # res. rule 3
# ufw insert 1 allow from 1.2.3.4/26 to any port 22 proto tcp # res. rule 1

# cat /etc/ufw/user.rules
...
### RULES ###

### tuple ### allow tcp 22 0.0.0.0/0 any 1.2.3.0/26 in
-A ufw-user-input -p tcp --dport 22 -s 1.2.3.0/26 -j ACCEPT

### tuple ### allow any 22 0.0.0.0/0 any 1.1.1.0/26 in
-A ufw-user-input -p tcp --dport 22 -s 1.1.1.0/26 -j ACCEPT
-A ufw-user-input -p udp --dport 22 -s 1.1.1.0/26 -j ACCEPT

### tuple ### allow any 22 0.0.0.0/0 any 1.2.3.0/26 in
-A ufw-user-input -p tcp --dport 22 -s 1.2.3.0/26 -j ACCEPT
-A ufw-user-input -p udp --dport 22 -s 1.2.3.0/26 -j ACCEPT
...

This shows the initial insert is correct.

Running 'ufw delete 3', I see:
# sudo ufw delete 3
Deleting:
 allow from 1.2.3.0/26 to any port 22
Proceed with operation (y|n)? y
Rules updated

This show the correct rule was selected.

# cat /etc/ufw/user.rules
...
### RULES ###

### tuple ### allow tcp 22 0.0.0.0/0 any 1.2.3.0/26 in
-A ufw-user-input -p tcp --dport 22 -s 1.2.3.0/26 -j ACCEPT

### tuple ### allow any 22 0.0.0.0/0 any 1.1.1.0/26 in
-A ufw-user-input -p tcp --dport 22 -s 1.1.1.0/26 -j ACCEPT
-A ufw-user-input -p udp --dport 22 -s 1.1.1.0/26 -j ACCEPT
...

This show the correct rule was removed from the file.

Now, after purging ufw from the system and reinstalling it (to ensure none of the changes are there), adding one rule (so inserts work) and enabling, I see:

# ufw insert 1 allow from 1.1.1.1/26 to any port 22
# ufw insert 2 allow from 1.2.3.4/26 to any port 22
# ufw insert 1 allow from 1.2.3.4/26 to any port 22 proto tcp

# iptables -L ufw-user-input -n
Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 1.2.3.0/26 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 1.1.1.0/26 0.0.0.0/0 tcp dpt:22
ACCEPT udp -- 1.1.1.0/26 0.0.0.0/0 udp dpt:22
ACCEPT tcp -- 1.2.3.0/26 0.0.0.0/0 tcp dpt:22
ACCEPT udp -- 1.2.3.0/26 0.0.0.0/0 udp dpt:22

So the rules are in the correct order (but we can see where the problem will come in).

# ufw delete 3
Deleting:
 allow from 1.2.3.0/26 to any port 22
Proceed with operation (y|n)? y
Rule deleted

# iptables -L ufw-user-input -n
Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 1.1.1.0/26 0.0.0.0/0 tcp dpt:22
ACCEPT udp -- 1.1.1.0/26 0.0.0.0/0 udp dpt:22
ACCEPT tcp -- 1.2.3.0/26 0.0.0.0/0 tcp dpt:22

Which demonstrates the problem, while the ufw rules files are correct, ufw removed the first matching tcp and udp rules in the firewal...

Read more...

information type: Private Security → Public Security
Changed in ufw:
importance: Undecided → High
status: New → Triaged
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

ufw has a concept of reloading the chain under certain circumstances. I believe the fix will be to add a check to see if there are any overlapping deletes (eg, for a matching delete rule, count how many rules with protocol 'any' would overlap with a rules with a protocol) and reload the chain if there are.

summary: - ufw deletes entries in wrong order
+ ufw delete can confuse protocol-specific rule with otherwise matching
+ 'proto any' rule
Changed in ufw:
assignee: nobody → Jamie Strandboge (jdstrand)
Revision history for this message
Jamie Strandboge (jdstrand) wrote :
Changed in ufw:
status: Triaged → Fix Committed
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Released 0.36.1 with the fix for this: https://launchpad.net/ufw/0.36/0.36.1

Changed in ufw:
status: Fix Committed → Fix Released
Changed in ufw (Ubuntu):
status: New → In Progress
assignee: nobody → Jamie Strandboge (jdstrand)
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ufw - 0.36.1-1

---------------
ufw (0.36.1-1) unstable; urgency=medium

  * New upstream release (LP: #1808463, LP: #1831186, LP: #1838764,
    LP: #1556419, LP: #1933117). Drop the following (included upstream):
    - 0002-fix-check-requirements.patch
    - 0003-lp1838764.patch
    - 0004-make-root-tests-chain-order-agnostic.patch
    - 0005-use-only-python3.patch
    - 0006-bug921680.patch
    - 0007-bug921680-pt2.patch
    - 0008-fix-check-requirements-again.patch
    - 0009-empty-non-functioning-ipt-modules.patch
    - 0010-add-other-firewall-checks.patch
    - 0011-suppress-legacy-warnings-in-tests.patch
    - 0012-pyflakes3.patch
    - 0013-add-prepend-to-help.patch
  * Remaining changes:
    - 0001-optimize-boot.patch
    - python3-versions.diff

  [ Jamie Strandboge ]
  * ufw.lintian-overrides:
    - remove init.d-script-possible-missing-stop override
    - adjust "allow to" override
    - override spare-manual-page for ufw-framework as it gives additional
      details for the ufw command
  * 0002-fix-copyright.patch: src/ufw: update copyright year
  * debian/*: use my <email address hidden> email address

  [ Debian Janitor ]
  * Use secure copyright file specification URI.
  * Use set -e rather than passing -e on the shebang-line.
  * Set upstream metadata fields: Repository, Repository-Browse.
  * Update watch file format version to 4.

  [ Bastian Triller ]
  * debian/ufw.logrotate: use rsyslog-rotate instead of invoke-rc.d
    (Closes: 993525)

 -- Jamie Strandboge <email address hidden> Sun, 19 Sep 2021 05:46:12 +0000

Changed in ufw (Ubuntu):
status: In Progress → Fix Released
tags: added: seg sts-sponsor-mfo
Changed in ufw (Ubuntu Bionic):
status: New → In Progress
importance: Undecided → Medium
assignee: nobody → Mauricio Faria de Oliveira (mfo)
Changed in ufw (Ubuntu Focal):
status: New → In Progress
importance: Undecided → Medium
assignee: nobody → Mauricio Faria de Oliveira (mfo)
Changed in ufw (Ubuntu Hirsute):
status: New → In Progress
importance: Undecided → Medium
assignee: nobody → Mauricio Faria de Oliveira (mfo)
description: updated
Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Verified test packages (ppa:mfo/lp1946804) for
the Hirsute, Focal, and Bionic releases.

...

Without the patch:

Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222
ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111

With the patch:

Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111
ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222

...

Versions tested (original/without patch)
I: Version: 0.36.1-1
H: Version: 0.36-7.1
F: Version: 0.36-6
B: Version: 0.36-0ubuntu0.18.04.1

Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Uploaded to H/F/B.
(debdiffs in bug 1946804)

Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Derek, or anyone else affected,

Accepted ufw into hirsute-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ufw/0.36-7.1ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-hirsute to verification-done-hirsute. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-hirsute. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in ufw (Ubuntu Hirsute):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-hirsute
Changed in ufw (Ubuntu Focal):
status: In Progress → Fix Committed
tags: added: verification-needed-focal
Revision history for this message
Brian Murray (brian-murray) wrote :

Hello Derek, or anyone else affected,

Accepted ufw into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ufw/0.36-6ubuntu1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in ufw (Ubuntu Bionic):
status: In Progress → Fix Committed
tags: added: verification-needed-bionic
Revision history for this message
Brian Murray (brian-murray) wrote :

Hello Derek, or anyone else affected,

Accepted ufw into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ufw/0.36-0ubuntu0.18.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Revision history for this message
Jamie Strandboge (jdstrand) wrote (last edit ):

Tested 0.36-6ubuntu1 on focal. apt upgrade succeeded and after reboot the firewall came up with the expected rules in the expected order and I spot-checked allowed and deny traffic. I was able to verify the this bug is fixed via the test steps.

tags: added: verification-done-focal
removed: verification-needed-focal
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Tested 0.36-0ubuntu0.18.04.2 on bionic. apt upgrade succeeded and after reboot the firewall came up with the expected rules in the expected order and I spot-checked allowed and deny traffic. I was able to verify the this bug is fixed via the test steps.

tags: added: verification-done-bionic
removed: verification-needed-bionic
Revision history for this message
Mauricio Faria de Oliveira (mfo) wrote :

Tested with Bionic, Focal, and Hirsute, with the test steps provided. All good.

Bionic:
---

Before:

# dpkg -s ufw | grep Version:
Version: 0.36-0ubuntu0.18.04.1

# iptables -L ufw-user-input -n
Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222
ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111

After:

# dpkg -s ufw | grep Version:
Version: 0.36-0ubuntu0.18.04.2

# iptables -L ufw-user-input -n
Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111
ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222

Focal:
---

Before:

# dpkg -s ufw | grep Version:
Version: 0.36-6

# iptables -L ufw-user-input -n
Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222
ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111

After:

# dpkg -s ufw | grep Version:
Version: 0.36-6ubuntu1

# iptables -L ufw-user-input -n
Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111
ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222

Hirsute:
---

Before:

# dpkg -s ufw | grep Version:
Version: 0.36-7.1

Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222
ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111

After:

# dpkg -s ufw | grep Version:
Version: 0.36-7.1ubuntu1

Chain ufw-user-input (1 references)
target prot opt source destination
ACCEPT tcp -- 0.0.0.0/0 0.0.0.0/0 tcp dpt:22
ACCEPT tcp -- 1.1.1.1 0.0.0.0/0 tcp spt:1111
ACCEPT tcp -- 2.2.2.2 0.0.0.0/0 tcp spt:2222

tags: added: verification-done verification-done-hirsute
removed: verification-needed verification-needed-hirsute
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ufw - 0.36-7.1ubuntu1

---------------
ufw (0.36-7.1ubuntu1) hirsute; urgency=medium

  * d/p/0015-set-default-policy-after-load.patch: fix boot stall on
    iscsi/network root filesystem when starting ufw (LP: #1946804)
  * d/p/0016-unconditionally-reload-with-delete.patch: fix corner
    case of rule deletion with specific/any proto (LP: #1933117)

 -- Mauricio Faria de Oliveira <email address hidden> Mon, 25 Oct 2021 17:58:58 -0300

Changed in ufw (Ubuntu Hirsute):
status: Fix Committed → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Update Released

The verification of the Stable Release Update for ufw has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ufw - 0.36-6ubuntu1

---------------
ufw (0.36-6ubuntu1) focal; urgency=medium

  * d/p/0012-set-default-policy-after-load.patch: fix boot stall on
    iscsi/network root filesystem when starting ufw (LP: #1946804)
  * d/p/0013-unconditionally-reload-with-delete.patch: fix corner case
    of rule deletion with specific/any proto (LP: #1933117)

 -- Mauricio Faria de Oliveira <email address hidden> Mon, 25 Oct 2021 14:30:14 -0300

Changed in ufw (Ubuntu Focal):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ufw - 0.36-0ubuntu0.18.04.2

---------------
ufw (0.36-0ubuntu0.18.04.2) bionic; urgency=medium

  * d/p/0002-set-default-policy-after-load.patch: fix boot stall on
    iscsi/network root filesystem when starting ufw (LP: #1946804)
  * d/p/0003-unconditionally-reload-with-delete.patch: fix corner case
    of rule deletion with specific/any proto (LP: #1933117)

 -- Mauricio Faria de Oliveira <email address hidden> Mon, 25 Oct 2021 14:30:24 -0300

Changed in ufw (Ubuntu Bionic):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers