double "block all targeting port 0" Automatically generated rules on the WAN

[ryp43]

Hi!

I have noticed that there is a double "block all targeting port 0" Automatically generated rules on the WAN interface - OPNsense 23.7.7_3-amd64

   IPv4+6 TCP/UDP   *   *   *   *   *   *   *   block all targeting port 0   
     IPv4+6 TCP/UDP   *   *   *   *   *   *   *   block all targeting port 0

Might be a bug?

[doktornotor]

It's actually not double. It's from and to port 0, for IPv4 and IPv6. The GUI display is kinda suboptimal.

Code: [Select]

block in log quick inet proto {tcp udp} from {any} port {0} to {any} label "7b5bdc64d7ae74be1932f6764a591da5" # block all targeting port 0
block in log quick inet6 proto {tcp udp} from {any} port {0} to {any} label "7b5bdc64d7ae74be1932f6764a591da5" # block all targeting port 0
block in log quick inet proto {tcp udp} from {any} to {any} port {0} label "ae69f581dc429e3484a65f8ecd63baa5" # block all targeting port 0
block in log quick inet6 proto {tcp udp} from {any} to {any} port {0} label "ae69f581dc429e3484a65f8ecd63baa5" # block all targeting port 0


[ryp43]

Thank you!

[franco]

This looks like one of those empty() issue where "0" is treated as "" but that is not what we want... let me fix that.


Cheers,
Franco

[franco]

Ahh, better? https://github.com/opnsense/core/commit/a2d55c89394

# opnsense-patch a2d55c89394


Cheers,
Franco

[doktornotor]

This looks like one of those empty() issue where "0" is treated as "" but that is not what we want... let me fix that.

Astonishingly, someone has decided to re-implement the same stupid PHP design in JS. #SMH 

https://github.com/vvmspace/empty-js

[franco]

empty() has the clear advantage of acting like an evaluation and not caring about whatever you do inside if it fails. But, yes, the "0" handling is a clear disadvantage ever since isset() was nerfed to throw warnings on undefined array access...

[doktornotor]

Other than that, I don't think the patch has changed anything in the GUI? The [s|d]port is still shown as * - at least looking at other interfaces than WAN. (That one still cannot be expanded for me.)

[franco]

https://imgur.com/a/aNjfTqi looks good to me

I think manual rules may have the same issue but I'm too lazy ... pprint_port()

[doktornotor]

Well I am probably haunted... Even restarted the WebUI.

[franco]

Applied twice with opnsense-patch? Second time reverts the patch.


Cheers,
Franco

[franco]

Sometimes I'm a proper idiot. It needs both spots and I decided to ditch the other because the first mod wasn't working...

https://github.com/opnsense/core/commit/e37a47371823


Cheers,
Franco

[doktornotor]

No dice. I must be missing something.

[franco]

You need both patches active. I totally missed this too.

But 23.7.8 has both patches anyway.


Cheers,
Franco

[doktornotor]

Well...

Code: [Select]

# opnsense-patch a2d55c89394 && opnsense-patch e37a47371823
Fetched a2d55c89394 via https://github.com/opnsense/core
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|From a2d55c8939489fedd714bdb70ac0e640c5911df5 Mon Sep 17 00:00:00 2001
|From: Franco Fichtner <franco@opnsense.org>
|Date: Thu, 9 Nov 2023 09:06:50 +0100
|Subject: [PATCH] firewall: port can be zero in automatic rule, render
| accordingly
|
|PR: https://forum.opnsense.org/index.php?topic=36885.0
|---
| src/opnsense/mvc/app/library/OPNsense/Firewall/FilterRule.php | 4 ++--
| 1 file changed, 2 insertions(+), 2 deletions(-)
|
|diff --git a/src/opnsense/mvc/app/library/OPNsense/Firewall/FilterRule.php b/src/opnsense/mvc/app/library/OPNsense/Firewall/FilterRule.php
|index 1ddbef8a79..8ef346b7c3 100644
|--- a/src/opnsense/mvc/app/library/OPNsense/Firewall/FilterRule.php
|+++ b/src/opnsense/mvc/app/library/OPNsense/Firewall/FilterRule.php
--------------------------
Patching file opnsense/mvc/app/library/OPNsense/Firewall/FilterRule.php using Plan A...
Hunk #1 succeeded at 329.
Hunk #2 succeeded at 355.
done
All patches have been applied successfully.  Have a nice day.
Fetched e37a47371823 via https://github.com/opnsense/core
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|From e37a4737182314444539c0b83ccfd37fed4ab85f Mon Sep 17 00:00:00 2001
|From: Franco Fichtner <franco@opnsense.org>
|Date: Thu, 9 Nov 2023 12:17:46 +0100
|Subject: [PATCH] firewall: also patch this spot -.-
|
|---
| src/www/guiconfig.inc | 2 +-
| 1 file changed, 1 insertion(+), 1 deletion(-)
|
|diff --git a/src/www/guiconfig.inc b/src/www/guiconfig.inc
|index 7911fa1ab5..1fa010938d 100644
|--- a/src/www/guiconfig.inc
|+++ b/src/www/guiconfig.inc
--------------------------
Patching file www/guiconfig.inc using Plan A...
Hunk #1 succeeded at 340.
done
All patches have been applied successfully.  Have a nice day.
# configctl webgui restart
OK

Same result as the screenshot above. Nevermind. Not important enough to deal with this PHP nonsense.