fw4: fix formatting of default log prefix

When using the explicit or implicit rule name as default log prefix, ensure
that is followed by a colon and a space to yield properly formatted firewall
log messages.

Also align the processing logic of `option log` in `config nat` sections with
that in `config rule` and `config redirect`.

Ref: https://forum.openwrt.org/t/x/137182/8
Signed-off-by: Jo-Philipp Wich <jo@mein.io>

diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 29ae0534afbd50612d621db021c1d42602a7e940..2a1e397c2e84c9437319232760c528c36dd3e148 100644 (file)
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -2295,7 +2295,7 @@ return {
 
                switch (this.parse_bool(rule.log)) {
                case true:
-                       rule.log = rule.name;
+                       rule.log = `${rule.name}: `;
                        break;
 
                case false:
@@ -2595,7 +2595,7 @@ return {
 
                switch (this.parse_bool(redir.log)) {
                case true:
-                       redir.log = redir.name;
+                       redir.log = `${redir.name}: `;
                        break;
 
                case false:
@@ -3016,6 +3016,15 @@ return {
                        return;
                }
 
+               switch (this.parse_bool(snat.log)) {
+               case true:
+                       snat.log = `${snat.name}: `;
+                       break;
+
+               case false:
+                       delete snat.log;
+               }
+
                let add_rule = (family, proto, saddrs, daddrs, raddrs, sport, dport, rport, snat) => {
                        let n = {
                                ...snat,

diff --git a/tests/03_rules/11_log b/tests/03_rules/11_log
new file mode 100644 (file)
index 0000000..ecbabe5
--- /dev/null
+++ b/tests/03_rules/11_log
@@ -0,0 +1,232 @@
+Testing that `option log 1` enables rule logging and sets the rule name as
+log prefix. Also testing that setting settin `option log` to a non-boolean
+string uses that string verbatim as log prefix.
+
+-- Testcase --
+{%
+       include("./root/usr/share/firewall4/main.uc", {
+               getenv: function(varname) {
+                       switch (varname) {
+                       case 'ACTION':
+                               return 'print';
+                       }
+               }
+       })
+%}
+-- End --
+
+-- File uci/helpers.json --
+{}
+-- End --
+
+-- File uci/firewall.json --
+{
+       "zone": [
+               {
+                       "name": "wan"
+               }
+       ],
+       "rule": [
+               {
+                       "proto": "any",
+                       "log": "1"
+               },
+               {
+                       "name": "Explicit rule name",
+                       "proto": "any",
+                       "log": "1"
+               },
+               {       "proto": "any",
+                       "log": "Explicit prefix: "
+               }
+       ],
+       "redirect": [
+               {
+                       "proto": "tcp",
+                       "src": "wan",
+                       "dest_ip": "10.0.0.2",
+                       "dest_port": "22",
+                       "log": "1"
+               },
+               {
+                       "name": "Explicit redirect name",
+                       "proto": "tcp",
+                       "src": "wan",
+                       "dest_ip": "10.0.0.3",
+                       "dest_port": "23",
+                       "log": "1"
+               },
+               {
+                       "proto": "tcp",
+                       "src": "wan",
+                       "dest_ip": "10.0.0.4",
+                       "dest_port": "24",
+                       "log": "Explicit prefix: "
+               }
+       ],
+       "nat": [
+               {
+                       "src": "wan",
+                       "target": "MASQUERADE",
+                       "log": "1"
+               },
+               {
+                       "name": "Explicit nat name",
+                       "src": "wan",
+                       "target": "MASQUERADE",
+                       "log": "1"
+               },
+               {
+                       "src": "wan",
+                       "target": "MASQUERADE",
+                       "log": "Explicit log prefix: "
+               }
+       ]
+}
+-- End --
+
+-- Expect stdout --
+table inet fw4
+flush table inet fw4
+
+table inet fw4 {
+       #
+       # Defines
+       #
+
+       define wan_devices = {  }
+       define wan_subnets = {  }
+
+
+       #
+       # 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"
+       }
+
+       chain forward {
+               type filter hook forward priority filter; policy drop;
+
+               ct state established,related accept comment "!fw4: Allow forwarded established and related flows"
+       }
+
+       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"
+               counter log prefix "@rule[0]: " comment "!fw4: @rule[0]"
+               counter log prefix "Explicit rule name: " comment "!fw4: Explicit rule name"
+               counter log prefix "Explicit prefix: " comment "!fw4: @rule[2]"
+       }
+
+       chain prerouting {
+               type filter hook prerouting priority filter; policy accept;
+       }
+
+       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_wan {
+               ct status dnat accept comment "!fw4: Accept port redirections"
+               jump drop_from_wan
+       }
+
+       chain output_wan {
+               jump drop_to_wan
+       }
+
+       chain forward_wan {
+               ct status dnat accept comment "!fw4: Accept port forwards"
+               jump drop_to_wan
+       }
+
+       chain helper_wan {
+       }
+
+       chain drop_from_wan {
+       }
+
+       chain drop_to_wan {
+       }
+
+
+       #
+       # NAT rules
+       #
+
+       chain dstnat {
+               type nat hook prerouting priority dstnat; policy accept;
+       }
+
+       chain srcnat {
+               type nat hook postrouting priority srcnat; policy accept;
+       }
+
+       chain dstnat_wan {
+               meta nfproto ipv4 counter log prefix "@redirect[0]: " dnat 10.0.0.2:22 comment "!fw4: @redirect[0]"
+               meta nfproto ipv4 counter log prefix "Explicit redirect name: " dnat 10.0.0.3:23 comment "!fw4: Explicit redirect name"
+               meta nfproto ipv4 counter log prefix "Explicit prefix: " dnat 10.0.0.4:24 comment "!fw4: @redirect[2]"
+       }
+
+       chain srcnat_wan {
+               meta nfproto ipv4 counter log prefix "@nat[0]: " masquerade comment "!fw4: @nat[0]"
+               meta nfproto ipv4 counter log prefix "Explicit nat name: " masquerade comment "!fw4: Explicit nat name"
+               meta nfproto ipv4 counter log prefix "Explicit log prefix: " masquerade comment "!fw4: @nat[2]"
+       }
+
+
+       #
+       # Raw rules (notrack)
+       #
+
+       chain raw_prerouting {
+               type filter hook prerouting priority raw; policy accept;
+       }
+
+       chain raw_output {
+               type filter hook output priority raw; policy accept;
+       }
+
+
+       #
+       # Mangle rules
+       #
+
+       chain mangle_prerouting {
+               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 route hook output priority mangle; policy accept;
+       }
+
+       chain mangle_forward {
+               type filter hook forward priority mangle; policy accept;
+       }
+}
+-- End --