Firewall logging broken 22.03/nftables

Installing and Using OpenWrt
1

Hi,

I'm curious about what you think. I believe the logging of dropped/rejected incoming packets other than the WAN interface doesn't work.

All my interfaces have this option

option log '1'

All reject log entries originate from WAN. Yet I know there where other rejects happening, too.
When I look at nftables there is only a chain called reject_from_WAN but there is no reject_from_MGMT for example for my MGMT interface.

This is a screenshot from my MGMT Firewall zone setting:

grafik
grafik887×604 58.7 KB

Regards
Junicast

2

For the creation of that chain the INPUT policy for the MGMT zone must be set to REJECT, or there must be at least one traffic rule with src MGMT -> dest Device and target/action set to REJECT.

The logging rules will not be created until you attach an interface to the zone.

3

I'm especially interested in logging rejected forwarding packets rather than input. That's why my input is set to Allow and forward to Reject.
For testing I configured a device in "Covered devices". The resulting nftables did not change though.

4

The log option does not really apply to inter-zone forwarded traffic due to the nature of the chain setup.

Log rules are only hit for:

  • dropped/rejected input traffic on a zone
  • dropped/rejected forward traffic among interfaces of the same zone (in the majority of cases, zones only have one interface)
  • dropped/rejected output traffic on a zone

Traffic from one zone to another which does not match any of the specific whitelistings will end up in the generic handle_reject chain or the drop policy of chain forward. The per-zone log options do not really apply there since those locations are not zone specific.

I guess the ruleset generation could be extended to add final log rules to forward_$zone chains in case the global forwarding policy is either reject or drop but this is not the case right now (and was not supported in fw3 either).

Edit: Here's a lightly tested patch that should implement the behavior you were expecting:

diff --git a/root/usr/share/firewall4/templates/ruleset.uc b/root/usr/share/firewall4/templates/ruleset.uc
index eaa1f04..5f482ec 100644
--- a/root/usr/share/firewall4/templates/ruleset.uc
+++ b/root/usr/share/firewall4/templates/ruleset.uc
@@ -240,6 +240,9 @@ table inet fw4 {
 {%  endif %}
 {%  fw4.includes('chain-append', `forward_${zone.name}`) %}
                jump {{ zone.forward }}_to_{{ zone.name }}
+{%  if (fw4.forward_policy() != "accept" && (zone.log & 1)): %}
+               log prefix "{{ fw4.forward_policy() }} {{ zone.name }} forward: "
+{%  endif %}
        }
 
 {%  if (zone.dflags.helper): %}

You should be able to add the change locally to /usr/share/firewall4/templates/ruleset.uc on your system. Make sure to make a backup of the original file before editing it.

2 Likes
5

Is this a ruby template? This looks similar to puppet erb templates.
It seems the position is wrong, I'm getting this error:

/dev/stdin:488:16-18: Error: syntax error, unexpected log
               log prefix "reject KIDS forward: "
               ^^^
The rendered ruleset contains errors, not doing firewall restart.
6

No, not ruby. But the template tag syntax is inspired by Jinja.

Yeah, you likely added the modification at the wrong spot, at least it works locally here. I added the complete modified file as Github gist here, so you can try replacing the entire file:

https://gist.githubusercontent.com/jow-/add2adfe256af77f8c805035c26ac797/raw/7594446dc5e0778d2b7c8e7a64049a2ed7e4a649/ruleset.uc

Md5sum: 64b17e707073bbd9f2a375cc44a146b1
Sha256sum: 0bafc09df9f0311230cdd22dc2f2f090ce1a5b821651517508caffd4b1cf1747

1 Like
7

I somehow messed up the brackets. Of course it was Ninja2 from Ansible it reminded me of, not Puppet lol.

This modification is rather simple. I'm wondering if integration of this in LUCI is also that easy. This would be a nice feature to add I think.

1 Like
8

@jow A different logging issue is the re-use of the name as the log prefix. Without a trailing space in the log prefix text, the name runs into the rest of the log message.

Before (rule name "DNS Intercept"):

Sun Sep 25 17:32:19 2022 kern.warn kernel: [66718.246809] DNS InterceptIN=eth0 OUT= MAC=...

I would suggest a patch similar to this:

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

After:

Sun Sep 25 18:45:25 2022 kern.warn kernel: [71104.062274] DNS Intercept: IN=eth0 OUT= MAC=
1 Like
9

Thanks for the heads up, this makes sense and I'll add it.

2 Likes