Cannot insert IPV6 rule before IPV4 rules

Bug #1368411 reported by babipanghang on 2014-09-11
56
This bug affects 10 people
Affects Status Importance Assigned to Milestone
ufw
Medium
Jamie Strandboge
ufw (Debian)
Fix Released
Unknown
ufw (Ubuntu)
Status tracked in Disco
Bionic
Medium
Jamie Strandboge
Cosmic
Medium
Jamie Strandboge
Disco
Medium
Jamie Strandboge

Bug Description

[Impact]

ufw's 'insert' command is designed to work with 'ufw status numbered' to insert rules in specific places in the ruleset. This makes it more difficult than it should be for using ufw as part of an IPS/dynamic firewall (eg, fail2ban) since if the firewall already has an IPv4 rule then the user/IPS must calculate the position of an IPv6-only rule before inserting it.

From the git commit:

"
add 'prepend' command

Introduce 'prepend' command to add rules to the top of the IPv4 and/or
IPv6 chains. This is particularly useful for dynamic firewalls/IPS (eg,
fail2ban). Unlike 'insert', 'prepend' does not require knowledge about
the IPv6 rule number so integration into IPS is much easier.
"

[Test Case]

$ sudo ufw allow 22/tcp
$ sudo ufw allow from 1.2.3.4
$ sudo ufw allow from 2001:db8::/32
$ sudo ufw enable
$ sudo ufw status numbered
...
[ 1] 22/tcp ALLOW IN Anywhere
[ 2] Anywhere ALLOW IN 1.2.3.4
[ 3] 22/tcp (v6) ALLOW IN Anywhere (v6)
[ 4] Anywhere (v6) ALLOW IN 2001:db8::/32

# unchanged from 0.35
$ sudo ufw insert 1 deny from 2a02:2210:12:a:b820:fff:fea2:25d1
ERROR: Invalid position '1'

# new in 0.36
$ sudo ufw prepend deny from 2a02:2210:12:a:b820:fff:fea2:25d1
$ sudo ufw prepend deny from 6.7.8.9
$ sudo ufw status numbered
...
[ 1] Anywhere DENY IN 6.7.8.9
[ 2] 22/tcp ALLOW IN Anywhere
[ 3] Anywhere ALLOW IN 1.2.3.4
[ 4] Anywhere (v6) DENY IN 2a02:2210:12:a:b820:fff:fea2:25d1
[ 5] 22/tcp (v6) ALLOW IN Anywhere (v6)
[ 6] Anywhere (v6) ALLOW IN 2001:db8::/32

[Regression Potential]

ufw has a clean methodology for adding new commands so while frontend.py necessarily has some logic changes to calculate where to insert the rule (ie, if IPv4 at the top, if IPv6 before other IPv6 rules and if both, both), the changes were minimal and only are used if 'prepend' is specified (so people only using the previous command set should be fine).

[Other Info]

The ufw prepend command is new in 0.36 and thus only available in Debian, Ubuntu disco and the ufw snap for a few weeks. The snap is known to work with fail2ban and the prepend command in production environments since it was available.

= Original description =

I am unable to insert any rules concerning IPV6 before IPV4 rules. Thus, when IPV4 rules are numbered 1 to 5 and IPV6 rules are numbered 6 to 10, the following command:
[code]
ufw insert 1 deny from 2a02:2210:12:a:b820:fff:fea2:25d1
[/code]
errors with "ERROR: Invalid position '1'".

However, the command
[code]
ufw insert 6 deny from 2a02:2210:12:a:b820:fff:fea2:25d1
[/code]
succeeds.

In my case, this poses a problem, since I am trying to insert rules from a script against brute force attacks. The script needs to insert blocking rules before a number of other rules that open up some ports (since the order of rules is important in ufw). However since the number of IPV4 rules will be changing all the time, the position of the first available number for an IPV6 address is hard to determine.

Proposed solution: either allow IPV6 rules to precede IPV4 rules, or implement a keyword defining the first available position; e.g. "ufw insert first deny from 2a02:2210:12:a:b820:fff:fea2:25d1".

BTW: this was all figured out with ufw version 0.31.1-1, Ubuntu 12.04.5 LTS,

Changed in ufw (Ubuntu):
status: New → Confirmed
importance: Undecided → Medium
Changed in ufw (Ubuntu):
status: Confirmed → Triaged
Frank (palvoelgyi) wrote :

This is my solution.

--- ufw-0.34/src/frontend.py 2015-08-20 20:10:26.000000000 +0200
+++ ufw-0.34/src/frontend.py 2015-12-21 09:46:25.311587993 +0100
@@ -451,7 +451,7 @@ class UFWFrontend:
                     elif ip_version == "v6":
                         if r.position > num_v4:
                             r.set_position(r.position - num_v4)
- elif r.position != 0 and r.position <= num_v4:
+ elif r.position != 0 and r.position > num_v4+num_v6:
                             pos_err_msg += str(r.position) + "'"
                             raise UFWError(pos_err_msg)
                         r.set_v6(True)

Jochen Fahrner (jofa) wrote :

Hi Frank, your patch did not work for me. I did it this way:

--- frontend.bak 2016-03-01 16:21:22.000000000 +0100
+++ frontend.py 2016-03-01 16:26:23.000000000 +0100
@@ -403,6 +403,8 @@
                         r.set_v6(False)
                         tmp = self.backend.set_rule(r)
                     elif ip_version == "v6":
+ if r.position != 0 and r.position <= num_v4:
+ r.position = num_v4 + 1
                         if r.position > num_v4:
                             r.set_position(r.position - num_v4)
                         elif r.position != 0 and r.position <= num_v4:

Jochen Fahrner (jofa) on 2016-03-01
Changed in ufw (Ubuntu):
status: Triaged → Confirmed
Toni Lähdekorpi (lahdekorpi) wrote :

How is this still an issue?

I tried the patch with 0.35-0ubuntu2 in xenial:

+ if r.position != 0 and r.position <= num_v4:
+ r.position = num_v4 + 1

And it works perfectly. Couldn't this be merged as there is no obvious downside on recalculating the rule position for IPv6 addresses?

Justin Coffman (jcoffman) wrote :

Given the growing prevalence of IPv6 connectivity, this is a huge problem for this package. This bug has been open for nearly three years. Why is this still here?

Justin Coffman (jcoffman) wrote :

Over three years, actually. Coming up on four. Wow.

dino99 (9d9) wrote :
tags: added: artful bionic trusty upgrade-software-version xenial
dino99 (9d9) wrote :

That ppa can be helpful to get feedback

ppa:jdstrand/ufw-daily

tags: removed: upgrade-software-version
Richard Laager (rlaager) wrote :

Taking into account the two proposed patches, and what I believe the code to be doing, attached is a patch I believe is suitable for inclusion.

The attachment "0005-lp1368411.patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

tags: added: patch
Richard Laager (rlaager) wrote :

Attached is an updated version of the patch that builds. The previous one was failing because there's a test case that makes sure an "insert 2" of an IPv6 rule fails. That's enforcing the existence of the behavior that here we are arguing is a bug.

Changed in ufw (Debian):
status: Unknown → New
Changed in ufw (Debian):
status: New → Fix Released
Changed in ufw (Debian):
status: Fix Released → New
Changed in ufw:
status: New → In Progress
assignee: nobody → Jamie Strandboge (jdstrand)
Jamie Strandboge (jdstrand) wrote :

Thanks for all the feedback! FYI, since '1' in ufw corresponds to the literal rule number '1', this is going to be implemented with a new 'prepend' command. Eg:

$ sudo ufw allow 22/tcp
$ sudo ufw allow from 1.2.3.4
$ sudo ufw allow from 2001:db8::/32
$ sudo ufw status numbered
...
[1] 22/tcp ALLOW IN Anywhere
[2] Anywhere ALLOW IN 1.2.3.4
[3] 22/tcp (v6) ALLOW IN Anywhere (v6)
[4] Anywhere (v6) ALLOW IN 2001:db8::/32

$ sudo ufw prepend deny from 2a02:2210:12:a:b820:fff:fea2:25d1
$ sudo ufw prepend deny from 6.7.8.9
$ sudo ufw status numbered
...
[1] Anywhere DENY IN 6.7.8.9
[2] 22/tcp ALLOW IN Anywhere
[3] Anywhere ALLOW IN 1.2.3.4
[4] Anywhere (v6) DENY IN 2a02:2210:12:a:b820:fff:fea2:25d1
[5] 22/tcp (v6) ALLOW IN Anywhere (v6)
[6] Anywhere (v6) ALLOW IN 2001:db8::/32

Changed in ufw:
status: In Progress → Fix Committed
Changed in ufw (Ubuntu):
status: Confirmed → Triaged
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in ufw (Ubuntu Bionic):
status: New → Triaged
Changed in ufw (Ubuntu Cosmic):
status: New → Triaged
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in ufw (Ubuntu Bionic):
assignee: nobody → Jamie Strandboge (jdstrand)
Changed in ufw (Ubuntu Cosmic):
importance: Undecided → Medium
Changed in ufw (Ubuntu Bionic):
importance: Undecided → Medium
Changed in ufw:
importance: Undecided → Low
importance: Low → Medium
Changed in ufw (Ubuntu Disco):
status: Triaged → In Progress
Jamie Strandboge (jdstrand) wrote :

This is fixed in the new 0.36 release.

Changed in ufw:
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :

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

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

  * New upstream release (LP: #1782384, LP: #1664133, LP: #1509725,
    LP: #1695718, LP: #1719211, LP: #1775043, LP: #1204579, LP: #1652163,
    LP: #1377600, Closes: 686248, LP: #1368411, LP: #1586258, Closes: 909163,
    Closes: 884932, LP: #1558068)
    - drop 0002-bug849628.patch (included upstream)
    - drop 0003-use-default-tcp-syncookies.patch (included upstream)
    - drop 0004-lp1633698.patch (included upstream)
  * Remaining changes:
    - 0001-optimize-boot.patch
  * debian/ufw.maintscript: remove /etc/bash_completion.d/ufw on upgrade
    (LP: #1602834)
  * debian/control: remove no longer needed xs-python-version and
    x-python3-version fields
  * update debian/before6.rules.md5sum for file shipped in 0.35-6. While both
    before.rules and before6.rules were updated in this new upstream release,
    0.35-6 mistakenly already had its own md5sum for before.rules, so we don't
    need to add it now.

 -- Jamie Strandboge <email address hidden> Fri, 14 Dec 2018 17:50:47 +0000

Changed in ufw (Ubuntu Disco):
status: In Progress → Fix Released
Changed in ufw (Debian):
status: New → Fix Released
description: updated
Changed in ufw (Ubuntu Bionic):
status: Triaged → In Progress
Changed in ufw (Ubuntu Cosmic):
status: Triaged → In Progress

An upload of ufw to cosmic-proposed has been rejected from the upload queue for the following reason: "All bugs mentioned in the .changes file (so therefore also in the new debian/changelog entries) need to comply with SRU standards (test-case, regression potential). Please re-upload after filling out the required info or modify changelog to exclude irrelevant bug numbers.".

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.