Firewall / os-firewall plugin: core inclusion as a base for firewall rule MVC/API conversion

[fichtner]

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

• I have read the contributing guide lines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
• I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Is your feature request related to a problem? Please describe.

Add more properties to the module so we can start closing the gap between core and the plugin rules, possibly helping to ease future migrations.

Describe the solution you like

• eventually we want to have this in core and implemented as a single piece of code that automatically adds features as they are included
• create a list of missing features to work on mid-term

Describe alternatives you considered

N/A

Additional context

N/A

[AdSchellevis]

8591377

[fichtner]

Nice! We should get rid of "pfplugin" use as much as possible to consolidate the code, like 490efb1

[fichtner]

Migration is a bit weird:

Changes between selected versions
--
--- /conf/backup/config-1704477177.7315.xml	2024-01-05 18:52:57.762926000 +0100
+++ /conf/backup/config-1704657874.9951.xml	2024-01-07 21:04:35.024330000 +0100
@@ -737,15 +737,6 @@
</created>
<disabled>1</disabled>
</rule>
-    <npt>
-      <category/>
-      <descr/>
-      <interface>wan</interface>
-      <trackif/>
-      <source>
-        <address>ff00::</address>
-      </source>
-    </npt>
<onetoone/>
</nat>
<filter>
@@ -1119,8 +1110,8 @@
</widgets>
<revision>
<username>(system)</username>
-    <description>/usr/local/bin/phpunit made changes</description>
-    <time>1704477177.7315</time>
+    <description>/usr/local/opnsense/mvc/script/run_migrations.php made changes</description>
+    <time>1704657874.9951</time>
</revision>
<openvpn>
<openvpn-client>
@@ -2103,9 +2094,10 @@
</category>
</categories>
</Category>
-      <Filter version="1.0.2">
+      <Filter version="1.0.3">
<rules/>
<snatrules/>
+        <npt/>
</Filter>
</Firewall>
<vnstat>

[AdSchellevis]

@fichtner it should omit faulty input to avoid breakage of the records that are parseable. So in this case, the question is if ff00:: should be valid (in which case we might change the validation slightly)

[fichtner]

It worked before and it's an IPv6. I'm a little worried about changing behaviour here dropping such entries.

[fichtner]

Maybe as an added info the /128 is not stored from the page if it was looking for a subnet notation....

[AdSchellevis]

The original validations didn't match reality unfortunately (alias passed for example), but allowing single addresses isn't an issue.

core/src/www/firewall_nat_npt_edit.php

Lines 93 to 95 in cd3fa5a

	if (!is_ipaddroralias($pconfig['src'])) {
	$input_errors[] = sprintf(gettext("%s is not a valid source IP address or alias."), $pconfig['src']);
	}

[fichtner]

The alias on the source is ok?

[AdSchellevis]

nope, not supported

[fichtner]

nope, not supported

confirmed earlier to day... that's probably the reason why the page never had a selectpicker with alias selection although it validated during submit (copy + paste)