From 5f61dbfe219ac5f7ad7e5e04748357d0ebb3debc Mon Sep 17 00:00:00 2001 From: Jo-Philipp Wich Date: Sat, 22 Jan 2022 20:36:29 +0100 Subject: [PATCH] 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 --- root/usr/share/firewall4/templates/ruleset.uc | 17 + root/usr/share/ucode/fw4.uc | 23 +- tests/01_configuration/01_ruleset | 8 + tests/02_zones/01_policies | 8 + tests/02_zones/02_masq | 8 + tests/02_zones/03_masq_src_dest_restrictions | 8 + tests/03_rules/01_direction | 8 + tests/03_rules/02_enabled | 8 + tests/03_rules/03_constraints | 32 +- tests/03_rules/04_icmp | 8 + tests/03_rules/05_mangle | 308 ++++++++++++++++++ 11 files changed, 407 insertions(+), 29 deletions(-) create mode 100644 tests/03_rules/05_mangle diff --git a/root/usr/share/firewall4/templates/ruleset.uc b/root/usr/share/firewall4/templates/ruleset.uc index 56d18ec..dadb1fd 100644 --- 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 8859659..ab6fbdf 100644 --- 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 c1ec09e..da33cce 100644 --- 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 fc6a295..7336df5 100644 --- 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 69f37d4..02f52cb 100644 --- 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 da1ae91..27208af 100644 --- 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 b2079c7..7c6dd60 100644 --- 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 247f236..d6933c1 100644 --- 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 8b1e04b..a8e3f66 100644 --- 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 834ecc7..d3c97db 100644 --- 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 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 -- -- 2.30.2