DHCP lease issue 2.3.2
-
OK, this may not be the proper forum section. Please let me know if it isn't, I got a bit confused trying to figure it out.
I'm not on 2.3.3, but 2.3.2 (or some sub release of that. I'm on the stable branch and it says that I'm up to date).
I recently found an issue where DHCP server default lease time was set to 604800 (the note claims a default of 7200), maximum lease time was blank (the note claims a default of 86400).
DHCP requests were being assigned 86400, as it was the max, and not 604800. It was not immediately clear why this was happening because the wording for the "max" setting talks about how it is the maximum which will get assigned if a client requests a specific expiration (which I now know to be DHCP option 51). The default mentions that it will assign that value if a client does NOT request a specific expiration time.
That all seemed to make sense, except when a new device had an issue I was researching, I noticed it was receiving 86400 as the lease time. Since the other issue was related to some DHCP settings, I was poking around and equated the 86400 as a result of the device requesting a specific lease time (otherwise expecting that it would have assigned 604800).
After reviewing dhcpdump output, running some tests, etc, I realized that it was truly a maximum setting of 86400. One of those tests included attempting to change the default and max values and then getting an error that the max had to be larger than default, as well as more than 60 (which was obviously not an issue in this case).
OK. So now it makes sense why the 86400 was getting assigned, but how did it ever get to that point? Well, I upgraded, at some point, and restored a config file, so I'm guessing that's the issue.
So it seems to be quite a corner-case, and the code obviously checks the values properly on save, but perhaps additional checks should exist. Or maybe only having the check on save but if the value exists, use that value.
I'm not sure what the proper response is, but I wanted to bring it up for others to be aware.
If I can provide any other clarification, please let me know.
-
That 604800 looks like a mangled 86400 with an extra 0 thrown in. So yes, probably from some manually edited config file that was restored.
The back-end that generates all the daemon conf files (e.g. for dhcpd, unbound…) should check interactions between pfSense config parameters and make adjustments where essential to make the parameters valid for feeding to the daemon concerned. That will ensure that no matter what errors are in a config file (as far as possible!), something reasonably sensible will be done (as long as the back-end run-time fixup does not lower security).
There could be a config validation routine that runs at the point of restoring a config. That could do all the checks that the "save" button currently does everywhere, and warn the user about invalid settings. To do that effectively requires re-engineering the code so that all validation checks are nicely contained in callable functions (not inline with the UI code). Then anything that wants to do validation can easily call the common functions.
-
604800 is 7 days worth of seconds.
So, in this case, nothing manually edited in a config file, just probably something I edited in the GUI before a check existed. I can't remember doing that, specifically, but it seems the most likely case.
Config validation at restore would be a good option.
I also thought of some sort of "default to X" values, along the lines of adjusting the improper value to something proper, but then how would the user know something was wrong? That new value would need to get saved so that the next time the GUI was referenced, if there was an issue, proper troubleshooting could occur. Of course, that means something undesired has probably already happened at that point.
Perhaps, ultimately, some sort of notification from the backend conf file generation. Notification could display on the GUI and, if you have it set up, send a notification as well.
I'm not sure what the best "quick fix" style option would be, but I wanted to point this out at least so folks were aware and perhaps it would lead to some ideas if someone was troubleshooting something that seemed to be fine.
-
Perhaps, ultimately, some sort of notification from the backend conf file generation. Notification could display on the GUI and, if you have it set up, send a notification as well.
Yes, if the backend "fixes" something on-the-fly to avoid trouble, then it should make a notification so that someone will see it one day and follow it up.