[PATCH] fw4: handle bad forward_zone packets with v_from_z

[Gordon Maclean]

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 at gmail.com> wrote:
>
> From: Gordon Maclean <dsmtngoat at 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 at 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
>