www/nginx add a default value > 0 to ban_ttl to prevent UI slow down over time
Important notices Before you add a new report, we ask you kindly to acknowledge the following:
- [x] I have read the contributing guide lines at https://github.com/opnsense/plugins/blob/master/CONTRIBUTING.md
- [x] I have searched the existing issues, open and closed, and I'm convinced that mine is new.
- [x] When the request is meant for an existing plugin, I've added its name to the title.
Is your feature request related to a problem? Please describe.
Our opnsense instances have gotten slower and slower over time, we even got connection aborts as soon, as we logged into web UI. I spent quite some time googling for solutions, but without finding related issues like #3397. Last week, the UI of one instance got unusable, so I investigated further. It turned out, that one PHP process, ngx_autoblock.php put 100% load on all CPUs by running multiple times. After analyzing the script, I found out, that the banned IPs are written to config.xml, on this instance 50k entries. By googling for a way to remove all entries, I found the issues, describing ban_ttl and the configuration field under advanced settings.
Describe the solution you'd like
I would add a default value other that 0 to the ban_ttl field, as 0 means to keep the IPs forever in the ban list. I have set my instances to 2880 (two days) this reduces the list to round about 100 entries. One of the issues mentions a value of 120 minutes which will aslo work - every value is better than indefinite.
Adding a value to the ban_ttl field will remove older IPs from the list as soon, as you press Apply - it takes some time but it works. (hint for People searching for slow OPNsense UI)
Describe alternatives you've considered
I have also considered the "Unlock All"-button in UI that is described in #4087 but it would require manual intervention.
Why isn't Autoblock TTL a valid option here?
Hi Sopex, yes that's the field for the ban_ttl configuration value. My proposal was not, to add a field, but to add a default value other than 0, as 0 will keep all IPs forever. Cheers, Björn
Hello brafreider,
It's definitely not up to me, but are there examples of different values being used as default by someone else? I believe 0 is pretty valid as a default.
Maybe bring it forward and not under advanced, but nginx is a "complicated" plugin, I suppose most users go over all the options while setting it up.
I agree, but from my understanding a default value should be something, that works and not something that jams your appliance over time. Even making the field required and giving no default value would be better than 0. I am configuring NGINX instances for years now, not with opnsense that often, but from ground on. All other options within the integration make sense, if you know NGINX.
The Autoblock TTL is not an NGINX option like activate and there is no documentation available what will happen if you use the default value. Inline doc only says "Set autoblock lifetime in minutes. Set to 0 for infinite.".
What happens is, that every blocked IP adds multiple lines to your OPNsense configuration XML that will stay there forever, if you chose 0...
Cheers, Björn
@brafreider I have made a suggestion, it's probably a bit more than you hoped/suggested for, but I believe it's a decent default.
This issue has been automatically timed-out (after 180 days of inactivity).
For more information about the policies for this repository, please read https://github.com/opnsense/plugins/blob/master/CONTRIBUTING.md for further details.
If someone wants to step up and work on this issue, just let us know, so we can reopen the issue and assign an owner to it.
If we can reopen this and consider the PR :) Thanks
@sopex These IPs should not be dumped into the config.xml in the first place.
They could be flushed into their own file so a slowdown cannot even happen in the first place.
I know then its not easily piped through the HA sync process or in configuration backups, but this ban stuff should be rather ethereal anyway.
@sopex These IPs should not be dumped into the config.xml in the first place.
They could be flushed into their own file so a slowdown cannot even happen in the first place.
I know then its not easily piped through the HA sync process or in configuration backups, but this ban stuff should be rather ethereal anyway.
Indeed but until someone implements that, dumping them in config.xml in perpetuity is not optimal.
@kulikov-a could you consider flushing these autoban IP addresses into their own file separate from the config.xml?
This issue has been automatically timed-out (after 180 days of inactivity).
For more information about the policies for this repository, please read https://github.com/opnsense/plugins/blob/master/CONTRIBUTING.md for further details.
If someone wants to step up and work on this issue, just let us know, so we can reopen the issue and assign an owner to it.
@Monviech Hi! When I initially proposed the changes at https://github.com/opnsense/plugins/pull/3106, my logic was simple: when working with a ban list, it is impossible to predict the administrator's desire and it is not really necessary - the main thing is to give the ability to manage this number. Ban_ttl should not be 0 in any case imo - not so much because of the storage location of the entry, but from simple common sense) (I generally limit myself to a couple of hours. In my opinion, this is enough from a security point of view and quite optimal from a storage point of view). Regarding separate storage - I am not sure that this is optimal from the point of view of backup and synchronization (that is, such a solution should be further discussed), especially if there are no problems when a reasonable TTL specified. and I do not quite understand the status of my previous proposal (https://github.com/opnsense/plugins/pull/4600)
Hi @kulikov-a , I pinged in your last proposal to get it considered again.
For the ttl, I would propose migrating users away from 0 as ive seen these issues multiple times in the past, also in commercial support as side effect.
I would advice to remove the default and required from the model, put a default value in the jinja template or wherever this is set, and create a migration from 0 to "" (empty string) so currently affected users are moved to a new safe default.
A hint in the form can expose the empty default.
@Monviech thanks! yep, I'll try to suggest something about ttl default (maybe not as quickly as I'd like, but I'll try not to delay)
@kulikov-a Thank you for looking into it.
Additionally I would validate the integer field that minimum value is at least 1, and maximum value not more than a week.
I would really like if "bad" custom values are prohibited due to the impact (its hard to debug in support).
@Monviech Hi! please take a look at https://github.com/opnsense/plugins/pull/4937 when you have time. I left the option to return the previous behavior if the Administrator knows what he is doing and is ready to clean the list manually in the GUI (i can imagine that case). But I added a warning about this in the form