Improve DHCP handling

[DL6ER]

What does this implement/fix?

1. Only add dhcp.leaseTime if it is a non-empty string. By default it is non-empty, however we have seen users making it empty. If empty, the default lease time is one hour for IPv4 and one day for IPv6 (dnsmasq defaults, see their man page).
2. Add new option dhcp.netmask defaulting to 0.0.0.0 (auto-discovery as before this PR) but freely settable to, e.g. 255.255.0.0 to make a /16 DHCP network
3. Add API string verification for IPv4 and IPv6 addresses. As ipv4 and ipv6 are not standardized format properties, we add them as a x-format instead. However, since swagger-api supports them there is some hope this will end up in a future revision of the OpenAPI standard. Even if not, having the verification using a non-default property seems still useful
4. Reject invalid config items (error 400) instead of merely logging a warning to the log, e.g.,
   
   where I tried to set
   
   This still adds a warning to FTL.log:

   2023-11-04 12:41:12.792 [2357991/T2358010] WARNING: API: Config item is invalid (dhcp.end: not a valid IPv4 address)
   

The first two items address a bug reported on Discourse where a user tried to specify a DHCP range from 192.168.0.300 to 192.168.0.500 with an empty leaseTime config option. The fourth item goes into this direction was well as it will allow us to show the error in a more human-digestible form to the user (upcoming web PR).

Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A

---

By submitting this pull request, I confirm the following:

1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
2. I have commented my proposed changes within the code.
3. I am willing to help maintain this change if there are issues with it later.
4. It is compatible with the EUPL 1.2 license
5. I have squashed any insignificant commits. (git rebase)

Checklist:

• The code change is tested and works locally.
• I based my code and PRs against the repositories developmental branch.
• I signed off all commits. Pi-hole enforces the DCO for all contributions
• I signed all my commits. Pi-hole requires signatures to verify authorship
• I have read the above and my PR is ready for review.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/api-error-when-enabling-dhcp-server/66079/8

[rdwebdesign]

Is it possible to use shortened IPv6 addresses here like 2001:db8::1234:5678?
Will this code work in this case?

[DL6ER]

Thanks for your comment, something got lost here. As you can see, I added import ipaddress above and actually have also used it for the check but this got somehow lost. I will add it back in

[rdwebdesign]

I tested using 192.168.0.0 as start, end and router addresses and it was considered valid in all cases.
I also tested using start = end and start > end. Both cases were considered valid.

Should we add other validation rules?

[DL6ER]

The validation of all DHCP-related stuff happens in the embedded dnsmasq. If it does not complain it accepted the values. I will add some extra checks for both cases.

[DL6ER]

[yubiuser]

By default it is non-empty, however we have seen users making it empty.

I'm seeing this as default values. Is this expected? I think we once had some logic to derive a DHCP range/gateway automatically from the interface settings.

[rdwebdesign]

I'm not sure if we should provide default values. Maybe start with empty fields is the right way.

[DL6ER]

Hmm, yes, this is actually tricky. We changed the variable type from string to IPv4. The default was an empty string before, however, there isn't something like an empty string any longer in IPv4 - everything zero is exactly what @yubiuser saw: 0.0.0.0.

I can see two things how we could proceed right now:

1. None of the config options which are of type IPv4 or IPv6 need to allow 0.0.0.0. We can add an internal conversion rule that 0.0.0.0 is in fact empty. This is actually my preferred way.
2. We do what @yubiuser suggested and try to populate the fields with (hopefully) meaningful content. I thought we'd removed this in v5 already because PIHOLE_INTERFACE in setupVars.conf was often not even set and determining useful values in such a scenario may be difficult.
   On the other hand, FTL offers /api/network/gateway which is not used by anything in the web interface and we could use the code herein to get the default interface of the entire system and use it's interface's IP address, gateway and netmask to fill the fields. It won't harm if there is real content in here as long as the active checkbox is still off.

What do you say? (1) is obviously the much easier way to go forward but we are known to not be afraid of work at Pi-hole ;-)

[yubiuser]

We can go with option 1). Less hand-holding and forcing users to think about their DHCP range before they apply the settings. If we set wrong defaults (which would never happen) and break anything we are the once to blame.
I'd also like to suggest to populate lease time with some default value (e.g. 24h)

[rdwebdesign]

I think option 1 is better.

The interface+API are already verifying most of the entries to avoid invalid values.
In my opinion, we should only provide default values where they are really needed. It makes no sense to guess what are the preferences for each possible user.

[DL6ER]

Thanks for your comment, option no. 1 is it then.

I'd also like to suggest to populate lease time with some default value (e.g. 24h)

See the API description for dhcp.leaseTime, it is intentionally left unspecified because:

If the lease time is given, then leases will be given for that length of time. If not given, the default lease time is one hour for IPv4 and one day for IPv6.

The lease time can be in seconds, or minutes (e.g., "45m") or hours (e.g., "1h") or days (like "2d") or even weeks ("1w"). You may also use "infinite" as string but be aware of the drawbacks