From 39f350af5d51e42a3c858edfaddef52c58cd09e0 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 4 Nov 2023 08:14:03 +0100 Subject: [PATCH 1/7] Only add dhcp.leaseTime if it actually set. If not given, the default lease time is one hour for IPv4 and one day for IPv6 (dnsmasq defaults, see their man page). Signed-off-by: DL6ER --- src/config/dnsmasq_config.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/config/dnsmasq_config.c b/src/config/dnsmasq_config.c index e727e3a7a..20a4a91a0 100644 --- a/src/config/dnsmasq_config.c +++ b/src/config/dnsmasq_config.c @@ -419,11 +419,13 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_ fputs("# DHCP server setting\n", pihole_conf); fputs("dhcp-authoritative\n", pihole_conf); fputs("dhcp-leasefile="DHCPLEASESFILE"\n", pihole_conf); - fprintf(pihole_conf, "dhcp-range=%s,%s,%s\n", + fprintf(pihole_conf, "dhcp-range=%s,%s", conf->dhcp.start.v.s, - conf->dhcp.end.v.s, - conf->dhcp.leaseTime.v.s); - fprintf(pihole_conf, "dhcp-option=option:router,%s\n", + conf->dhcp.end.v.s); + // Lease time is optional, only add it if it is set + if(strlen(conf->dhcp.leaseTime.v.s) > 0) + fprintf(pihole_conf, ",%s", conf->dhcp.leaseTime.v.s); + fprintf(pihole_conf, "\ndhcp-option=option:router,%s\n", conf->dhcp.router.v.s); if(conf->dhcp.rapidCommit.v.b) From 6f2c6fe8011ab0e9286fe6011efcaa2d6aec5794 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 4 Nov 2023 08:20:09 +0100 Subject: [PATCH 2/7] Add dhcp.netmask (type IPv4 address) and change the type of dhcp.{start,end,router} from string to IPv4 address to allow detection of invalid settings (e.g., "192.168.1.3000") early on Signed-off-by: DL6ER --- src/api/docs/content/specs/config.yaml | 3 +++ src/config/config.c | 19 +++++++++++++------ src/config/config.h | 3 ++- src/config/dnsmasq_config.c | 15 ++++++++++++--- test/pihole.toml | 11 +++++++++++ 5 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index db3154d14..8158be03b 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -299,6 +299,8 @@ components: type: string router: type: string + netmask: + type: string domain: type: string leaseTime: @@ -627,6 +629,7 @@ components: end: "192.168.0.250" router: "192.168.0.1" domain: "lan" + netmask: "0.0.0.0" leaseTime: "24h" ipv6: true rapidCommit: true diff --git a/src/config/config.c b/src/config/config.c index 72c364070..184b74f36 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -687,23 +687,23 @@ void initConfig(struct config *conf) conf->dhcp.start.k = "dhcp.start"; conf->dhcp.start.h = "Start address of the DHCP address pool"; conf->dhcp.start.a = cJSON_CreateStringReference(", e.g., \"192.168.0.10\""); - conf->dhcp.start.t = CONF_STRING; + conf->dhcp.start.t = CONF_STRUCT_IN_ADDR; conf->dhcp.start.f = FLAG_RESTART_FTL; - conf->dhcp.start.d.s = (char*)""; + memset(&conf->dhcp.start.d.in_addr, 0, sizeof(struct in_addr)); conf->dhcp.end.k = "dhcp.end"; conf->dhcp.end.h = "End address of the DHCP address pool"; conf->dhcp.end.a = cJSON_CreateStringReference(", e.g., \"192.168.0.250\""); - conf->dhcp.end.t = CONF_STRING; + conf->dhcp.end.t = CONF_STRUCT_IN_ADDR; conf->dhcp.end.f = FLAG_RESTART_FTL; - conf->dhcp.end.d.s = (char*)""; + memset(&conf->dhcp.end.d.in_addr, 0, sizeof(struct in_addr)); conf->dhcp.router.k = "dhcp.router"; conf->dhcp.router.h = "Address of the gateway to be used (typically the address of your router in a home installation)"; conf->dhcp.router.a = cJSON_CreateStringReference(", e.g., \"192.168.0.1\""); - conf->dhcp.router.t = CONF_STRING; + conf->dhcp.router.t = CONF_STRUCT_IN_ADDR; conf->dhcp.router.f = FLAG_RESTART_FTL; - conf->dhcp.router.d.s = (char*)""; + memset(&conf->dhcp.router.d.in_addr, 0, sizeof(struct in_addr)); conf->dhcp.domain.k = "dhcp.domain"; conf->dhcp.domain.h = "The DNS domain used by your Pi-hole"; @@ -712,6 +712,13 @@ void initConfig(struct config *conf) conf->dhcp.domain.f = FLAG_RESTART_FTL | FLAG_ADVANCED_SETTING; conf->dhcp.domain.d.s = (char*)"lan"; + conf->dhcp.netmask.k = "dhcp.netmask"; + conf->dhcp.netmask.h = "The netmask used by your Pi-hole. For directly connected networks (i.e., networks on which the machine running Pi-hole has an interface) the netmask is optional and may be set to \"0.0.0.0\": it will then be determined from the interface configuration itself. For networks which receive DHCP service via a relay agent, we cannot determine the netmask itself, so it should explicitly be specified, otherwise Pi-hole guesses based on the class (A, B or C) of the network address."; + conf->dhcp.netmask.a = cJSON_CreateStringReference(", e.g., \"255.255.255.0\" or \"0.0.0.0\" for auto-discovery"); + conf->dhcp.netmask.t = CONF_STRUCT_IN_ADDR; + conf->dhcp.netmask.f = FLAG_RESTART_FTL | FLAG_ADVANCED_SETTING; + memset(&conf->dhcp.netmask.d.in_addr, 0, sizeof(struct in_addr)); + conf->dhcp.leaseTime.k = "dhcp.leaseTime"; conf->dhcp.leaseTime.h = "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."; conf->dhcp.leaseTime.a = cJSON_CreateStringReference("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"); diff --git a/src/config/config.h b/src/config/config.h index c230627ee..d75ef0e9d 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -88,7 +88,7 @@ enum conf_type { #define FLAG_PSEUDO_ITEM (1 << 2) #define FLAG_INVALIDATE_SESSIONS (1 << 3) #define FLAG_WRITE_ONLY (1 << 4) -#define FLAG_ENV_VAR (1 << 5) +#define FLAG_ENV_VAR (1 << 5) struct conf_item { const char *k; // item Key @@ -178,6 +178,7 @@ struct config { struct conf_item end; struct conf_item router; struct conf_item domain; + struct conf_item netmask; struct conf_item leaseTime; struct conf_item ipv6; struct conf_item rapidCommit; diff --git a/src/config/dnsmasq_config.c b/src/config/dnsmasq_config.c index 20a4a91a0..210665583 100644 --- a/src/config/dnsmasq_config.c +++ b/src/config/dnsmasq_config.c @@ -419,9 +419,18 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_ fputs("# DHCP server setting\n", pihole_conf); fputs("dhcp-authoritative\n", pihole_conf); fputs("dhcp-leasefile="DHCPLEASESFILE"\n", pihole_conf); - fprintf(pihole_conf, "dhcp-range=%s,%s", - conf->dhcp.start.v.s, - conf->dhcp.end.v.s); + char start[INET_ADDRSTRLEN] = { 0 }, end[INET_ADDRSTRLEN] = { 0 }; + inet_ntop(AF_INET, &conf->dhcp.start.v.in_addr, start, INET_ADDRSTRLEN); + inet_ntop(AF_INET, &conf->dhcp.end.v.in_addr, end, INET_ADDRSTRLEN); + fprintf(pihole_conf, "dhcp-range=%s,%s", start, end); + // Net mask is optional, only add if it is not 0.0.0.0 + const struct in_addr inaddr_empty = {0}; + if(memcmp(&conf->dhcp.netmask.v.in_addr, &inaddr_empty, sizeof(inaddr_empty)) != 0) + { + char netmask[INET_ADDRSTRLEN] = { 0 }; + inet_ntop(AF_INET, &conf->dhcp.netmask.v.in_addr, netmask, INET_ADDRSTRLEN); + fprintf(pihole_conf, ",%s", conf->dhcp.netmask.v.s); + } // Lease time is optional, only add it if it is set if(strlen(conf->dhcp.leaseTime.v.s) > 0) fprintf(pihole_conf, ",%s", conf->dhcp.leaseTime.v.s); diff --git a/test/pihole.toml b/test/pihole.toml index 2fc68e648..60525e433 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -376,6 +376,17 @@ # domain = "lan" + # The netmask used by your Pi-hole. For directly connected networks (i.e., networks on + # which the machine running Pi-hole has an interface) the netmask is optional and may + # be set to "0.0.0.0": it will then be determined from the interface configuration + # itself. For networks which receive DHCP service via a relay agent, we cannot + # determine the netmask itself, so it should explicitly be specified, otherwise + # Pi-hole guesses based on the class (A, B or C) of the network address. + # + # Possible values are: + # , e.g., "255.255.255.0" or "0.0.0.0" for auto-discovery + netmask = "0.0.0.0" + # 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. # From 75cd372d0e4c00ad2a3df986ca6d02bc0577092f Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 4 Nov 2023 12:12:02 +0100 Subject: [PATCH 3/7] Add string format verification in API checker Signed-off-by: DL6ER --- src/api/docs/content/specs/config.yaml | 8 ++++ src/config/dnsmasq_config.c | 10 +++-- test/api/libs/responseVerifyer.py | 54 ++++++++++++++++++++------ 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index 8158be03b..59f24bf25 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -268,8 +268,10 @@ components: type: boolean IPv4: type: string + x-format: ipv4 IPv6: type: string + x-format: ipv6 blocking: type: object properties: @@ -279,8 +281,10 @@ components: type: boolean IPv4: type: string + x-format: ipv4 IPv6: type: string + x-format: ipv6 rateLimit: type: object properties: @@ -295,12 +299,16 @@ components: type: boolean start: type: string + x-format: ipv4 end: type: string + x-format: ipv4 router: type: string + x-format: ipv4 netmask: type: string + x-format: ipv4 domain: type: string leaseTime: diff --git a/src/config/dnsmasq_config.c b/src/config/dnsmasq_config.c index 210665583..3a3894af1 100644 --- a/src/config/dnsmasq_config.c +++ b/src/config/dnsmasq_config.c @@ -419,9 +419,12 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_ fputs("# DHCP server setting\n", pihole_conf); fputs("dhcp-authoritative\n", pihole_conf); fputs("dhcp-leasefile="DHCPLEASESFILE"\n", pihole_conf); - char start[INET_ADDRSTRLEN] = { 0 }, end[INET_ADDRSTRLEN] = { 0 }; + char start[INET_ADDRSTRLEN] = { 0 }, + end[INET_ADDRSTRLEN] = { 0 }, + router[INET_ADDRSTRLEN] = { 0 }; inet_ntop(AF_INET, &conf->dhcp.start.v.in_addr, start, INET_ADDRSTRLEN); inet_ntop(AF_INET, &conf->dhcp.end.v.in_addr, end, INET_ADDRSTRLEN); + inet_ntop(AF_INET, &conf->dhcp.router.v.in_addr, router, INET_ADDRSTRLEN); fprintf(pihole_conf, "dhcp-range=%s,%s", start, end); // Net mask is optional, only add if it is not 0.0.0.0 const struct in_addr inaddr_empty = {0}; @@ -429,13 +432,12 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_ { char netmask[INET_ADDRSTRLEN] = { 0 }; inet_ntop(AF_INET, &conf->dhcp.netmask.v.in_addr, netmask, INET_ADDRSTRLEN); - fprintf(pihole_conf, ",%s", conf->dhcp.netmask.v.s); + fprintf(pihole_conf, ",%s", netmask); } // Lease time is optional, only add it if it is set if(strlen(conf->dhcp.leaseTime.v.s) > 0) fprintf(pihole_conf, ",%s", conf->dhcp.leaseTime.v.s); - fprintf(pihole_conf, "\ndhcp-option=option:router,%s\n", - conf->dhcp.router.v.s); + fprintf(pihole_conf, "\ndhcp-option=option:router,%s\n", router); if(conf->dhcp.rapidCommit.v.b) fputs("dhcp-rapid-commit\n", pihole_conf); diff --git a/test/api/libs/responseVerifyer.py b/test/api/libs/responseVerifyer.py index bc692fd04..c09d154a4 100644 --- a/test/api/libs/responseVerifyer.py +++ b/test/api/libs/responseVerifyer.py @@ -10,7 +10,7 @@ # Please see LICENSE file for your rights under this license. import io -import pprint +import ipaddress import random import zipfile from libs.openAPI import openApi @@ -221,8 +221,32 @@ def verify_teleporter_zip(self, teleporter_archive: bytes): return self.errors + # Check if a string is a valid IPv4 address + def valid_ipv4(self, addr: str) -> bool: + octets = addr.split(".") # type: list[str] + if len(octets) != 4: + return False + for octet in octets: + if not octet.isdigit(): + return False + if int(octet) < 0 or int(octet) > 255: + return False + return True + + + # Check if a string is a valid IPv6 address + def valid_ipv6(self, addr: str) -> bool: + # Split the address into parts + parts = addr.split(":") # type: list[str] + # Check if the address is a valid IPv6 address + if len(parts) != 8: + return False + + # Verify a single property's type - def verify_type(self, prop_type: any, yaml_type: str, yaml_nullable: bool): + def verify_type(self, prop: any, yaml_type: str, yaml_nullable: bool, yaml_format: str = None): + # Get the type of the property + prop_type = type(prop) # None is an acceptable reply when this is specified in the API specs if prop_type is type(None) and yaml_nullable: return True @@ -230,6 +254,14 @@ def verify_type(self, prop_type: any, yaml_type: str, yaml_nullable: bool): if yaml_type not in self.YAML_TYPES: self.errors.append("Property type \"" + yaml_type + "\" is not valid in OpenAPI specs") return False + if yaml_format is not None: + # Check if the format is correct + if yaml_format == "ipv4" and not type(ipaddress.ip_address(prop)) is ipaddress.IPv4Address: + self.errors.append("Property \"" + str(prop) + "\" is not a valid IPv4 address") + return False + elif yaml_format == "ipv6" and not type(ipaddress.ip_address(prop)) is ipaddress.IPv6Address: + self.errors.append("Property \"" + str(prop) + "\" is not a valid IPv6 address") + return False return prop_type in self.YAML_TYPES[yaml_type] @@ -306,17 +338,19 @@ def verify_property(self, YAMLprops: dict, YAMLexamples: dict, FTLprops: dict, p # if not defined as string, integer, etc.) yaml_nullable = 'nullable' in YAMLprop and YAMLprop['nullable'] == True + # Get format of this property (if defined) + yaml_format = YAMLprop['format'] if 'format' in YAMLprop else YAMLprop['x-format'] if 'x-format' in YAMLprop else None + # Add this property to the YAML response self.YAMLresponse[flat_path] = [] # Check type of YAML example (if defined) if 'example' in YAMLprop: - example_type = type(YAMLprop['example']) # Check if the type of the example matches the # type we defined in the API specs self.YAMLresponse[flat_path].append(YAMLprop['example']) - if not self.verify_type(example_type, yaml_type, yaml_nullable): - self.errors.append(f"API example ({str(example_type)}) does not match defined type ({yaml_type}) in {flat_path} (nullable: " + ("True" if yaml_nullable else "False") + ")") + if not self.verify_type(YAMLprop['example'], yaml_type, yaml_nullable, yaml_format): + self.errors.append(f"API example ({str(type(YAMLprop['example']))}) does not match defined type ({yaml_type}) in {flat_path} (nullable: " + ("True" if yaml_nullable else "False") + ")") return False # Check type of externally defined YAML examples (next to schema) @@ -340,16 +374,14 @@ def verify_property(self, YAMLprops: dict, YAMLexamples: dict, FTLprops: dict, p if skip_this: continue # Check if the type of the example matches the type we defined in the API specs - example_type = type(example) self.YAMLresponse[flat_path].append(example) - if not self.verify_type(example_type, yaml_type, yaml_nullable): - self.errors.append(f"API example ({str(example_type)}) does not match defined type ({yaml_type}) in {flat_path} (nullable: " + ("True" if yaml_nullable else "False") + ")") + if not self.verify_type(example, yaml_type, yaml_nullable, yaml_format): + self.errors.append(f"API example ({str(type(example))}) does not match defined type ({yaml_type}) in {flat_path} (nullable: " + ("True" if yaml_nullable else "False") + ")") return False # Compare type of FTL's reply against what we defined in the API specs - ftl_type = type(FTLprop) - if not self.verify_type(ftl_type, yaml_type, yaml_nullable): - self.errors.append(f"FTL's reply ({str(ftl_type)}) does not match defined type ({yaml_type}) in {flat_path}") + if not self.verify_type(FTLprop, yaml_type, yaml_nullable, yaml_format): + self.errors.append(f"FTL's reply ({str(type(FTLprop))}) does not match defined type ({yaml_type}) in {flat_path}") return False return all_okay From b96779905727e7bfccfbeefe569a36fe18b7bfff Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 4 Nov 2023 12:23:02 +0100 Subject: [PATCH 4/7] Reject invalid config items (error 400) instead of merely logging a warning to the log Signed-off-by: DL6ER --- src/api/config.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/api/config.c b/src/api/config.c index f04604bcb..453052dac 100644 --- a/src/api/config.c +++ b/src/api/config.c @@ -693,8 +693,23 @@ static int api_config_patch(struct ftl_conn *api) const char *response = getJSONvalue(new_item, elem, &newconf); if(response != NULL) { - log_err("/api/config: %s invalid: %s", new_item->k, response); - continue; + char *hint = calloc(strlen(new_item->k) + strlen(response) + 3, sizeof(char)); + if(hint == NULL) + { + free_config(&newconf); + return send_json_error(api, 500, + "internal_error", + "Failed to allocate memory for hint", + NULL); + } + strcpy(hint, new_item->k); + strcat(hint, ": "); + strcat(hint, response); + free_config(&newconf); + return send_json_error_free(api, 400, + "bad_request", + "Config item is invalid", + hint, true); } // Get pointer to memory location of this conf_item (global) From a356cbd13baac00db0538cd71518d6c9e9fc142e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 15 Nov 2023 11:43:10 +0100 Subject: [PATCH 5/7] Add additional DHCP range tests Signed-off-by: DL6ER --- src/config/dnsmasq_config.c | 67 +++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/config/dnsmasq_config.c b/src/config/dnsmasq_config.c index 3a3894af1..8656bf6dc 100644 --- a/src/config/dnsmasq_config.c +++ b/src/config/dnsmasq_config.c @@ -221,6 +221,73 @@ static void write_config_header(FILE *fp, const char *description) bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_config, char errbuf[ERRBUF_SIZE]) { + // Early config checks + if(conf->dhcp.active.v.b) + { + // Check if the addresses are valid + // The addresses should neither be 0.0.0.0 nor 255.255.255.255 + if((ntohl(conf->dhcp.start.v.in_addr.s_addr) == 0) || + (ntohl(conf->dhcp.start.v.in_addr.s_addr) == 0xFFFFFFFF)) + { + strncpy(errbuf, "DHCP start address is not valid", ERRBUF_SIZE); + log_err("Unable to update dnsmasq configuration: %s", errbuf); + return false; + } + if((ntohl(conf->dhcp.end.v.in_addr.s_addr) == 0) || + (ntohl(conf->dhcp.end.v.in_addr.s_addr) == 0xFFFFFFFF)) + { + strncpy(errbuf, "DHCP end address is not valid", ERRBUF_SIZE); + log_err("Unable to update dnsmasq configuration: %s", errbuf); + return false; + } + if((ntohl(conf->dhcp.router.v.in_addr.s_addr) == 0) || + (ntohl(conf->dhcp.router.v.in_addr.s_addr) == 0xFFFFFFFF)) + { + strncpy(errbuf, "DHCP router address is not valid", ERRBUF_SIZE); + log_err("Unable to update dnsmasq configuration: %s", errbuf); + return false; + } + // The addresses should neither end in .0 or .255 in the last octet + if((ntohl(conf->dhcp.start.v.in_addr.s_addr) & 0xFF) == 0 || + (ntohl(conf->dhcp.start.v.in_addr.s_addr) & 0xFF) == 0xFF) + { + strncpy(errbuf, "DHCP start address is not valid", ERRBUF_SIZE); + log_err("Unable to update dnsmasq configuration: %s", errbuf); + return false; + } + if((ntohl(conf->dhcp.end.v.in_addr.s_addr) & 0xFF) == 0 || + (ntohl(conf->dhcp.end.v.in_addr.s_addr) & 0xFF) == 0xFF) + { + strncpy(errbuf, "DHCP end address is not valid", ERRBUF_SIZE); + log_err("Unable to update dnsmasq configuration: %s", errbuf); + return false; + } + if((ntohl(conf->dhcp.router.v.in_addr.s_addr) & 0xFF) == 0 || + (ntohl(conf->dhcp.router.v.in_addr.s_addr) & 0xFF) == 0xFF) + { + strncpy(errbuf, "DHCP router address is not valid", ERRBUF_SIZE); + log_err("Unable to update dnsmasq configuration: %s", errbuf); + return false; + } + + // Check if the DHCP range is valid (start needs to be smaller than end) + if(ntohl(conf->dhcp.start.v.in_addr.s_addr) >= ntohl(conf->dhcp.end.v.in_addr.s_addr)) + { + strncpy(errbuf, "DHCP range start address is larger than or equal to the end address", ERRBUF_SIZE); + log_err("Unable to update dnsmasq configuration: %s", errbuf); + return false; + } + + // Check if the router address is within the DHCP range + if(ntohl(conf->dhcp.router.v.in_addr.s_addr) >= ntohl(conf->dhcp.start.v.in_addr.s_addr) && + ntohl(conf->dhcp.router.v.in_addr.s_addr) <= ntohl(conf->dhcp.end.v.in_addr.s_addr)) + { + strncpy(errbuf, "DHCP router address should not be within DHCP range", ERRBUF_SIZE); + log_err("Unable to update dnsmasq configuration: %s", errbuf); + return false; + } + } + log_debug(DEBUG_CONFIG, "Opening "DNSMASQ_TEMP_CONF" for writing"); FILE *pihole_conf = fopen(DNSMASQ_TEMP_CONF, "w"); // Return early if opening failed From a69130585e0303064fe78da4b9a3a2edf0e3cba3 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 15 Nov 2023 11:49:03 +0100 Subject: [PATCH 6/7] Use package ipaddress for IP address validation in API tests Signed-off-by: DL6ER --- test/api/libs/responseVerifyer.py | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/test/api/libs/responseVerifyer.py b/test/api/libs/responseVerifyer.py index c09d154a4..dcfc8f6f6 100644 --- a/test/api/libs/responseVerifyer.py +++ b/test/api/libs/responseVerifyer.py @@ -223,24 +223,21 @@ def verify_teleporter_zip(self, teleporter_archive: bytes): # Check if a string is a valid IPv4 address def valid_ipv4(self, addr: str) -> bool: - octets = addr.split(".") # type: list[str] - if len(octets) != 4: - return False - for octet in octets: - if not octet.isdigit(): - return False - if int(octet) < 0 or int(octet) > 255: - return False - return True - + try: + if type(ipaddress.ip_address(addr)) is ipaddress.IPv4Address: + return True + except ValueError: + pass + return False # Check if a string is a valid IPv6 address def valid_ipv6(self, addr: str) -> bool: - # Split the address into parts - parts = addr.split(":") # type: list[str] - # Check if the address is a valid IPv6 address - if len(parts) != 8: - return False + try: + if type(ipaddress.ip_address(addr)) is ipaddress.IPv6Address: + return True + except ValueError: + pass + return False # Verify a single property's type From 503c0538ed3c6bc6cfb1541d6be16e44e742c8b2 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 15 Nov 2023 22:09:03 +0100 Subject: [PATCH 7/7] IPv4 address 0.0.0.0 and IPv6 address :: correspond to empty strings in FTL settings Signed-off-by: DL6ER --- src/api/config.c | 35 +++++++++++++++++++++----- src/config/cli.c | 14 +++++++++-- src/config/config.c | 10 ++++---- src/config/toml_helper.c | 42 ++++++++++++++++++++++++++++--- test/api/libs/responseVerifyer.py | 10 ++++++-- 5 files changed, 92 insertions(+), 19 deletions(-) diff --git a/src/api/config.c b/src/api/config.c index 453052dac..168c558c7 100644 --- a/src/api/config.c +++ b/src/api/config.c @@ -128,12 +128,22 @@ static cJSON *addJSONvalue(const enum conf_type conf_type, union conf_value *val return cJSON_CreateStringReference(get_temp_unit_str(val->temp_unit)); case CONF_STRUCT_IN_ADDR: { + // Special case 0.0.0.0 -> return empty string + if(val->in_addr.s_addr == INADDR_ANY) + return cJSON_CreateStringReference(""); + + // else: normal address char addr4[INET_ADDRSTRLEN] = { 0 }; inet_ntop(AF_INET, &val->in_addr, addr4, INET_ADDRSTRLEN); return cJSON_CreateString(addr4); // Performs a copy } case CONF_STRUCT_IN6_ADDR: { + // Special case :: -> return empty string + if(memcmp(&val->in6_addr, &in6addr_any, sizeof(in6addr_any)) == 0) + return cJSON_CreateStringReference(""); + + // else: normal address char addr6[INET6_ADDRSTRLEN] = { 0 }; inet_ntop(AF_INET6, &val->in6_addr, addr6, INET6_ADDRSTRLEN); return cJSON_CreateString(addr6); // Performs a copy @@ -400,11 +410,19 @@ static const char *getJSONvalue(struct conf_item *conf_item, cJSON *elem, struct struct in_addr addr4 = { 0 }; if(!cJSON_IsString(elem)) return "not of type string"; - if(!inet_pton(AF_INET, elem->valuestring, &addr4)) + if(strlen(elem->valuestring) == 0) + { + // Special case: empty string -> 0.0.0.0 + conf_item->v.in_addr.s_addr = INADDR_ANY; + } + else if(inet_pton(AF_INET, elem->valuestring, &addr4)) + { + // Set item + memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); + } + else return "not a valid IPv4 address"; - // Set item - memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); - log_debug(DEBUG_CONFIG, "%s = %s", conf_item->k, elem->valuestring); + log_debug(DEBUG_CONFIG, "%s = \"%s\"", conf_item->k, elem->valuestring); break; } case CONF_STRUCT_IN6_ADDR: @@ -412,11 +430,16 @@ static const char *getJSONvalue(struct conf_item *conf_item, cJSON *elem, struct struct in6_addr addr6 = { 0 }; if(!cJSON_IsString(elem)) return "not of type string"; - if(!inet_pton(AF_INET6, elem->valuestring, &addr6)) + if(strlen(elem->valuestring) == 0) + { + // Special case: empty string -> :: + memcpy(&conf_item->v.in6_addr, &in6addr_any, sizeof(in6addr_any)); + } + else if(!inet_pton(AF_INET6, elem->valuestring, &addr6)) return "not a valid IPv6 address"; // Set item memcpy(&conf_item->v.in6_addr, &addr6, sizeof(addr6)); - log_debug(DEBUG_CONFIG, "%s = %s", conf_item->k, elem->valuestring); + log_debug(DEBUG_CONFIG, "%s = \"%s\"", conf_item->k, elem->valuestring); break; } case CONF_JSON_STRING_ARRAY: diff --git a/src/config/cli.c b/src/config/cli.c index ddc23db98..fab52d2eb 100644 --- a/src/config/cli.c +++ b/src/config/cli.c @@ -295,7 +295,12 @@ static bool readStringValue(struct conf_item *conf_item, const char *value, stru case CONF_STRUCT_IN_ADDR: { struct in_addr addr4 = { 0 }; - if(inet_pton(AF_INET, value, &addr4)) + if(strlen(value) == 0) + { + // Special case: empty string -> 0.0.0.0 + conf_item->v.in_addr.s_addr = INADDR_ANY; + } + else if(inet_pton(AF_INET, value, &addr4)) memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); else { @@ -307,7 +312,12 @@ static bool readStringValue(struct conf_item *conf_item, const char *value, stru case CONF_STRUCT_IN6_ADDR: { struct in6_addr addr6 = { 0 }; - if(inet_pton(AF_INET6, value, &addr6)) + if(strlen(value) == 0) + { + // Special case: empty string -> :: + memcpy(&conf_item->v.in6_addr, &in6addr_any, sizeof(in6addr_any)); + } + else if(inet_pton(AF_INET6, value, &addr6)) memcpy(&conf_item->v.in6_addr, &addr6, sizeof(addr6)); else { diff --git a/src/config/config.c b/src/config/config.c index 184b74f36..9bdd80b38 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -686,21 +686,21 @@ void initConfig(struct config *conf) conf->dhcp.start.k = "dhcp.start"; conf->dhcp.start.h = "Start address of the DHCP address pool"; - conf->dhcp.start.a = cJSON_CreateStringReference(", e.g., \"192.168.0.10\""); + conf->dhcp.start.a = cJSON_CreateStringReference(" or empty string (\"\"), e.g., \"192.168.0.10\""); conf->dhcp.start.t = CONF_STRUCT_IN_ADDR; conf->dhcp.start.f = FLAG_RESTART_FTL; memset(&conf->dhcp.start.d.in_addr, 0, sizeof(struct in_addr)); conf->dhcp.end.k = "dhcp.end"; conf->dhcp.end.h = "End address of the DHCP address pool"; - conf->dhcp.end.a = cJSON_CreateStringReference(", e.g., \"192.168.0.250\""); + conf->dhcp.end.a = cJSON_CreateStringReference(" or empty string (\"\"), e.g., \"192.168.0.250\""); conf->dhcp.end.t = CONF_STRUCT_IN_ADDR; conf->dhcp.end.f = FLAG_RESTART_FTL; memset(&conf->dhcp.end.d.in_addr, 0, sizeof(struct in_addr)); conf->dhcp.router.k = "dhcp.router"; conf->dhcp.router.h = "Address of the gateway to be used (typically the address of your router in a home installation)"; - conf->dhcp.router.a = cJSON_CreateStringReference(", e.g., \"192.168.0.1\""); + conf->dhcp.router.a = cJSON_CreateStringReference(" or empty string (\"\"), e.g., \"192.168.0.1\""); conf->dhcp.router.t = CONF_STRUCT_IN_ADDR; conf->dhcp.router.f = FLAG_RESTART_FTL; memset(&conf->dhcp.router.d.in_addr, 0, sizeof(struct in_addr)); @@ -713,8 +713,8 @@ void initConfig(struct config *conf) conf->dhcp.domain.d.s = (char*)"lan"; conf->dhcp.netmask.k = "dhcp.netmask"; - conf->dhcp.netmask.h = "The netmask used by your Pi-hole. For directly connected networks (i.e., networks on which the machine running Pi-hole has an interface) the netmask is optional and may be set to \"0.0.0.0\": it will then be determined from the interface configuration itself. For networks which receive DHCP service via a relay agent, we cannot determine the netmask itself, so it should explicitly be specified, otherwise Pi-hole guesses based on the class (A, B or C) of the network address."; - conf->dhcp.netmask.a = cJSON_CreateStringReference(", e.g., \"255.255.255.0\" or \"0.0.0.0\" for auto-discovery"); + conf->dhcp.netmask.h = "The netmask used by your Pi-hole. For directly connected networks (i.e., networks on which the machine running Pi-hole has an interface) the netmask is optional and may be set to an empty string (\"\"): it will then be determined from the interface configuration itself. For networks which receive DHCP service via a relay agent, we cannot determine the netmask itself, so it should explicitly be specified, otherwise Pi-hole guesses based on the class (A, B or C) of the network address."; + conf->dhcp.netmask.a = cJSON_CreateStringReference(" (e.g., \"255.255.255.0\") or empty string (\"\") for auto-discovery"); conf->dhcp.netmask.t = CONF_STRUCT_IN_ADDR; conf->dhcp.netmask.f = FLAG_RESTART_FTL | FLAG_ADVANCED_SETTING; memset(&conf->dhcp.netmask.d.in_addr, 0, sizeof(struct in_addr)); diff --git a/src/config/toml_helper.c b/src/config/toml_helper.c index 7f40b683f..2dffff6a5 100644 --- a/src/config/toml_helper.c +++ b/src/config/toml_helper.c @@ -364,6 +364,13 @@ void writeTOMLvalue(FILE * fp, const int indent, const enum conf_type t, union c break; case CONF_STRUCT_IN_ADDR: { + // Special case: 0.0.0.0 -> return empty string + if(v->in_addr.s_addr == INADDR_ANY) + { + printTOMLstring(fp, "", toml); + break; + } + // else: normal address char addr4[INET_ADDRSTRLEN] = { 0 }; inet_ntop(AF_INET, &v->in_addr, addr4, INET_ADDRSTRLEN); printTOMLstring(fp, addr4, toml); @@ -371,6 +378,13 @@ void writeTOMLvalue(FILE * fp, const int indent, const enum conf_type t, union c } case CONF_STRUCT_IN6_ADDR: { + // Special case: :: -> return empty string + if(memcmp(&v->in6_addr, &in6addr_any, sizeof(in6addr_any)) == 0) + { + printTOMLstring(fp, "", toml); + break; + } + // else: normal address char addr6[INET6_ADDRSTRLEN] = { 0 }; inet_ntop(AF_INET6, &v->in6_addr, addr6, INET6_ADDRSTRLEN); printTOMLstring(fp, addr6, toml); @@ -654,7 +668,12 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t const toml_datum_t val = toml_string_in(toml, key); if(val.ok) { - if(inet_pton(AF_INET, val.u.s, &addr4)) + if(strlen(val.u.s) == 0) + { + // Special case: empty string -> 0.0.0.0 + conf_item->v.in_addr.s_addr = INADDR_ANY; + } + else if(inet_pton(AF_INET, val.u.s, &addr4)) memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); else log_warn("Config %s is invalid (not of type IPv4 address)", conf_item->k); @@ -670,7 +689,12 @@ void readTOMLvalue(struct conf_item *conf_item, const char* key, toml_table_t *t const toml_datum_t val = toml_string_in(toml, key); if(val.ok) { - if(inet_pton(AF_INET6, val.u.s, &addr6)) + if(strlen(val.u.s) == 0) + { + // Special case: empty string -> :: + memcpy(&conf_item->v.in6_addr, &in6addr_any, sizeof(in6addr_any)); + } + else if(inet_pton(AF_INET6, val.u.s, &addr6)) memcpy(&conf_item->v.in6_addr, &addr6, sizeof(addr6)); else log_warn("Config %s is invalid (not of type IPv6 address)", conf_item->k); @@ -910,7 +934,12 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) case CONF_STRUCT_IN_ADDR: { struct in_addr addr4 = { 0 }; - if(inet_pton(AF_INET, envvar, &addr4)) + if(strlen(envvar) == 0) + { + // Special case: empty string -> 0.0.0.0 + conf_item->v.in_addr.s_addr = INADDR_ANY; + } + else if(inet_pton(AF_INET, envvar, &addr4)) memcpy(&conf_item->v.in_addr, &addr4, sizeof(addr4)); else log_warn("ENV %s is invalid (not of type IPv4 address)", envkey); @@ -919,7 +948,12 @@ bool readEnvValue(struct conf_item *conf_item, struct config *newconf) case CONF_STRUCT_IN6_ADDR: { struct in6_addr addr6 = { 0 }; - if(inet_pton(AF_INET6, envvar, &addr6)) + if(strlen(envvar) == 0) + { + // Special case: empty string -> :: + memcpy(&conf_item->v.in6_addr, &in6addr_any, sizeof(in6addr_any)); + } + else if(inet_pton(AF_INET6, envvar, &addr6)) memcpy(&conf_item->v.in6_addr, &addr6, sizeof(addr6)); else log_warn("ENV %s is invalid (not of type IPv6 address)", envkey); diff --git a/test/api/libs/responseVerifyer.py b/test/api/libs/responseVerifyer.py index dcfc8f6f6..b527c682f 100644 --- a/test/api/libs/responseVerifyer.py +++ b/test/api/libs/responseVerifyer.py @@ -223,6 +223,9 @@ def verify_teleporter_zip(self, teleporter_archive: bytes): # Check if a string is a valid IPv4 address def valid_ipv4(self, addr: str) -> bool: + # Empty string is valid (0.0.0.0) + if len(addr) == 0: + return True try: if type(ipaddress.ip_address(addr)) is ipaddress.IPv4Address: return True @@ -232,6 +235,9 @@ def valid_ipv4(self, addr: str) -> bool: # Check if a string is a valid IPv6 address def valid_ipv6(self, addr: str) -> bool: + # Empty string is valid (::) + if len(addr) == 0: + return True try: if type(ipaddress.ip_address(addr)) is ipaddress.IPv6Address: return True @@ -253,10 +259,10 @@ def verify_type(self, prop: any, yaml_type: str, yaml_nullable: bool, yaml_forma return False if yaml_format is not None: # Check if the format is correct - if yaml_format == "ipv4" and not type(ipaddress.ip_address(prop)) is ipaddress.IPv4Address: + if yaml_format == "ipv4" and not self.valid_ipv4(prop): self.errors.append("Property \"" + str(prop) + "\" is not a valid IPv4 address") return False - elif yaml_format == "ipv6" and not type(ipaddress.ip_address(prop)) is ipaddress.IPv6Address: + elif yaml_format == "ipv6" and not self.valid_ipv6(prop): self.errors.append("Property \"" + str(prop) + "\" is not a valid IPv6 address") return False return prop_type in self.YAML_TYPES[yaml_type]