From 61502fb5a1c11d8a64d6bd9d9ce1408e747e900b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 1 Dec 2023 11:52:38 +0100 Subject: [PATCH 1/2] Add dns.revServers and migrate dns.revServer. This allows multiple reverse servers to be added Signed-off-by: DL6ER --- src/api/docs/content/specs/config.yaml | 22 ++----- src/config/config.c | 34 ++--------- src/config/config.h | 8 +-- src/config/dnsmasq_config.c | 46 ++++++++++++--- src/config/toml_reader.c | 46 +++++++++++++++ src/setupVars.c | 82 ++++++++++++++++++++++++-- test/pihole.toml | 47 +++++++-------- 7 files changed, 196 insertions(+), 89 deletions(-) diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index 46577c5f..b9dd714c 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -233,17 +233,10 @@ components: type: integer optimizer: type: integer - revServer: - type: object - properties: - active: - type: boolean - cidr: - type: string - target: - type: string - domain: - type: string + revServers: + type: array + items: + type: string blocking: type: object properties: @@ -611,11 +604,8 @@ components: cache: size: 10000 optimizer: 3600 - revServer: - active: false - cidr: "192.168.0.0/24" - target: "192.168.0.1" - domain: "lan" + revServers: + - "true,192.168.0.0/24,192.168.0.1,lan" blocking: active: true mode: 'NULL' diff --git a/src/config/config.c b/src/config/config.c index 5d6eaf1d..f84175ec 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -584,6 +584,12 @@ void initConfig(struct config *conf) conf->dns.blocking.mode.t = CONF_ENUM_BLOCKING_MODE; conf->dns.blocking.mode.d.blocking_mode = MODE_NULL; + conf->dns.revServers.k = "dns.revServers"; + conf->dns.revServers.h = "Reverse server (former also called \"conditional forwarding\") feature\n Array of reverse servers each one in one of the following forms: \",[/],[#],\"\n\n Individual components:\n\n : either \"true\" or \"false\"\n\n [/]: Address range for the reverse server feature in CIDR notation. If the prefix length is omitted, either 32 (IPv4) or 128 (IPv6) are substituted (exact address match). This is almost certainly not what you want here.\n Example: \"192.168.0.0/24\" for the range 192.168.0.1 - 192.168.0.255\n\n [#]: Target server to be used for the reverse server feature\n Example: \"192.168.0.1#53\"\n\n : Domain used for the reverse server feature (e.g., \"fritz.box\")\n Example: \"fritz.box\"\n\n A valid line could look like this: \"true,192.168.0.0/24,192.168.0.1,fritz.box\""; + conf->dns.revServers.t = CONF_JSON_STRING_ARRAY; + conf->dns.revServers.d.json = cJSON_CreateArray(); + conf->dns.revServers.f = FLAG_RESTART_FTL; + // sub-struct dns.rate_limit conf->dns.rateLimit.count.k = "dns.rateLimit.count"; conf->dns.rateLimit.count.h = "Rate-limited queries are answered with a REFUSED reply and not further processed by FTL.\n The default settings for FTL's rate-limiting are to permit no more than 1000 queries in 60 seconds. Both numbers can be customized independently. It is important to note that rate-limiting is happening on a per-client basis. Other clients can continue to use FTL while rate-limited clients are short-circuited at the same time.\n For this setting, both numbers, the maximum number of queries within a given time, and the length of the time interval (seconds) have to be specified. For instance, if you want to set a rate limit of 1 query per hour, the option should look like RATE_LIMIT=1/3600. The time interval is relative to when FTL has finished starting (start of the daemon + possible delay by DELAY_STARTUP) then it will advance in steps of the rate-limiting interval. If a client reaches the maximum number of queries it will be blocked until the end of the current interval. This will be logged to /var/log/pihole/FTL.log, e.g. Rate-limiting 10.0.1.39 for at least 44 seconds. If the client continues to send queries while being blocked already and this number of queries during the blocking exceeds the limit the client will continue to be blocked until the end of the next interval (FTL.log will contain lines like Still rate-limiting 10.0.1.39 as it made additional 5007 queries). As soon as the client requests less than the set limit, it will be unblocked (Ending rate-limitation of 10.0.1.39).\n Rate-limiting may be disabled altogether by setting both values to zero (this results in the same behavior as before FTL v5.7).\n How many queries are permitted..."; @@ -659,34 +665,6 @@ void initConfig(struct config *conf) conf->dns.reply.blocking.v6.f = FLAG_ADVANCED_SETTING; memset(&conf->dns.reply.blocking.v6.d.in6_addr, 0, sizeof(struct in6_addr)); - // sub-struct revServer - conf->dns.revServer.active.k = "dns.revServer.active"; - conf->dns.revServer.active.h = "Is the reverse server (former also called \"conditional forwarding\") feature enabled?"; - conf->dns.revServer.active.t = CONF_BOOL; - conf->dns.revServer.active.d.b = false; - conf->dns.revServer.active.f = FLAG_RESTART_FTL; - - conf->dns.revServer.cidr.k = "dns.revServer.cidr"; - conf->dns.revServer.cidr.h = "Address range for the reverse server feature in CIDR notation. If the prefix length is omitted, either 32 (IPv4) or 128 (IPv6) are substitutet (exact address match). This is almost certainly not what you want here."; - conf->dns.revServer.cidr.a = cJSON_CreateStringReference("[/], e.g., \"192.168.0.0/24\" for the range 192.168.0.1 - 192.168.0.255"); - conf->dns.revServer.cidr.t = CONF_STRING; - conf->dns.revServer.cidr.d.s = (char*)""; - conf->dns.revServer.cidr.f = FLAG_RESTART_FTL; - - conf->dns.revServer.target.k = "dns.revServer.target"; - conf->dns.revServer.target.h = "Target server tp be used for the reverse server feature"; - conf->dns.revServer.target.a = cJSON_CreateStringReference("[#], e.g., \"192.168.0.1\""); - conf->dns.revServer.target.t = CONF_STRING; - conf->dns.revServer.target.d.s = (char*)""; - conf->dns.revServer.target.f = FLAG_RESTART_FTL; - - conf->dns.revServer.domain.k = "dns.revServer.domain"; - conf->dns.revServer.domain.h = "Domain used for the reverse server feature (e.g., \"fritz.box\")"; - conf->dns.revServer.domain.a = cJSON_CreateStringReference(""); - conf->dns.revServer.domain.t = CONF_STRING; - conf->dns.revServer.domain.d.s = (char*)""; - conf->dns.revServer.domain.f = FLAG_RESTART_FTL; - // sub-struct dhcp conf->dhcp.active.k = "dhcp.active"; conf->dhcp.active.h = "Is the embedded DHCP server enabled?"; diff --git a/src/config/config.h b/src/config/config.h index 12dd5194..edd02395 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -139,6 +139,7 @@ struct config { struct conf_item queryLogging; struct conf_item cnameRecords; struct conf_item port; + struct conf_item revServers; struct { struct conf_item size; struct conf_item optimizer; @@ -169,13 +170,6 @@ struct config { struct conf_item count; struct conf_item interval; } rateLimit; - - struct { - struct conf_item active; - struct conf_item cidr; - struct conf_item target; - struct conf_item domain; - } revServer; } dns; struct { diff --git a/src/config/dnsmasq_config.c b/src/config/dnsmasq_config.c index cc262bc7..ffc8c83c 100644 --- a/src/config/dnsmasq_config.c +++ b/src/config/dnsmasq_config.c @@ -447,24 +447,52 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_ } fputs("\n", pihole_conf); - if(conf->dns.revServer.active.v.b) + const unsigned int revServers = cJSON_GetArraySize(conf->dns.revServers.v.json); + for(unsigned int i = 0; i < revServers; i++) { - fputs("# Reverse server setting\n", pihole_conf); - fprintf(pihole_conf, "rev-server=%s,%s\n", - conf->dns.revServer.cidr.v.s, conf->dns.revServer.target.v.s); + cJSON *revServer = cJSON_GetArrayItem(conf->dns.revServers.v.json, i); + + // Split comma-separated string into its components + char *copy = strdup(revServer->valuestring); + char *active = strtok(copy, ","); + char *cidr = strtok(NULL, ","); + char *target = strtok(NULL, ","); + char *domain = strtok(NULL, ","); + + // Skip inactive reverse servers + if(active != NULL && + strcmp(active, "true") != 0 && + strcmp(active, "1") != 0) + { + log_debug(DEBUG_CONFIG, "Skipping inactive reverse server: %s", revServer->valuestring); + free(copy); + continue; + } + + if(active == NULL || cidr == NULL || target == NULL || domain == NULL) + { + log_err("Invalid reverse server string: %s", revServer->valuestring); + free(copy); + continue; + } + + fprintf(pihole_conf, "# Reverse server setting (%u%s server)\n", + i+1, get_ordinal_suffix(i+1)); + fprintf(pihole_conf, "rev-server=%s,%s\n", cidr, target); // If we have a reverse domain, we forward all queries to this domain to // the same destination - if(strlen(conf->dns.revServer.domain.v.s) > 0) - fprintf(pihole_conf, "server=/%s/%s\n", - conf->dns.revServer.domain.v.s, conf->dns.revServer.target.v.s); + if(strlen(domain) > 0) + fprintf(pihole_conf, "server=/%s/%s\n", domain, target); // Forward unqualified names to the target only when the "never forward // non-FQDN" option is NOT ticked if(!conf->dns.domainNeeded.v.b) - fprintf(pihole_conf, "server=//%s\n", - conf->dns.revServer.target.v.s); + fprintf(pihole_conf, "server=//%s\n", target); fputs("\n", pihole_conf); + + // Free copy of string + free(copy); } // When there is a Pi-hole domain set and "Never forward non-FQDNs" is diff --git a/src/config/toml_reader.c b/src/config/toml_reader.c index 7bad6af9..34d4dadc 100644 --- a/src/config/toml_reader.c +++ b/src/config/toml_reader.c @@ -28,6 +28,48 @@ static toml_table_t *parseTOML(const unsigned int version); static void reportDebugFlags(void); +// Migrate config from old to new, returns true if a restart is required +static bool migrate_config(toml_table_t *toml, struct config *newconf) +{ + bool restart = false; + toml_table_t *dns = toml_table_in(toml, "dns"); + if(dns) + { + toml_table_t *revServer = toml_table_in(dns, "revServer"); + if(revServer) + { + // Read old config + toml_datum_t active = toml_bool_in(revServer, "active"); + toml_datum_t cidr = toml_string_in(revServer, "cidr"); + toml_datum_t target = toml_string_in(revServer, "target"); + toml_datum_t domain = toml_string_in(revServer, "domain"); + + // Necessary condition: all values must exist and CIDR and target must not be empty + if(active.ok && cidr.ok && target.ok && domain.ok && strlen(cidr.u.s) > 0 && strlen(target.u.s)) + { + // Build comma-separated string of all values + char *old = calloc((active.u.b ? 4 : 5) + strlen(cidr.u.s) + strlen(target.u.s) + strlen(domain.u.s) + 4, sizeof(char)); + if(old) + { + // Add to new config + sprintf(old, "%s,%s,%s,%s", active.u.s ? "true" : "false", cidr.u.s, target.u.s, domain.u.s); + log_debug(DEBUG_CONFIG, "Config setting dns.revServer MIGRATED: %s", old); + cJSON_AddItemToArray(newconf->dns.revServers.v.json, cJSON_CreateString(old)); + restart = true; + } + } + else + log_warn("Config setting dns.revServer INVALID - ignoring: %s %s %s %s", active.ok ? active.u.s : "NULL", cidr.ok ? cidr.u.s : "NULL", target.ok ? target.u.s : "NULL", domain.ok ? domain.u.s : "NULL"); + } + else + log_info("dns.revServer DOES NOT EXIST"); + } + else + log_info("dns DOES NOT EXIST"); + + return restart; +} + bool readFTLtoml(struct config *oldconf, struct config *newconf, toml_table_t *toml, const bool verbose, bool *restart, const unsigned int version) @@ -124,6 +166,10 @@ bool readFTLtoml(struct config *oldconf, struct config *newconf, } } + // Migrate config from old to new + if(migrate_config(toml, newconf) && restart != NULL) + *restart = true; + // Report debug config if enabled set_debug_flags(newconf); if(verbose) diff --git a/src/setupVars.c b/src/setupVars.c index e0040571..7414e3cd 100644 --- a/src/setupVars.c +++ b/src/setupVars.c @@ -119,6 +119,83 @@ static void get_conf_bool_from_setupVars(const char *key, struct conf_item *conf key, conf_item->k, conf_item->v.b ? "true" : "false"); } +static void get_revServer_from_setupVars(void) +{ + bool active = false; + char *cidr = NULL; + char *target = NULL; + char *domain = NULL; + const char *active_str = read_setupVarsconf("REV_SERVER"); + if(active_str == NULL) + { + // Do not change default value, this value is not set in setupVars.conf + log_debug(DEBUG_CONFIG, "setupVars.conf:REV_SERVER -> Not set"); + + // Free memory, harmless to call if read_setupVarsconf() didn't return a result + clearSetupVarsArray(); + return; + } + else + { + // Parameter present in setupVars.conf + active = getSetupVarsBool(active_str); + } + + // Free memory, harmless to call if read_setupVarsconf() didn't return a result + clearSetupVarsArray(); + + char *cidr_str = read_setupVarsconf("REV_SERVER_CIDR"); + if(cidr_str != NULL) + { + cidr = strdup(cidr_str); + trim_whitespace(cidr); + } + + // Free memory, harmless to call if read_setupVarsconf() didn't return a result + clearSetupVarsArray(); + + char *target_str = read_setupVarsconf("REV_SERVER_TARGET"); + if(target_str != NULL) + { + target = strdup(target_str); + trim_whitespace(target); + } + + // Free memory, harmless to call if read_setupVarsconf() didn't return a result + clearSetupVarsArray(); + + char *domain_str = read_setupVarsconf("REV_SERVER_DOMAIN"); + if(domain_str != NULL) + { + domain = strdup(domain_str); + trim_whitespace(domain); + } + + // Free memory, harmless to call if read_setupVarsconf() didn't return a result + clearSetupVarsArray(); + + if(active && cidr != NULL && target != NULL && domain != NULL) + { + // Build comma-separated string of all values + char *old = calloc(strlen(active_str) + strlen(cidr) + strlen(target) + strlen(domain) + 4, sizeof(char)); + if(old) + { + // Add to new config + sprintf(old, "%s,%s,%s,%s", active_str, cidr, target, domain); + cJSON_AddItemToArray(config.dns.revServers.v.json, cJSON_CreateString(old)); + free(old); + } + } + + // Free memory + if(cidr != NULL) + free(cidr); + if(target != NULL) + free(target); + if(domain != NULL) + free(domain); +} + static void get_conf_string_array_from_setupVars(const char *key, struct conf_item *conf_item) { // Verify we are allowed to use this function @@ -416,10 +493,7 @@ void importsetupVarsConf(void) get_conf_listeningMode_from_setupVars(); // Try to obtain REV_SERVER settings - get_conf_bool_from_setupVars("REV_SERVER", &config.dns.revServer.active); - get_conf_string_from_setupVars("REV_SERVER_CIDR", &config.dns.revServer.cidr); - get_conf_string_from_setupVars("REV_SERVER_TARGET", &config.dns.revServer.target); - get_conf_string_from_setupVars("REV_SERVER_DOMAIN", &config.dns.revServer.domain); + get_revServer_from_setupVars(); // Try to obtain DHCP settings get_conf_bool_from_setupVars("DHCP_ACTIVE", &config.dhcp.active); diff --git a/test/pihole.toml b/test/pihole.toml index 46909b67..c040c46a 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -206,6 +206,28 @@ # Port used by the DNS server port = 53 + # Reverse server (former also called "conditional forwarding") feature + # Array of reverse servers each one in one of the following forms: + # ",[/],[#]," + # + # Individual components: + # + # : either "true" or "false" + # + # [/]: Address range for the reverse server feature in CIDR + # notation. If the prefix length is omitted, either 32 (IPv4) or 128 (IPv6) are + # substituted (exact address match). This is almost certainly not what you want here. + # Example: "192.168.0.0/24" for the range 192.168.0.1 - 192.168.0.255 + # + # [#]: Target server to be used for the reverse server feature + # Example: "192.168.0.1#53" + # + # : Domain used for the reverse server feature (e.g., "fritz.box") + # Example: "fritz.box" + # + # A valid line could look like this: "true,192.168.0.0/24,192.168.0.1,fritz.box" + revServers = [] + [dns.cache] # Cache size of the DNS server. Note that expiring cache entries naturally make room # for new insertions over time. Setting this number too high will have an adverse @@ -348,31 +370,6 @@ # ... in the set interval before rate-limiting? interval = 0 ### CHANGED, default = 60 - [dns.revServer] - # Is the reverse server (former also called "conditional forwarding") feature enabled? - active = false - - # Address range for the reverse server feature in CIDR notation. If the prefix length - # is omitted, either 32 (IPv4) or 128 (IPv6) are substitutet (exact address match). - # This is almost certainly not what you want here. - # - # Possible values are: - # [/], e.g., "192.168.0.0/24" for the range 192.168.0.1 - - # 192.168.0.255 - cidr = "" - - # Target server tp be used for the reverse server feature - # - # Possible values are: - # [#], e.g., "192.168.0.1" - target = "" - - # Domain used for the reverse server feature - # - # Possible values are: - # (e.g., "fritz.box") - domain = "" - [dhcp] # Is the embedded DHCP server enabled? active = false From af361c17b657fafdab631d7ad338699a5ada1fb2 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 1 Dec 2023 12:44:10 +0100 Subject: [PATCH 2/2] Add allowed values for dns.revServers Signed-off-by: DL6ER --- src/config/config.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/config/config.c b/src/config/config.c index f84175ec..0eafa173 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -585,7 +585,8 @@ void initConfig(struct config *conf) conf->dns.blocking.mode.d.blocking_mode = MODE_NULL; conf->dns.revServers.k = "dns.revServers"; - conf->dns.revServers.h = "Reverse server (former also called \"conditional forwarding\") feature\n Array of reverse servers each one in one of the following forms: \",[/],[#],\"\n\n Individual components:\n\n : either \"true\" or \"false\"\n\n [/]: Address range for the reverse server feature in CIDR notation. If the prefix length is omitted, either 32 (IPv4) or 128 (IPv6) are substituted (exact address match). This is almost certainly not what you want here.\n Example: \"192.168.0.0/24\" for the range 192.168.0.1 - 192.168.0.255\n\n [#]: Target server to be used for the reverse server feature\n Example: \"192.168.0.1#53\"\n\n : Domain used for the reverse server feature (e.g., \"fritz.box\")\n Example: \"fritz.box\"\n\n A valid line could look like this: \"true,192.168.0.0/24,192.168.0.1,fritz.box\""; + conf->dns.revServers.h = "Reverse server (former also called \"conditional forwarding\") feature\n Array of reverse servers each one in one of the following forms: \",[/],[#],\"\n\n Individual components:\n\n : either \"true\" or \"false\"\n\n [/]: Address range for the reverse server feature in CIDR notation. If the prefix length is omitted, either 32 (IPv4) or 128 (IPv6) are substituted (exact address match). This is almost certainly not what you want here.\n Example: \"192.168.0.0/24\" for the range 192.168.0.1 - 192.168.0.255\n\n [#]: Target server to be used for the reverse server feature\n Example: \"192.168.0.1#53\"\n\n : Domain used for the reverse server feature (e.g., \"fritz.box\")\n Example: \"fritz.box\""; + conf->dns.revServers.a = cJSON_CreateStringReference("array of reverse servers each one in one of the following forms: \",[/],[#],\", e.g., \"true,192.168.0.0/24,192.168.0.1,fritz.box\""); conf->dns.revServers.t = CONF_JSON_STRING_ARRAY; conf->dns.revServers.d.json = cJSON_CreateArray(); conf->dns.revServers.f = FLAG_RESTART_FTL;