Re: [PATCH] fw4: handle bad forward_zone packets with v_from_z

2022-09-28 Thread 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  wrote:
>
> From: Gordon Maclean 
>
> Received forward packets which fail acceptance tests are finally handled
> by a _to_ chain where  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 "_from_",
> 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 
> ---
>  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
>

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] Send bad forward_zone packets to verdict_from_zone

2022-09-28 Thread Gordon Maclean
This patch was resubmitted, in a format more closely matching the openwrt
patch guidelines, with the subject
   "[PATCH] fw4: handle bad forward_zone packets with v_from_z" .


Gordon


On Wed, Sep 28, 2022 at 9:31 AM  wrote:
>
> From: Gordon Maclean 
>
> Received forward packets which fail acceptance tests are finally handled by a 
> _to_ chain
> where  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.
>
> As a result the final resolution depends on the default policy for the 
> forward chain, which for a
> reasonably configured router is "drop" or "reject", so this is unlikely to be 
> a security hole,
> This does not match what the user has configured as the resolution of forward 
> packets received
> for the zone.  Also, if the user has enabled logging of failed packets, these 
> packets will not be logged.
>
> This patch may also result in failues in firewall4/tests. That has not been 
> investigated.
> ---
>  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
>

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] Send bad forward_zone packets to verdict_from_zone

2022-09-28 Thread Jo-Philipp Wich
Hi,

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.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH] fw4: handle bad forward_zone packets with v_from_z

2022-09-28 Thread dsmtngoat
From: Gordon Maclean 

Received forward packets which fail acceptance tests are finally handled
by a _to_ chain where  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 "_from_",
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 
---
 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


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


[PATCH] Send bad forward_zone packets to verdict_from_zone

2022-09-28 Thread dsmtngoat
From: Gordon Maclean 

Received forward packets which fail acceptance tests are finally handled by a 
_to_ chain
where  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.

As a result the final resolution depends on the default policy for the forward 
chain, which for a
reasonably configured router is "drop" or "reject", so this is unlikely to be a 
security hole,
This does not match what the user has configured as the resolution of forward 
packets received
for the zone.  Also, if the user has enabled logging of failed packets, these 
packets will not be logged.

This patch may also result in failues in firewall4/tests. That has not been 
investigated.
---
 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


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


Re: [PATCH] argp-standalone: generate a shared library instead of static library

2022-09-28 Thread Petr Štetiar
Thomas Langer  [2022-09-13 17:47:56]:

Hi,

> Change the argp-standalone package to produce a shared library instead
> of a static library for the target. The host build is not changed.

we can see that from the diff already, so we would prefer to know, why would
you like to have this change included.

> Update related packages to add it as a direct dependency, otherwise the
> buildsystem will complain.
> 
> Please check also https://github.com/openwrt/packages/pull/19357,
> a related pull request for the packages feed, to avoid that this change
> is breaking all the packages that depend on argp-standalone.

Can't this be solved in some less intrusive way? I can imagine, that this is
not going to be bisect friendly and could provide other woes in the future.

What about introducing shared library in conjunction to the static one, thus
not breaking anything? Then if it makes sense, we could convert existing
static users to shared version in a next step.

Cheers,

Petr

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel