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>

2 files changed, 243 insertions, 2 deletions

diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 29ae053..2a1e397 100644
--- 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
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 --