| Message ID | 20220928164930.626856-1-dsmtngoat@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | fw4: handle bad forward_zone packets with v_from_z | expand |
This comment was posted to "[PATCH] Send bad forward_zone packets to verdict_from_zone". > the forward policy for zones is supposed to only apply to forwarded traffic > among interfaces of the same zone. If I read it correctly, your patch would > change this long standing behavior to something else. In this patch, forwarded traffic for a zone is still handled by chains for interfaces of that zone. For a typical simple home router with a "wan" zone, the current behavior of firewall4 for a forwarded zone is to create these chains, extracted from the output of "fw4 print": chain forward { type filter hook forward priority filter; policy drop; ... accept established, related iifname "wan" jump forward_wan comment "!fw4: Handle wan IPv4/IPv6 forward traffic" jump handle_reject } ... chain forward_wan { ... accept icmpv6, esp, isakmp jump drop_to_wan } ... chain drop_to_wan { oifname "wan" counter log prefix "drop wan out: " drop comment "!fw4: drop wan IPv4/IPv6 traffic" } I believe "jump drop_to_wan" in chain forward_wan is incorrect, it should be "jump drop_from_wan". Chain "drop_to_wan" won't do anything with packets received on "wan" but destined for other zones, because it matches packets with oifname "wan", not iifname "wan". The additional change to fw4.uc ensures that "drop_from_wan" is defined, even if the default policy for forward is not drop: chain drop_from_wan { iifname "wan" counter log prefix "drop wan in: " drop comment "!fw4: drop wan IPv4/IPv6 } Gordon Gordon On Wed, Sep 28, 2022 at 10:50 AM <dsmtngoat@gmail.com> wrote: > > From: Gordon Maclean <dsmtngoat@gmail.com> > > Received forward packets which fail acceptance tests are finally handled > by a <verdict>_to_<zone> chain where <verdict> is typically > "drop" or "reject". This "_to_" chain only matches packets destined > for the interface, and so does not match packets destined for interfaces > other than where they were received. > > The final resolution of these packets will then be the policy for the > forward chain, which for a reasonably configured router is "drop" or > "reject", so this is unlikely to be a security hole. However, > this does not match what the user has configured as the verdict for > forward packets received for the zone. Also, if the user has enabled > logging of failed packets, these packets will not be logged. > > This patch changes the forward vertict to "<verdict>_from_<zone>", > and enables the definition of that chain for received packets. > > This patch may also result in failures in firewall4/tests, which has not > been investigated. > > Signed-off-by: Gordon Maclean <dsmtngoat@gmail.com> > --- > root/usr/share/firewall4/templates/ruleset.uc | 2 +- > root/usr/share/ucode/fw4.uc | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/root/usr/share/firewall4/templates/ruleset.uc b/root/usr/share/firewall4/templates/ruleset.uc > index eaa1f04..daef252 100644 > --- a/root/usr/share/firewall4/templates/ruleset.uc > +++ b/root/usr/share/firewall4/templates/ruleset.uc > @@ -239,7 +239,7 @@ table inet fw4 { > ct status dnat accept comment "!fw4: Accept port forwards" > {% endif %} > {% fw4.includes('chain-append', `forward_${zone.name}`) %} > - jump {{ zone.forward }}_to_{{ zone.name }} > + jump {{ zone.forward }}_from_{{ zone.name }} > } > > {% if (zone.dflags.helper): %} > diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc > index 29ae053..a6c1ae5 100644 > --- a/root/usr/share/ucode/fw4.uc > +++ b/root/usr/share/ucode/fw4.uc > @@ -2113,6 +2113,7 @@ return { > > zone.sflags = {}; > zone.sflags[zone.input] = true; > + zone.sflags[zone.forward] = true; > > zone.dflags = {}; > zone.dflags[zone.output] = true; > -- > 2.37.3 >
diff --git a/root/usr/share/firewall4/templates/ruleset.uc b/root/usr/share/firewall4/templates/ruleset.uc index eaa1f04..daef252 100644 --- a/root/usr/share/firewall4/templates/ruleset.uc +++ b/root/usr/share/firewall4/templates/ruleset.uc @@ -239,7 +239,7 @@ table inet fw4 { ct status dnat accept comment "!fw4: Accept port forwards" {% endif %} {% fw4.includes('chain-append', `forward_${zone.name}`) %} - jump {{ zone.forward }}_to_{{ zone.name }} + jump {{ zone.forward }}_from_{{ zone.name }} } {% if (zone.dflags.helper): %} diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc index 29ae053..a6c1ae5 100644 --- a/root/usr/share/ucode/fw4.uc +++ b/root/usr/share/ucode/fw4.uc @@ -2113,6 +2113,7 @@ return { zone.sflags = {}; zone.sflags[zone.input] = true; + zone.sflags[zone.forward] = true; zone.dflags = {}; zone.dflags[zone.output] = true;