ruleset: fix chain selection for mark and dscp targets

Align the chain selection logic for mark and dscp targets with the one
implemented in firewall3 with commit https://git.openwrt.org/61db17e

Also add corresponding testcases to assert the correct selection logic.

Signed-off-by: Jo-Philipp Wich <jo@mein.io>

diff --git a/root/usr/share/firewall4/templates/ruleset.uc b/root/usr/share/firewall4/templates/ruleset.uc
index 56d18ec1007bbc830a739f788e03338af5d26892..dadb1fd26de723b5781037bb5c673f134ce89341 100644 (file)
--- a/root/usr/share/firewall4/templates/ruleset.uc
+++ b/root/usr/share/firewall4/templates/ruleset.uc
@@ -360,6 +360,20 @@ table inet fw4 {
 {% endfor %}
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+{% for (let rule in fw4.rules("mangle_postrouting")): %}
+               {%+ include("rule.uc", { fw4, rule }) %}
+{% endfor %}
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+{% for (let rule in fw4.rules("mangle_input")): %}
+               {%+ include("rule.uc", { fw4, rule }) %}
+{% endfor %}
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
 {% for (let rule in fw4.rules("mangle_output")): %}
@@ -369,6 +383,9 @@ table inet fw4 {
 
        chain mangle_forward {
                type filter hook forward priority mangle; policy accept;
+{% for (let rule in fw4.rules("mangle_forward")): %}
+               {%+ include("rule.uc", { fw4, rule }) %}
+{% endfor %}
 {% for (let zone in fw4.zones()): %}
 {%  if (zone.mtu_fix): %}
 {%   for (let rule in zone.match_rules): %}

diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 8859659c7ebc51d815a8f891ef854642f4c76e20..ab6fbdf2f65c713c5b92b784316cda6fca7f6628 100644 (file)
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -1997,10 +1997,6 @@ return {
                        this.warn_section(data, "must specify a source zone for target '" + rule.target + "'");
                        return;
                }
-               else if (rule.target in ["dscp", "mark"] && rule.dest) {
-                       this.warn_section(data, "must not specify option 'dest' for target '" + rule.target + "'");
-                       return;
-               }
                else if (rule.target == "dscp" && !rule.set_dscp) {
                        this.warn_section(data, "must specify option 'set_dscp' for target 'dscp'");
                        return;
@@ -2088,13 +2084,22 @@ return {
                                r.src.zone.dflags.helper = true;
                        }
                        else if (r.target == "mark" || r.target == "dscp") {
-                               if (r.src) {
+                               if ((r.src?.any && r.dest?.any) || (r.src?.zone && r.dest?.zone))
+                                       r.chain = "mangle_forward";
+                               else if (r.src?.any && r.dest?.zone)
+                                       r.chain = "mangle_postrouting";
+                               else if (r.src?.zone && r.dest?.any)
                                        r.chain = "mangle_prerouting";
-                                       r.src.zone.dflags[r.target] = true;
-                               }
-                               else {
+                               else if (r.src && !r.dest)
+                                       r.chain = "mangle_input";
+                               else
                                        r.chain = "mangle_output";
-                               }
+
+                               if (r.src?.zone)
+                                       r.src.zone.dflags[r.target] = true;
+
+                               if (r.dest?.zone)
+                                       r.dest.zone.dflags[r.target] = true;
                        }
                        else {
                                r.chain = "output";

diff --git a/tests/01_configuration/01_ruleset b/tests/01_configuration/01_ruleset
index c1ec09ed4ad181234c8ca834171043c30795fc7b..da33ccebcad0904afc85b69d9b141dd64eed4968 100644 (file)
--- a/tests/01_configuration/01_ruleset
+++ b/tests/01_configuration/01_ruleset
@@ -257,6 +257,14 @@ table inet fw4 {
                type filter hook prerouting priority mangle; policy accept;
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
        }

diff --git a/tests/02_zones/01_policies b/tests/02_zones/01_policies
index fc6a2953af7815076f1857f6e58ccb367f59e48d..7336df53bcbfd53e5087cd3382bf15648ab43c4a 100644 (file)
--- a/tests/02_zones/01_policies
+++ b/tests/02_zones/01_policies
@@ -232,6 +232,14 @@ table inet fw4 {
                type filter hook prerouting priority mangle; policy accept;
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
        }

diff --git a/tests/02_zones/02_masq b/tests/02_zones/02_masq
index 69f37d4c318416b7b24685c53131b9a42ae81c46..02f52cb4ddac38f85e494f8197138545c62c6bf1 100644 (file)
--- a/tests/02_zones/02_masq
+++ b/tests/02_zones/02_masq
@@ -240,6 +240,14 @@ table inet fw4 {
                type filter hook prerouting priority mangle; policy accept;
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
        }

diff --git a/tests/02_zones/03_masq_src_dest_restrictions b/tests/02_zones/03_masq_src_dest_restrictions
index da1ae91b069459e5ba09739559548fa3b35ac1f4..27208afb947c5d4dcbe10de319dbf2359872eda8 100644 (file)
--- a/tests/02_zones/03_masq_src_dest_restrictions
+++ b/tests/02_zones/03_masq_src_dest_restrictions
@@ -236,6 +236,14 @@ table inet fw4 {
                type filter hook prerouting priority mangle; policy accept;
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
        }

diff --git a/tests/03_rules/01_direction b/tests/03_rules/01_direction
index b2079c75dda1155d2bd5787c44834f2c84366144..7c6dd600aeae23978b3815e50597ff5d870a30b2 100644 (file)
--- a/tests/03_rules/01_direction
+++ b/tests/03_rules/01_direction
@@ -137,6 +137,14 @@ table inet fw4 {
                type filter hook prerouting priority mangle; policy accept;
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
        }

diff --git a/tests/03_rules/02_enabled b/tests/03_rules/02_enabled
index 247f23643edec8886042907e4790c2c536b8f23e..d6933c11c88a9feaba3ff18a5c1c36d900851a7e 100644 (file)
--- a/tests/03_rules/02_enabled
+++ b/tests/03_rules/02_enabled
@@ -132,6 +132,14 @@ table inet fw4 {
                type filter hook prerouting priority mangle; policy accept;
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
        }

diff --git a/tests/03_rules/03_constraints b/tests/03_rules/03_constraints
index 8b1e04b4a4cce8ec3a42debee02b5927251f05fa..a8e3f66cccfc446dc6b48e2912362758046c846f 100644 (file)
--- a/tests/03_rules/03_constraints
+++ b/tests/03_rules/03_constraints
@@ -46,31 +46,17 @@ Testing various option constraints.
                        "target": "notrack"
                },
 
-               {
-                       ".description": "DSCP rules must not specify a destination",
-                       "proto": "any",
-                       "name": "DSCP rule #1",
-                       "dest": "*",
-                       "target": "dscp"
-               },
                {
                        ".description": "DSCP rules require a set_dscp option",
                        "proto": "any",
-                       "name": "DSCP rule #2",
+                       "name": "DSCP rule #1",
                        "target": "dscp"
                },
 
-               {
-                       ".description": "Mark rules must not specify a destination",
-                       "proto": "any",
-                       "name": "Mark rule #1",
-                       "dest": "*",
-                       "target": "mark"
-               },
                {
                        ".description": "Mark rules require a set_xmark or set_mark option",
                        "proto": "any",
-                       "name": "Mark rule #2",
+                       "name": "Mark rule #1",
                        "target": "mark"
                },
        ]
@@ -81,10 +67,8 @@ Testing various option constraints.
 [!] Section @rule[0] (Helper rule #1) must specify a source zone for target 'helper'
 [!] Section @rule[1] (Helper rule #2) must specify option 'set_helper' for target 'helper'
 [!] Section @rule[2] (Notrack rule) must specify a source zone for target 'notrack'
-[!] Section @rule[3] (DSCP rule #1) must not specify option 'dest' for target 'dscp'
-[!] Section @rule[4] (DSCP rule #2) must specify option 'set_dscp' for target 'dscp'
-[!] Section @rule[5] (Mark rule #1) must not specify option 'dest' for target 'mark'
-[!] Section @rule[6] (Mark rule #2) must specify option 'set_mark' or 'set_xmark' for target 'mark'
+[!] Section @rule[3] (DSCP rule #1) must specify option 'set_dscp' for target 'dscp'
+[!] Section @rule[4] (Mark rule #1) must specify option 'set_mark' or 'set_xmark' for target 'mark'
 -- End --
 
 -- Expect stdout --
@@ -196,6 +180,14 @@ table inet fw4 {
                type filter hook prerouting priority mangle; policy accept;
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
        }

diff --git a/tests/03_rules/04_icmp b/tests/03_rules/04_icmp
index 834ecc799d8e264d63dcd6c169339a5808a60270..d3c97dbf15eb1fdbe01b0dfc2728a2590d908f8f 100644 (file)
--- a/tests/03_rules/04_icmp
+++ b/tests/03_rules/04_icmp
@@ -144,6 +144,14 @@ table inet fw4 {
                type filter hook prerouting priority mangle; policy accept;
        }
 
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+       }
+
        chain mangle_output {
                type filter hook output priority mangle; policy accept;
        }

diff --git a/tests/03_rules/05_mangle b/tests/03_rules/05_mangle
new file mode 100644 (file)
index 0000000..05aed75
--- /dev/null
+++ b/tests/03_rules/05_mangle
@@ -0,0 +1,308 @@
+Ensure that DSCP and MARK target rules end up in the appropriate chains,
+depending on the src and dest options.
+
+-- Testcase --
+{%
+       include("./root/usr/share/firewall4/main.uc", {
+               getenv: function(varname) {
+                       switch (varname) {
+                       case 'ACTION':
+                               return 'print';
+                       }
+               }
+       })
+%}
+-- End --
+
+-- File uci/helpers.json --
+{}
+-- End --
+
+-- File fs/open~_sys_class_net_eth0_flags.txt --
+0x1103
+-- End --
+
+-- File fs/open~_sys_class_net_eth1_flags.txt --
+0x1103
+-- End --
+
+-- File uci/firewall.json --
+{
+       "zone": [
+               {
+                       "name": "lan",
+                       "device": "eth0"
+               },
+               {
+                       "name": "wan",
+                       "device": "eth1"
+               }
+       ],
+       "rule": [
+               {
+                       ".description": "Source '*' and destination '*' should result in a forward rule",
+                       "name": "Mangle rule #1",
+                       "src": "*",
+                       "dest": "*",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               },
+               {
+                       ".description": "Source zone and destination zone should result in a forward rule",
+                       "name": "Mangle rule #2",
+                       "src": "lan",
+                       "dest": "wan",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               },
+               {
+                       ".description": "Any source zone and specific destination zone should result in a postrouting rule",
+                       "name": "Mangle rule #3",
+                       "src": "*",
+                       "dest": "wan",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               },
+               {
+                       ".description": "Specific source zone and any destination zone should result in a prerouting rule",
+                       "name": "Mangle rule #4",
+                       "src": "lan",
+                       "dest": "*",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               },
+               {
+                       ".description": "Specific source zone and no destination zone should result in an input rule",
+                       "name": "Mangle rule #5",
+                       "src": "lan",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               },
+               {
+                       ".description": "Any source zone and no destination zone should result in an input rule",
+                       "name": "Mangle rule #6",
+                       "src": "*",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               },
+               {
+                       ".description": "No source zone and no destination zone should result in an output rule",
+                       "name": "Mangle rule #7",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               },
+               {
+                       ".description": "No source zone and any destination zone should result in an output rule",
+                       "name": "Mangle rule #8",
+                       "dest": "*",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               },
+               {
+                       ".description": "No source zone and specific destination zone should result in an output rule",
+                       "name": "Mangle rule #9",
+                       "dest": "wan",
+                       "target": "DSCP",
+                       "set_dscp": "1"
+               }
+       ]
+}
+-- End --
+
+-- Expect stdout --
+table inet fw4
+flush table inet fw4
+
+table inet fw4 {
+       #
+       # Set definitions
+       #
+
+
+       #
+       # Defines
+       #
+
+       define lan_devices = { "eth0" }
+       define wan_devices = { "eth1" }
+
+       #
+       # User includes
+       #
+
+       include "/etc/nftables.d/*.nft"
+
+
+       #
+       # Filter rules
+       #
+
+       chain input {
+               type filter hook input priority filter; policy drop;
+
+               iifname "lo" accept comment "!fw4: Accept traffic from loopback"
+
+               ct state established,related accept comment "!fw4: Allow inbound established and related flows"
+               iifname "eth0" jump input_lan comment "!fw4: Handle lan IPv4/IPv6 input traffic"
+               iifname "eth1" jump input_wan comment "!fw4: Handle wan IPv4/IPv6 input traffic"
+       }
+
+       chain forward {
+               type filter hook forward priority filter; policy drop;
+
+               ct state established,related accept comment "!fw4: Allow forwarded established and related flows"
+               iifname "eth0" jump forward_lan comment "!fw4: Handle lan IPv4/IPv6 forward traffic"
+               iifname "eth1" jump forward_wan comment "!fw4: Handle wan IPv4/IPv6 forward traffic"
+       }
+
+       chain output {
+               type filter hook output priority filter; policy drop;
+
+               oifname "lo" accept comment "!fw4: Accept traffic towards loopback"
+
+               ct state established,related accept comment "!fw4: Allow outbound established and related flows"
+               oifname "eth0" jump output_lan comment "!fw4: Handle lan IPv4/IPv6 output traffic"
+               oifname "eth1" jump output_wan comment "!fw4: Handle wan IPv4/IPv6 output traffic"
+       }
+
+       chain handle_reject {
+               meta l4proto tcp reject with tcp reset comment "!fw4: Reject TCP traffic"
+               reject with icmpx type port-unreachable comment "!fw4: Reject any other traffic"
+       }
+
+       chain input_lan {
+               jump drop_from_lan
+       }
+
+       chain output_lan {
+               jump drop_to_lan
+       }
+
+       chain forward_lan {
+               jump drop_to_lan
+       }
+
+       chain drop_from_lan {
+               iifname "eth0" counter drop comment "!fw4: drop lan IPv4/IPv6 traffic"
+       }
+
+       chain drop_to_lan {
+               oifname "eth0" counter drop comment "!fw4: drop lan IPv4/IPv6 traffic"
+       }
+
+       chain input_wan {
+               jump drop_from_wan
+       }
+
+       chain output_wan {
+               jump drop_to_wan
+       }
+
+       chain forward_wan {
+               jump drop_to_wan
+       }
+
+       chain drop_from_wan {
+               iifname "eth1" counter drop comment "!fw4: drop wan IPv4/IPv6 traffic"
+       }
+
+       chain drop_to_wan {
+               oifname "eth1" counter drop comment "!fw4: drop wan IPv4/IPv6 traffic"
+       }
+
+
+       #
+       # NAT rules
+       #
+
+       chain dstnat {
+               type nat hook prerouting priority dstnat; policy accept;
+       }
+
+       chain srcnat {
+               type nat hook postrouting priority srcnat; policy accept;
+       }
+
+
+       #
+       # Raw rules (notrack & helper)
+       #
+
+       chain raw_prerouting {
+               type filter hook prerouting priority raw; policy accept;
+               iifname "eth0" jump helper_lan comment "!fw4: lan IPv4/IPv6 CT helper assignment"
+               iifname "eth1" jump helper_wan comment "!fw4: wan IPv4/IPv6 CT helper assignment"
+       }
+
+       chain raw_output {
+               type filter hook output priority raw; policy accept;
+       }
+
+       chain helper_lan {
+       }
+
+       chain helper_wan {
+       }
+
+
+       #
+       # Mangle rules
+       #
+
+       chain mangle_prerouting {
+               type filter hook prerouting priority mangle; policy accept;
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #4"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #4"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #4"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #4"
+       }
+
+       chain mangle_postrouting {
+               type filter hook postrouting priority mangle; policy accept;
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #3"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #3"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #3"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #3"
+       }
+
+       chain mangle_input {
+               type filter hook input priority mangle; policy accept;
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #5"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #5"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #5"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #5"
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #6"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #6"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #6"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #6"
+       }
+
+       chain mangle_output {
+               type filter hook output priority mangle; policy accept;
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #7"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #7"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #7"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #7"
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #8"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #8"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #8"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #8"
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #9"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #9"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #9"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #9"
+       }
+
+       chain mangle_forward {
+               type filter hook forward priority mangle; policy accept;
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #1"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #1"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #1"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #1"
+               meta nfproto ipv4 meta l4proto tcp counter ip dscp set 0x1 comment "!fw4: Mangle rule #2"
+               meta nfproto ipv6 meta l4proto tcp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #2"
+               meta nfproto ipv4 meta l4proto udp counter ip dscp set 0x1 comment "!fw4: Mangle rule #2"
+               meta nfproto ipv6 meta l4proto udp counter ip6 dscp set 0x1 comment "!fw4: Mangle rule #2"
+       }
+}
+-- End --