-
Notifications
You must be signed in to change notification settings - Fork 881
Firewall: Rules [new]: Fix handling of interfacenot, evaluate as floating rules in correct prio_group #9426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…ting rules in correct prio_group
fichtner
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks-like-it-does-the-thing type of approval
66de53d to
df0c802
Compare
|
I fixed something small and now it reflects reality in my test: Empty Interface, Inverted Group, Inverted Interface, Multiple Interface -> All in floating
@fichtner Should I wait on this one or is it okay for master? |
src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php
Show resolved
Hide resolved
src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php
Show resolved
Hide resolved
| // floating: empty, multiple, or inverted interface | ||
| (string)$this->interfacenot === "1" || | ||
| (strpos($interface, ",")) !== false || | ||
| empty($interface) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't seem to match our actual sorting
core/src/etc/inc/filter.lib.inc
Lines 675 to 681 in c6eaefc
| if (isset($rule['floating'])) { | |
| $prio = 200000; | |
| } elseif (isset($ifgroups[$rule['interface']])) { | |
| $prio = 300000 + $ifgroups[$rule['interface']]; | |
| } else { | |
| $prio = 400000; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be0b18930f36f9 says it's a floating rule pinned to one interface so this seems correct?
src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php
Outdated
Show resolved
Hide resolved
…ontroller.php Co-authored-by: Franco Fichtner <[email protected]>
…ontroller.php Co-authored-by: Franco Fichtner <[email protected]>
src/opnsense/mvc/app/controllers/OPNsense/Firewall/Api/FilterController.php
Outdated
Show resolved
Hide resolved
…ontroller.php Co-authored-by: Franco Fichtner <[email protected]>

Fixes: #9425
A bit unsure if I got all spots, but this seems to work.
@AdSchellevis please correct this one as needed.