[PATCH] fw4: fix handling of unaccepted forward packets

[Gordon Maclean]

OK, I finally understand that the forward option in a zone applies to traffic
within the zone. Calling it "intra-zone forward" might be more clear, though
I do see that it is stated explicitly in the "Firewall - Zone Settings" window.

Here's a change to enable logging of unaccepted inter-zone forwards,
in case it is of any use.

diff --git a/root/usr/share/firewall4/templates/ruleset.uc
b/root/usr/share/firewall4/templates/ruleset.uc
index d6eedfd..e848da8 100644
--- a/root/usr/share/firewall4/templates/ruleset.uc
+++ b/root/usr/share/firewall4/templates/ruleset.uc
@@ -243,6 +243,8 @@ table inet fw4 {
 {%  if (fw4.forward_policy() != "accept" && (zone.log & 1)): %}
                log prefix "{{ fw4.forward_policy() }} {{ zone.name }}
forward: "
 {%  endif %}
+{%  fw4.includes('chain-append', `forward_${zone.name}`) %}
+                jump {{ fw4.forward_policy() }}_from_{{ zone.name }}
        }

 {%  if (zone.dflags.helper): %}
diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 47e86cd..fe06f3e 100644
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -2112,7 +2112,9 @@ return {
                zone.masq6_dest_subnets =
subnets_group_by_masking(masq_dest_subnets[1]);

                zone.sflags = {};
-               zone.sflags[zone.input] = true;
+               zone.sflags["accept"] = true;
+               zone.sflags["reject"] = true;
+               zone.sflags["drop"] = true;

                zone.dflags = {};
                zone.dflags[zone.output] = true;

> Hi,
> you misunderstand the purpose of the zone forward policy. It is not meant to
> catch traffic from a zone to another zone, but traffic relayed from one
> interface to another interface within the same zone.

> Traffic from one zone to another zone is solely handled by the global forward
> policy in the defaults section (or individual rules).

> Your patch would change this long standing behavior, therefor NACK from me.
> ~ Jo


On Thu, Nov 3, 2022 at 3:10 PM <dsmtngoat at gmail.com> wrote:
>
> From: Gordon Maclean <dsmtngoat at gmail.com>
>
> This is a resumbit of
> [PATCH] fw4: handle bad forward_zone packets with v_from_z,
> with an updated commit message.
>
> Below, FROM and TO are capitalized for emphasis.
>
> Packets on the input chain that fail accceptance
> rules are eventually handled by a rule created by
> "jump {{ zone.input }}_FROM_{{ zone.name }}" in ruleset.uc.
>
> For a wan zone, with input policy "drop", this results in a jump to
> the "drop_FROM_wan" chain where they are optionally logged, and
> dropped.
>
> However, packets on the forward chain that fail acceptance rules
> are eventually handled by the rule created by
> "jump {{ zone.forward }}_TO_{{ zone.name }}" in ruleset.uc.
>
> For zone wan, with forward policy "drop", packets would be sent to
> the "drop_TO_wan" chain.
>
> This is a bug, since that chain matches packets sent TO the
> interface for the zone, not FROM the interface, and so will
> fail to match all unaccepted forwarded packets received on the
> zone. As a result these forwarded packets are handled by the
> global forward policy and not by the forward policy for the
> zone, and will not be logged.
>
> This patch sets the final disposition for unaccepted forwarded
> packets to be the same as for unaccepted input packets.
>
> 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 d6eedfd..833c762 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 (fw4.forward_policy() != "accept" && (zone.log & 1)): %}
>                 log prefix "{{ fw4.forward_policy() }} {{ zone.name }} forward: "
>  {%  endif %}
> diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
> index 47e86cd..74b7c81 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.38.1
>