Add additional config validation

[DL6ER]

What does this implement/fix?

FTL is already pretty selective when it comes to accepting config values as it has a strong filter on a config value's type. For instance, your cannot set dhcp.start to a number or hostname. Similarly, you canot set dns.port to 100,000 as such a high value does not exist or -12 as negative ports don't exist either.

However, we have two config value types that do not allow strict testing: strings and arrays of strings.

This PR adds the facility to add validation callbacks to such config values so FTL will be able to check validity of those, too. I added validators for almost all string config options with only a few exceptions, e.g. dns.upstreams as dnsmasq allows upstreams to be defined by IP address or hostname, optionally with a port - we can leave checking the values to dnsmasq and will tell the user on error:

$ sudo ./pihole-FTL --config dns.upstreams '["127.0.0.1#5335","1...1#123"]'
New dnsmasq configuration is not valid (dnsmasq: Name or service not known at line 35 of /etc/pihole/dnsmasq.conf.temp: "server=1...1#123"), config remains unchanged

Some examples for validators we're adding herein:

• dns.hosts:

  $ sudo ./pihole-FTL --config dns.hosts '["127.0.0.1"]'
  Invalid value: 1st element is not in the form "IP HOSTNAME" ("127.0.0.1")
  
  $ sudo ./pihole-FTL --config dns.hosts '["1.1.1.1 cf", "127.0.0...1 hostname"]'
  Invalid value: 2nd element is neither a valid IPv4 nor IPv6 address ("127.0.0...1")
  

• dns.domain (similarly dns.revServer.domain, dhcp.domain, webserver.domain):

  $ sudo ./pihole-FTL --config dns.domain "----"
  Invalid value: Not a valid domain ("----")
  

• dns.cnameRecords:

  $ sudo ./pihole-FTL --config dns.cnameRecords '["cname,,1"]'
  Invalid value: 1st element contains an empty string at position 1
  
  $ sudo ./pihole-FTL --config dns.cnameRecords '["cname"]'
  Invalid value: 1st element is not a valid CNAME definition
  

• dns.revServer.cidr:

  $ sudo ./pihole-FTL --config dns.revServer.cidr "1.1.1.1/55"
  New dnsmasq configuration is not valid (dnsmasq: bad IPv4 prefix length at line 82 of /etc/pihole/dnsmasq.conf.temp: "rev-server=1.1.1.1/55,192.168.2.1"), config remains unchanged
  
  $ sudo ./pihole-FTL --config dns.revServer.cidr "1.1.1..1/24"
  Invalid value: Not a valid IPv4 nor IPv6 address ("1.1.1..1")
  

• dns.revServer.target

  $ sudo ./pihole-FTL --config dns.revServer.target "1.1.1..1"
  Invalid value: Not a valid IPv4 nor IPv6 address ("1.1.1..1")
  
  $ sudo ./pihole-FTL --config dns.revServer.target "1.1.1.1#500000"
  Invalid value: Not a valid port ("500000")
  

• webserver.tls.cert (similarly all paths in files):

  $ sudo ./pihole-FTL --config webserver.tls.cert "/etc/pihole/tls.pem?"
  Invalid value: Not a valid file path ("/etc/pihole/tls.pem?")
  

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.

[yubiuser]

Could you please add a comment (in the source code), which items are validated by dnsmasq. Otherwise it's hard to judge if something was forgotten or left out on purpose.

[yubiuser]

In the line above we have config_changed=true;
> should this be placed after the new value validation? We reject the new config when the value is invalid in the end.

Otherwise manual editing of pihole.toml can still add invalid entries.

[DL6ER]

It doesn't matter. If validation fails, this function will free the temporary newconf in line 776 below and return. Nobody will ever look at the value of config_changed

[DL6ER]

Could you please add a comment (in the source code), which items are validated by dnsmasq. Otherwise it's hard to judge if something was forgotten or left out on purpose.

dnsmasq validates everything causing a config rewrite (settings marked with FLAG_RESTART_FTL) but we are not particularly happy how it does this for these two: dns.cnameRecords and dns.hosts.
dnsmasq accepts an empty target as a CNAME to the root zone. I don't think this is wrong as such bu it seems we don't want this to be valid. dnsmasq also "accepts" incomplete HOSTS records by simply ignoring lines not in the correct format.

[DL6ER]

Should one-character domains be considered valid? In the context of valid_domain() a minimum domain length of two characters is required.

FTL/src/tools/gravity-parseList.c

Lines 98 to 102 in 3bdf652

	// The last label must be at least 2 characters long
	// (len-1) because we start counting from zero
	if((len - 1) - last_dot < 2)
	return false;

[yubiuser]

I think it should be two characters for external domains, but accept single characters for local hostnames.

Someone might have a.lan, b.lan,...

[DL6ER]

Those are FQDN. The length limitation applies only to the last label (lan) in your example above, a would be rejected, a.lan is fine

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[DL6ER]

The last commit adds explicit places where the issue happens:

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved.

[DL6ER]

New examples:

tre,192.168.2.0/24,192.168.2.1,fritz.box

true,692.168.2.0/24,192.168.2.1,fritz.box

true,192.168.2.0/245,192.168.2.1,fritz.box

true,192.168.2.0/24,192.168.2.1#44455585,fritz.box

[PromoFaux]

This all looks good, do you plan on adding some unit tests?

[PromoFaux]

Great stuff, even greater with tests!