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

Order rules in traffic shaping not honored #6203

Closed
cotosso opened this issue Jun 12, 2020 · 5 comments
Closed

Order rules in traffic shaping not honored #6203

cotosso opened this issue Jun 12, 2020 · 5 comments
Assignees
Labels
verified All test cases were verified successfully

Comments

@cotosso
Copy link

cotosso commented Jun 12, 2020

When you create multiple rules in traffic shaping they are inserted in the tcpost chain (mangle) in reverse order.
It doesn't affect rules if they are non overlapping, but if they overlap (e.g. same source, destination, protocol) the first rule is the one that will be applied instead of the last one as it should be (same behaviour of policy routing).

Steps to reproduce

Create 2 or more rules in traffic shaping section

Expected behavior

In the tcpost chain we can see same rules in same order

Actual behavior

Rules are written in the tcpost chain inverted respect to what is in the cockpit gui

prio1

prio2

Components
NethServer release 7.8.2003 (final)
nethserver-firewall-base-3.9.3-1.ns7.noarch
nethserver-firewall-base-ui-3.9.3-1.ns7.noarch

@cotosso cotosso added the bug A defect of the software label Jun 12, 2020
@gsanchietti gsanchietti removed the bug A defect of the software label Jun 15, 2020
@gsanchietti
Copy link
Member

gsanchietti commented Jun 15, 2020

Current implementation has been designed in PR 28. The order of the rules shown in the UI reflects the user expectation: first match wins (like normal rules).
To obtain such effect, rules are written in reverse order inside the /etc/shorewall/mangle file.

I agree that current info tip inside the UI could be misleading, so I propose to remove it, just like in the old Server Manager.

You are suggesting to not reverse the rule order before writing the template, but this will highly change the firewall behavior.

Scenario:

  • I want to limit a Windows machine (IP 11.11.11.22) which usually is eager of bandwidth during the update
  • I want to reserve an amount of bandwidth for HTTPS traffic

This is how it's now implemented:
Screenshot from 2020-06-15 10-42-39

And iptables output is:

Chain tcpost (1 references)
 pkts bytes target     prot opt in     out     source               destination         
    0     0 MARK       tcp  --  *      *       11.11.11.0/24        0.0.0.0/0            tcp dpt:443 /* RULE#2 fwservice;https */ MARK xset 0x3/0xff
    0     0 MARK       all  --  *      *       11.11.11.22          0.0.0.0/0            /* RULE#3 any */ MARK xset 0x1/0xff
    0     0 CONNMARK   all  --  *      *       0.0.0.0/0            0.0.0.0/0            mark match ! 0x0/0xffff CONNMARK save mask 0xffff

So the windows machine will correctly get low bandwidth.
If the rules doesn't get reversed, the Windows machine will not be limited and will have also a reserved bandwidth!

I see no bug here, except the label on the UI which can be improved or removed.

gsanchietti added a commit to NethServer/nethserver-firewall-base that referenced this issue Jun 15, 2020
@nethbot
Copy link
Member

nethbot commented Jun 15, 2020

in 7.8.2003/testing:

@gsanchietti gsanchietti changed the title order rules in traffic shaping not honored Order rules in traffic shaping not honored Jun 15, 2020
@cotosso
Copy link
Author

cotosso commented Jun 15, 2020

Test Case

  • Create 2 or more rules in Traffic shaping section
  • look at the iptables mangle table, in the tcpost chain you should see the rules in inverted order
  • install the updated rpm
  • In mangle table - tcpost chain: you should now see rules in the same order they are displayed in the web UI

@cotosso cotosso added the testing Packages are available from testing repositories label Jun 15, 2020
@gsanchietti gsanchietti self-assigned this Jun 15, 2020
@gsanchietti
Copy link
Member

Before update iptables -nvL tcpost -t mangle:

Chain tcpost (1 references)
 pkts bytes target     prot opt in     out     source               destination         
    0     0 MARK       tcp  --  *      *       11.11.11.0/24        0.0.0.0/0            tcp dpt:443 /* RULE#2 fwservice;https */ MARK xset 0x3/0xff
  164 38228 MARK       all  --  *      *       11.11.11.22          0.0.0.0/0            /* RULE#3 any */ MARK xset 0x1/0xff
  164 38228 CONNMARK   all  --  *      *       0.0.0.0/0            0.0.0.0/0            mark match ! 0x0/0xffff CONNMARK save mask 0xffff

After update:

Chain tcpost (1 references)
 pkts bytes target     prot opt in     out     source               destination         
    0     0 MARK       all  --  *      *       11.11.11.22          0.0.0.0/0            /* RULE#3 any */ MARK xset 0x1/0xff
    0     0 MARK       tcp  --  *      *       11.11.11.0/24        0.0.0.0/0            tcp dpt:443 /* RULE#2 fwservice;https */ MARK xset 0x3/0xff
    0     0 CONNMARK   all  --  *      *       0.0.0.0/0            0.0.0.0/0            mark match ! 0x0/0xffff CONNMARK save mask 0xffff

I still do not agree on such change but the new behavior is applied correctly.

@gsanchietti gsanchietti added verified All test cases were verified successfully and removed testing Packages are available from testing repositories labels Jun 15, 2020
@nethbot
Copy link
Member

nethbot commented Jun 15, 2020

in 7.8.2003/updates:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
verified All test cases were verified successfully
Projects
None yet
Development

No branches or pull requests

3 participants