Skip to content
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

Wish: Firewall Rule Ordering #133

Closed
jared-gs opened this issue May 7, 2021 · 5 comments · Fixed by #172
Closed

Wish: Firewall Rule Ordering #133

jared-gs opened this issue May 7, 2021 · 5 comments · Fixed by #172
Labels
feature request New feature or request

Comments

@jared-gs
Copy link

jared-gs commented May 7, 2021

Hello there,

It would be great if the API allowed specifying firewall rule ordering, in either relative or absolute terms. Firewall rules are processed from top-to-bottom, so this is an important aspect the API could help deliver. Perhaps this could be done via lexicographic/alphabetical ordering based off the rule descriptions. Example:

000 - Firewall Rule A
100 - Firewall Rule B

New rule 050 - Firewall Rule C automatically goes between Firewall Rule A & Firewall Rule B. "Separators" might also be of use here as well. Or maybe a field when a rule is created/updated to specify its ordering in the larger chain of rules.

@jaredhendrickson13
Copy link
Owner

Hey!

Thanks for the request. This is a more difficult one I have wanted to do for some time but haven't found a great way to do this. I want this functionality to behave consistently between the API and the UI. Currently, firewall rules can be reordered by using the top field within PUT requests to /api/v1/firewall/rule. However, this simply moves the rule to the top of the ACL for that interface so it is definitely not a convenient solution, especially if you have large ACLs. My thinking is this should be relative to the rule's current position. Perhaps adding an up and down field that takes a numeric value of spaces to move the rule in the respective direction. This allows users to move the rules in whatever direction is needed and leaves the ordering criteria and processing up to them.

I want to avoid processing that only operates via API. For example, a weight or alphabetical ordering based on the rule's description for all users. However, if there is enough demand for such functionality it could potentially be added as an optional setting that can be enabled.

I'd love to hear other's input on how this should look, or better yet a pull request that can demonstrate this working well. This probably won't be included in the next minor release but could be included in the following release.

Thanks!

@jaredhendrickson13 jaredhendrickson13 added feature request New feature or request good first issue Good for newcomers help wanted Extra attention is needed needs research Issues that require further research labels May 11, 2021
@jared-gs
Copy link
Author

jared-gs commented May 11, 2021

Hey there @jaredhendrickson13 ,

Thanks for the reply. The desire to keep the API consistent with the UI make sense in terms of feature parity makes sense. I think the most comparable way the API could match the UI was not through specifying a relative position, but instead specifying a position number for CREATE/UPDATE calls. I think this would be simpler to write automation against if you could just "list" all the firewall rules and "put" them into the order they aught to be in.

Perhaps another option is an API service that invokes the the order-store operation currently used in the pfSense UI. The operation takes an array of tracker IDs and orders by how they are receieved: https://github.com/pfsense/pfsense/blob/a7086b04cae21ca742fdeefd1019ee1401b6dded/src/usr/local/www/firewall_rules.php#L231-L302

Likewise, I'm curious to get other people's thoughts on the matter.

@jared-gs
Copy link
Author

jared-gs commented Oct 3, 2021

Hey there @jaredhendrickson13,

Just checking in here. Has there been any update to this with upcoming features? I still see rule ordering one the key features to unblock programmatically defining firewall state as code like Terraform or Ansible. It would be good to know if you or others had any ideas on how this could be done.

Thanks!

@jaredhendrickson13
Copy link
Owner

Hey @jared-gs

I still don't have any definite plans for this yet, but I am planning on allotting some time to researching this further as a potential feature in v1.4.0. I am thinking an endpoint to sort firewall rules by different criteria such as the description, tracker, protocol, match weight, etc. would probably have the broadest use case. I'll keep this issue updated with any progress made.

On a similar note, v1.3.0 does contain the /api/v1/firewall/rule/flush endpoint which will essentially remove all existing firewall rules at once (without reloading the filter). This can be used to programmatically create the entire ACL from scratch each time. I currently use this with Ansible to ensure the ACL is ordered exactly as it shows in an Ansible inventory. That feature along with the top field seems to serve the needs of most, but of course doesn't cover all use cases. I agree a more robust rule sorting system would be a great addition to give some more options for this issue.

@jaredhendrickson13 jaredhendrickson13 removed help wanted Extra attention is needed good first issue Good for newcomers needs research Issues that require further research labels Dec 23, 2021
jaredhendrickson13 added a commit that referenced this issue Dec 23, 2021
jaredhendrickson13 added a commit that referenced this issue Dec 23, 2021
@jaredhendrickson13 jaredhendrickson13 linked a pull request Dec 23, 2021 that will close this issue
@jaredhendrickson13
Copy link
Owner

Hey!

Thanks for your patience. These features have been implemented on the v1.4.0 (#172) branch and are live on the v1.4.0beta_4 build that is posted on that pull request. Feel free to try it out. We are likely 1-2 months away from v1.4.0 being released as a production release.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants