From 3b103ae8aa494c843e83c7901f8dfe8f85f744d8 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Thu, 22 Dec 2022 23:19:05 +0000 Subject: [PATCH 01/10] If we hit a cache internal error, log the entry we failed to remove. This is code which should never run, but if it does, we now log information useful for debugging. Signed-off-by: DL6ER --- src/dnsmasq/cache.c | 180 ++++++++++++++++++++++++-------------------- 1 file changed, 98 insertions(+), 82 deletions(-) diff --git a/src/dnsmasq/cache.c b/src/dnsmasq/cache.c index 89c40f82e..d423f264d 100644 --- a/src/dnsmasq/cache.c +++ b/src/dnsmasq/cache.c @@ -29,6 +29,7 @@ static int bignames_left, hash_size; static void make_non_terminals(struct crec *source); static struct crec *really_insert(char *name, union all_addr *addr, unsigned short class, time_t now, unsigned long ttl, unsigned int flags); +static void dump_cache_entry(struct crec *cache, time_t now); /* type->string mapping: this is also used by the name-hash function as a mixing table. */ /* taken from https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml */ @@ -595,7 +596,7 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho struct crec *new, *target_crec = NULL; union bigname *big_name = NULL; int freed_all = (flags & F_REVERSE); - int free_avail = 0; + struct crec *free_avail = NULL; unsigned int target_uid; /* if previous insertion failed give up now. */ @@ -643,7 +644,7 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho /* Free entry at end of LRU list, use it. */ if (!(new->flags & (F_FORWARD | F_REVERSE))) - break; + break; /* End of LRU list is still in use: if we didn't scan all the hash chains for expired entries do that now. If we already tried that @@ -659,6 +660,8 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho if (!warned) { my_syslog(LOG_ERR, _("Internal error in cache.")); + /* Log the entry we tried to delete. */ + dump_cache_entry(free_avail, now); warned = 1; } insert_error = 1; @@ -668,13 +671,13 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho if (freed_all) { /* For DNSSEC records, uid holds class. */ - free_avail = 1; /* Must be free space now. */ + free_avail = new; /* Must be free space now. */ /* condition valid when stale-caching */ if (difftime(now, new->ttd) < 0) daemon->metrics[METRIC_DNS_CACHE_LIVE_FREED]++; - cache_scan_free(cache_get_name(new), &new->addr, new->uid, now, new->flags, NULL, NULL); + cache_scan_free(cache_get_name(new), &new->addr, new->uid, now, new->flags, NULL, NULL); } else { @@ -1767,6 +1770,95 @@ static char *sanitise(char *name) return name; } +static void dump_cache_entry(struct crec *cache, time_t now) +{ + (void)now; + static char *buff = NULL; + + char *p, *t = " "; + char *a = daemon->addrbuff, *n = cache_get_name(cache); + + /* String length is limited below */ + if (!buff && !(buff = whine_malloc(150))) + return; + + p = buff; + + *a = 0; + if (strlen(n) == 0 && !(cache->flags & F_REVERSE)) + n = ""; + p += sprintf(p, "%-30.30s ", sanitise(n)); + if ((cache->flags & F_CNAME) && !is_outdated_cname_pointer(cache)) + a = sanitise(cache_get_cname_target(cache)); + else if ((cache->flags & F_SRV) && !(cache->flags & F_NEG)) + { + int targetlen = cache->addr.srv.targetlen; + ssize_t len = sprintf(a, "%u %u %u ", cache->addr.srv.priority, + cache->addr.srv.weight, cache->addr.srv.srvport); + + if (targetlen > (40 - len)) + targetlen = 40 - len; + blockdata_retrieve(cache->addr.srv.target, targetlen, a + len); + a[len + targetlen] = 0; + } +#ifdef HAVE_DNSSEC + else if (cache->flags & F_DS) + { + if (!(cache->flags & F_NEG)) + sprintf(a, "%5u %3u %3u", cache->addr.ds.keytag, + cache->addr.ds.algo, cache->addr.ds.digest); + } + else if (cache->flags & F_DNSKEY) + sprintf(a, "%5u %3u %3u", cache->addr.key.keytag, + cache->addr.key.algo, cache->addr.key.flags); +#endif + else if (!(cache->flags & F_NEG) || !(cache->flags & F_FORWARD)) + { + a = daemon->addrbuff; + if (cache->flags & F_IPV4) + inet_ntop(AF_INET, &cache->addr, a, ADDRSTRLEN); + else if (cache->flags & F_IPV6) + inet_ntop(AF_INET6, &cache->addr, a, ADDRSTRLEN); + } + + if (cache->flags & F_IPV4) + t = "4"; + else if (cache->flags & F_IPV6) + t = "6"; + else if (cache->flags & F_CNAME) + t = "C"; + else if (cache->flags & F_SRV) + t = "V"; +#ifdef HAVE_DNSSEC + else if (cache->flags & F_DS) + t = "S"; + else if (cache->flags & F_DNSKEY) + t = "K"; +#endif + else /* non-terminal */ + t = "!"; + + p += sprintf(p, "%-40.40s %s%s%s%s%s%s%s%s%s%s ", a, t, + cache->flags & F_FORWARD ? "F" : " ", + cache->flags & F_REVERSE ? "R" : " ", + cache->flags & F_IMMORTAL ? "I" : " ", + cache->flags & F_DHCP ? "D" : " ", + cache->flags & F_NEG ? "N" : " ", + cache->flags & F_NXDOMAIN ? "X" : " ", + cache->flags & F_HOSTS ? "H" : " ", + cache->flags & F_CONFIG ? "C" : " ", + cache->flags & F_DNSSECOK ? "V" : " "); +#ifdef HAVE_BROKEN_RTC + p += sprintf(p, "%-24lu", cache->flags & F_IMMORTAL ? 0: (unsigned long)(cache->ttd - now)); +#else + p += sprintf(p, "%-24.24s", cache->flags & F_IMMORTAL ? "" : ctime(&(cache->ttd))); +#endif + if(cache->flags & (F_HOSTS | F_CONFIG) && cache->uid > 0) + p += sprintf(p, " %-40.40s", record_source(cache->uid)); + + my_syslog(LOG_INFO, "%s", buff); +} + /***************** Pi-hole modification *****************/ void get_dnsmasq_cache_info(struct cache_info *ci) { @@ -1847,90 +1939,14 @@ void dump_cache(time_t now) if (option_bool(OPT_DEBUG) || option_bool(OPT_LOG)) { - struct crec *cache ; + struct crec *cache; int i; my_syslog(LOG_INFO, "Host Address Flags Expires Source"); my_syslog(LOG_INFO, "------------------------------ ---------------------------------------- ---------- ------------------------ ------------"); for (i=0; ihash_next) - { - char *t = " "; - char *a = daemon->addrbuff, *p = daemon->namebuff, *n = cache_get_name(cache); - *a = 0; - if (strlen(n) == 0 && !(cache->flags & F_REVERSE)) - n = ""; - p += sprintf(p, "%-30.30s ", sanitise(n)); - if ((cache->flags & F_CNAME) && !is_outdated_cname_pointer(cache)) - a = sanitise(cache_get_cname_target(cache)); - else if ((cache->flags & F_SRV) && !(cache->flags & F_NEG)) - { - int targetlen = cache->addr.srv.targetlen; - ssize_t len = sprintf(a, "%u %u %u ", cache->addr.srv.priority, - cache->addr.srv.weight, cache->addr.srv.srvport); - - if (targetlen > (40 - len)) - targetlen = 40 - len; - blockdata_retrieve(cache->addr.srv.target, targetlen, a + len); - a[len + targetlen] = 0; - } -#ifdef HAVE_DNSSEC - else if (cache->flags & F_DS) - { - if (!(cache->flags & F_NEG)) - sprintf(a, "%5u %3u %3u", cache->addr.ds.keytag, - cache->addr.ds.algo, cache->addr.ds.digest); - } - else if (cache->flags & F_DNSKEY) - sprintf(a, "%5u %3u %3u", cache->addr.key.keytag, - cache->addr.key.algo, cache->addr.key.flags); -#endif - else if (!(cache->flags & F_NEG) || !(cache->flags & F_FORWARD)) - { - a = daemon->addrbuff; - if (cache->flags & F_IPV4) - inet_ntop(AF_INET, &cache->addr, a, ADDRSTRLEN); - else if (cache->flags & F_IPV6) - inet_ntop(AF_INET6, &cache->addr, a, ADDRSTRLEN); - } - - if (cache->flags & F_IPV4) - t = "4"; - else if (cache->flags & F_IPV6) - t = "6"; - else if (cache->flags & F_CNAME) - t = "C"; - else if (cache->flags & F_SRV) - t = "V"; -#ifdef HAVE_DNSSEC - else if (cache->flags & F_DS) - t = "S"; - else if (cache->flags & F_DNSKEY) - t = "K"; -#endif - else /* non-terminal */ - t = "!"; - - p += sprintf(p, "%-40.40s %s%s%s%s%s%s%s%s%s%s ", a, t, - cache->flags & F_FORWARD ? "F" : " ", - cache->flags & F_REVERSE ? "R" : " ", - cache->flags & F_IMMORTAL ? "I" : " ", - cache->flags & F_DHCP ? "D" : " ", - cache->flags & F_NEG ? "N" : " ", - cache->flags & F_NXDOMAIN ? "X" : " ", - cache->flags & F_HOSTS ? "H" : " ", - cache->flags & F_CONFIG ? "C" : " ", - cache->flags & F_DNSSECOK ? "V" : " "); -#ifdef HAVE_BROKEN_RTC - p += sprintf(p, "%-24lu", cache->flags & F_IMMORTAL ? 0: (unsigned long)(cache->ttd - now)); -#else - p += sprintf(p, "%-24.24s", cache->flags & F_IMMORTAL ? "" : ctime(&(cache->ttd))); -#endif - if(cache->flags & (F_HOSTS | F_CONFIG) && cache->uid > 0) - p += sprintf(p, " %s", record_source(cache->uid)); - - my_syslog(LOG_INFO, "%s", daemon->namebuff); - } + dump_cache_entry(cache, now); } } From 21cec0c01dbefcd0cacf6e8b4be7f486112ca0c4 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Mon, 2 Jan 2023 22:17:57 +0000 Subject: [PATCH 02/10] Log all cache internal errors. Signed-off-by: DL6ER --- src/dnsmasq/cache.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/dnsmasq/cache.c b/src/dnsmasq/cache.c index d423f264d..1c8b8f0ba 100644 --- a/src/dnsmasq/cache.c +++ b/src/dnsmasq/cache.c @@ -656,14 +656,9 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho insert. Once in this state, all inserts will probably fail. */ if (free_avail) { - static int warned = 0; - if (!warned) - { - my_syslog(LOG_ERR, _("Internal error in cache.")); - /* Log the entry we tried to delete. */ - dump_cache_entry(free_avail, now); - warned = 1; - } + my_syslog(LOG_ERR, _("Internal error in cache.")); + /* Log the entry we tried to delete. */ + dump_cache_entry(free_avail, now); insert_error = 1; return NULL; } From 45a760b3f8e5485fe04c88da978404d1f3b3cdb9 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 4 Jan 2023 23:10:07 +0000 Subject: [PATCH 03/10] Fix cosmetic big in dump_cache_entry() Signed-off-by: DL6ER --- src/dnsmasq/cache.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/dnsmasq/cache.c b/src/dnsmasq/cache.c index 1c8b8f0ba..5ad39a2b2 100644 --- a/src/dnsmasq/cache.c +++ b/src/dnsmasq/cache.c @@ -1079,7 +1079,6 @@ void add_hosts_entry(struct crec *cache, union all_addr *addr, int addrlen, the array rhash, hashed on address. Note that rhash and the values in ->next are only valid whilst reading hosts files: the buckets are then freed, and the ->next pointer used for other things. - Only insert each unique address once into this hashing structure. This complexity avoids O(n^2) divergent CPU use whilst reading @@ -1830,7 +1829,7 @@ static void dump_cache_entry(struct crec *cache, time_t now) else if (cache->flags & F_DNSKEY) t = "K"; #endif - else /* non-terminal */ + else if (!(cache->flags & F_NXDOMAIN)) /* non-terminal */ t = "!"; p += sprintf(p, "%-40.40s %s%s%s%s%s%s%s%s%s%s ", a, t, From 33c04059f4452236fd07f8023dd72f2228dc3ad1 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 11 Jan 2023 23:23:40 +0000 Subject: [PATCH 04/10] Fix bug which can break the invariants on the order of a hash chain. If there are multiple cache records with the same name but different F_REVERSE and/or F_IMMORTAL flags, the code added in fe9a134b could concievable break the REVERSE-FORWARD-IMMORTAL order invariant. Reproducing this is damn near impossible, but it is responsible for rare and otherwise inexplicable reversion between 2.87 and 2.88 which manifests itself as a cache internal error. All observed cases have depended on DNSSEC being enabled, but the bug could in theory manifest itself without DNSSEC Thanks to Timo van Roermund for reporting the bug and huge efforts to isolate it. Signed-off-by: DL6ER --- src/dnsmasq/cache.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/dnsmasq/cache.c b/src/dnsmasq/cache.c index 5ad39a2b2..0816cb545 100644 --- a/src/dnsmasq/cache.c +++ b/src/dnsmasq/cache.c @@ -237,19 +237,23 @@ static void cache_hash(struct crec *crecp) char *name = cache_get_name(crecp); struct crec **up = hash_bucket(name); - - if (!(crecp->flags & F_REVERSE)) + unsigned int flags = crecp->flags & (F_IMMORTAL | F_REVERSE); + + if (!(flags & F_REVERSE)) { while (*up && ((*up)->flags & F_REVERSE)) up = &((*up)->hash_next); - if (crecp->flags & F_IMMORTAL) + if (flags & F_IMMORTAL) while (*up && !((*up)->flags & F_IMMORTAL)) up = &((*up)->hash_next); } - /* Preserve order when inserting the same name multiple times. */ - while (*up && hostname_isequal(cache_get_name(*up), name)) + /* Preserve order when inserting the same name multiple times. + Do not mess up the flag invariants. */ + while (*up && + hostname_isequal(cache_get_name(*up), name) && + flags == ((*up)->flags & (F_IMMORTAL | F_REVERSE))) up = &((*up)->hash_next); crecp->hash_next = *up; From ffa4d338f16aeab7f7055c17b107187357b77586 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 15 Jan 2023 13:00:40 +0100 Subject: [PATCH 05/10] Allow selection of multiple query types in regex extension, like "abcabc;querytype=HTTPS,SVCB" Signed-off-by: DL6ER --- src/dnsmasq_interface.c | 2 +- src/regex.c | 81 +++++++++++++++++++++++++---------------- src/regex_r.h | 3 +- test/test_suite.bats | 14 +++++-- 4 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 49c8bc872..608801b8a 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -3347,7 +3347,7 @@ int check_struct_sizes(void) result += check_one_struct("DNSCacheData", sizeof(DNSCacheData), 16, 16); result += check_one_struct("ednsData", sizeof(ednsData), 72, 72); result += check_one_struct("overTimeData", sizeof(overTimeData), 32, 24); - result += check_one_struct("regexData", sizeof(regexData), 56, 44); + result += check_one_struct("regexData", sizeof(regexData), 64, 48); result += check_one_struct("SharedMemory", sizeof(SharedMemory), 24, 12); result += check_one_struct("ShmSettings", sizeof(ShmSettings), 16, 16); result += check_one_struct("countersStruct", sizeof(countersStruct), 248, 248); diff --git a/src/regex.c b/src/regex.c index e6753023d..4388ff85a 100644 --- a/src/regex.c +++ b/src/regex.c @@ -26,6 +26,11 @@ // cli_stuff() #include "args.h" +// Safety-measure for future extensions +#if TYPE_MAX > 30 +#error "Too many query types to be handled by a 32-bit integer" +#endif + const char *regextype[REGEX_MAX] = { "blacklist", "whitelist", "CLI" }; static regexData *white_regex = NULL; @@ -164,38 +169,48 @@ static bool compile_regex(const char *regexin, const enum regex_type regexid, co "Overwriting previous querytype setting", dbidx, regexin); - // Test input string against all implemented query types - for(enum query_types type = TYPE_A; type < TYPE_MAX; type++) + // Split input string into individual query types + char *saveptr2 = NULL; + char *token = strtok_r(extra, ",", &saveptr2); + while(token != NULL) { - // Check for querytype - if(strcasecmp(extra, querytypes[type]) == 0) + // Test input string against all implemented query types + for(enum query_types type = TYPE_A; type < TYPE_MAX; type++) { - regex[index].ext.query_type = type; - regex[index].ext.query_type_inverted = false; - break; + // Check for querytype + if(strcasecmp(token, querytypes[type]) == 0) + { + regex[index].ext.query_type ^= 1 << type; + break; + } + // Check for INVERTED querytype + else if(token[0] == '!' && strcasecmp(token + 1u, querytypes[type]) == 0) + { + regex[index].ext.query_type ^= ~(1 << type); + break; + } } - // Check for INVERTED querytype - else if(extra[0] == '!' && strcasecmp(extra + 1u, querytypes[type]) == 0) + // Check if we found a valid query type + if(regex[index].ext.query_type == 0) { - regex[index].ext.query_type = type; - regex[index].ext.query_type_inverted = true; - break; + logg_regex_warning(regextype[regexid], + "Unknown query type", + dbidx, regexin); + free(buf); + return false; } - } - // Nothing found - if(regex[index].ext.query_type == 0) - { - char msg[64] = { 0 }; - snprintf(msg, sizeof(msg), "Unknown querytype \"%s\"", extra); - logg_regex_warning(regextype[regexid], msg, dbidx, regexin); - } - // Debug output - else if(config.debug & DEBUG_REGEX) + // Get next token + token = strtok_r(NULL, ",", &saveptr2); + } + if(regex[index].ext.query_type != 0) { - logg(" This regex will %s match query type %s", - regex[index].ext.query_type_inverted ? "NOT" : "ONLY", - querytypes[regex[index].ext.query_type]); + logg(" Hint: This regex matches only specific query types:"); + for(int i = TYPE_A; i < TYPE_MAX; i++) + { + if(regex[index].ext.query_type & (1 << i)) + logg(" - %s", querytypes[i]); + } } } // option: ";invert" @@ -380,15 +395,14 @@ static int match_regex(const char *input, DNSCacheData* dns_cache, const int cli // Check query type filtering if(regex[index].ext.query_type != 0) { - if((!regex[index].ext.query_type_inverted && regex[index].ext.query_type != dns_cache->query_type) || - (regex[index].ext.query_type_inverted && regex[index].ext.query_type == dns_cache->query_type)) + if(!(regex[index].ext.query_type & (1 << dns_cache->query_type))) { if(config.debug & DEBUG_REGEX) { logg("Regex %s (%u, DB ID %i) NO match: \"%s\" vs. \"%s\"" - " (skipped because of query type %smatch)", + " (skipped because of query type mismatch)", regextype[regexid], index, regex[index].database_id, - input, regex[index].string, regex[index].ext.query_type_inverted ? "inversion " : "mis"); + input, regex[index].string); } continue; } @@ -436,9 +450,12 @@ static int match_regex(const char *input, DNSCacheData* dns_cache, const int cli // Check query type filtering if(regex[index].ext.query_type != 0) { - logg(" Hint: This regex %s type %s queries", - regex[index].ext.query_type_inverted ? "does not match" : "matches only", - querytypes[regex[index].ext.query_type]); + logg(" Hint: This regex matches only specific query types:"); + for(int i = TYPE_A; i < TYPE_MAX; i++) + { + if(regex[index].ext.query_type & (1 << i)) + logg(" - %s", querytypes[i]); + } } // Check inversion diff --git a/src/regex_r.h b/src/regex_r.h index 7ec979f7d..7be349b6d 100644 --- a/src/regex_r.h +++ b/src/regex_r.h @@ -30,13 +30,12 @@ typedef struct { bool available :1; struct { bool inverted :1; - bool query_type_inverted :1; bool custom_ip4 :1; bool custom_ip6 :1; - enum query_types query_type; enum reply_type reply; struct in_addr addr4; struct in6_addr addr6; + uint32_t query_type; } ext; int database_id; char *string; diff --git a/test/test_suite.bats b/test/test_suite.bats index d5e73b3f2..a899cf4df 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -998,7 +998,7 @@ run bash -c './pihole-FTL regex-test "f" g\;querytype=!A\;querytype=A' printf "%s\n" "${lines[@]}" [[ $status == 2 ]] - [[ ${lines[1]} == *"Overwriting previous querytype setting" ]] + [[ "${lines[@]}" == *"Overwriting previous querytype setting"* ]] } @test "Regex Test 41: Option \"^;reply=NXDOMAIN\" working as expected" { @@ -1050,14 +1050,14 @@ run bash -c './pihole-FTL regex-test "f" f\;querytype=A' printf "%s\n" "${lines[@]}" [[ $status == 0 ]] - [[ ${lines[4]} == " Hint: This regex matches only type A queries" ]] + [[ "${lines[@]}" == *"- A"* ]] } @test "Regex Test 48: Option \";querytype=!TXT\" reported on CLI" { run bash -c './pihole-FTL regex-test "f" f\;querytype=!TXT' printf "%s\n" "${lines[@]}" [[ $status == 0 ]] - [[ ${lines[4]} == " Hint: This regex does not match type TXT queries" ]] + [[ "${lines[@]}" != *"- TXT"* ]] } @test "Regex Test 49: Option \";reply=NXDOMAIN\" reported on CLI" { @@ -1074,6 +1074,14 @@ [[ ${lines[4]} == " Hint: This regex is inverted" ]] } +@test "Regex Test 51: Option \";querytype=A,HTTPS\" reported on CLI" { + run bash -c './pihole-FTL regex-test "f" f\;querytype=A,HTTPS' + printf "%s\n" "${lines[@]}" + [[ $status == 0 ]] + [[ "${lines[@]}" == *"- A"* ]] + [[ "${lines[@]}" == *"- HTTPS"* ]] +} + # x86_64-musl is built on busybox which has a slightly different # variant of ls displaying three, instead of one, spaces between the # user and group names. From 8b9e6c6c7e904bf4e2ee06719411946be3015b9e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 28 Jan 2023 13:34:09 +0100 Subject: [PATCH 06/10] Print regex type hints only in debug mode Signed-off-by: DL6ER --- src/regex.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/regex.c b/src/regex.c index 4388ff85a..ad0f5ea47 100644 --- a/src/regex.c +++ b/src/regex.c @@ -203,7 +203,7 @@ static bool compile_regex(const char *regexin, const enum regex_type regexid, co // Get next token token = strtok_r(NULL, ",", &saveptr2); } - if(regex[index].ext.query_type != 0) + if(regex[index].ext.query_type != 0 && config.debug & DEBUG_REGEX) { logg(" Hint: This regex matches only specific query types:"); for(int i = TYPE_A; i < TYPE_MAX; i++) From 1b62122a8c588e54d829a80f124c3ebaabe49bed Mon Sep 17 00:00:00 2001 From: Dominik Derigs Date: Mon, 23 Jan 2023 22:48:01 +0000 Subject: [PATCH 07/10] Add --no-ident option. Signed-off-by: DL6ER --- src/dnsmasq/dnsmasq.h | 3 ++- src/dnsmasq/option.c | 49 ++++++++++++++++++++++++------------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/dnsmasq/dnsmasq.h b/src/dnsmasq/dnsmasq.h index 9c9e825fe..2883b8d67 100644 --- a/src/dnsmasq/dnsmasq.h +++ b/src/dnsmasq/dnsmasq.h @@ -281,7 +281,8 @@ struct event_desc { #define OPT_STRIP_ECS 69 #define OPT_STRIP_MAC 70 #define OPT_NORR 71 -#define OPT_LAST 72 +#define OPT_NO_IDENT 72 +#define OPT_LAST 73 #define OPTION_BITS (sizeof(unsigned int)*8) #define OPTION_SIZE ( (OPT_LAST/OPTION_BITS)+((OPT_LAST%OPTION_BITS)!=0) ) diff --git a/src/dnsmasq/option.c b/src/dnsmasq/option.c index 0c61a22f6..96c094c61 100644 --- a/src/dnsmasq/option.c +++ b/src/dnsmasq/option.c @@ -189,6 +189,7 @@ struct myoption { #define LOPT_FAST_RETRY 376 #define LOPT_STALE_CACHE 377 #define LOPT_NORR 378 +#define LOPT_NO_IDENT 379 #ifdef HAVE_GETOPT_LONG static const struct option opts[] = @@ -378,6 +379,7 @@ static const struct myoption opts[] = { "port-limit", 1, 0, LOPT_RANDPORT_LIM }, { "fast-dns-retry", 2, 0, LOPT_FAST_RETRY }, { "use-stale-cache", 2, 0 , LOPT_STALE_CACHE }, + { "no-ident", 0, 0, LOPT_NO_IDENT }, { NULL, 0, 0, 0 } }; @@ -574,6 +576,7 @@ static struct { { LOPT_UMBRELLA, ARG_ONE, "[=]", gettext_noop("Send Cisco Umbrella identifiers including remote IP."), NULL }, { LOPT_QUIET_TFTP, OPT_QUIET_TFTP, NULL, gettext_noop("Do not log routine TFTP."), NULL }, { LOPT_NORR, OPT_NORR, NULL, gettext_noop("Suppress round-robin ordering of DNS records."), NULL }, + { LOPT_NO_IDENT, OPT_NO_IDENT, NULL, gettext_noop("Do not add CHAOS TXT records."), NULL }, { 0, 0, NULL, NULL, NULL } }; @@ -5761,27 +5764,6 @@ void read_opts(int argc, char **argv, char *compile_opts) daemon->randport_limit = 1; daemon->host_index = SRC_AH; -#ifndef NO_ID - add_txt("version.bind", "dnsmasq-" VERSION, 0 ); - add_txt("authors.bind", "Simon Kelley", 0); - add_txt("copyright.bind", COPYRIGHT, 0); - add_txt("cachesize.bind", NULL, TXT_STAT_CACHESIZE); - add_txt("insertions.bind", NULL, TXT_STAT_INSERTS); - add_txt("evictions.bind", NULL, TXT_STAT_EVICTIONS); - add_txt("misses.bind", NULL, TXT_STAT_MISSES); - add_txt("hits.bind", NULL, TXT_STAT_HITS); -#ifdef HAVE_AUTH - add_txt("auth.bind", NULL, TXT_STAT_AUTH); -#endif - add_txt("servers.bind", NULL, TXT_STAT_SERVERS); - /* Pi-hole modification */ - add_txt("privacylevel.pihole", NULL, TXT_PRIVACYLEVEL); - /************************/ -#endif - /******** Pi-hole modification ********/ - add_txt("version.FTL", (char*)get_FTL_version(), 0 ); - /**************************************/ - /* See comment above make_servers(). Optimises server-read code. */ mark_servers(0); @@ -5879,6 +5861,31 @@ void read_opts(int argc, char **argv, char *compile_opts) else one_file(CONFFILE, LOPT_CONF_OPT); + /* Add TXT records if wanted */ +#ifndef NO_ID + if (!option_bool(OPT_NO_IDENT)) + { + add_txt("version.bind", "dnsmasq-" VERSION, 0 ); + add_txt("authors.bind", "Simon Kelley", 0); + add_txt("copyright.bind", COPYRIGHT, 0); + add_txt("cachesize.bind", NULL, TXT_STAT_CACHESIZE); + add_txt("insertions.bind", NULL, TXT_STAT_INSERTS); + add_txt("evictions.bind", NULL, TXT_STAT_EVICTIONS); + add_txt("misses.bind", NULL, TXT_STAT_MISSES); + add_txt("hits.bind", NULL, TXT_STAT_HITS); +#ifdef HAVE_AUTH + add_txt("auth.bind", NULL, TXT_STAT_AUTH); +#endif + add_txt("servers.bind", NULL, TXT_STAT_SERVERS); + /* Pi-hole modification */ + add_txt("privacylevel.pihole", NULL, TXT_PRIVACYLEVEL); + /************************/ + } +#endif + /******** Pi-hole modification ********/ + add_txt("version.FTL", (char*)get_FTL_version(), 0 ); + /**************************************/ + /* port might not be known when the address is parsed - fill in here */ if (daemon->servers) { From 49e1c74455ff3523033f16f620f85b4ddf59bbd1 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 28 Jan 2023 15:28:36 +0100 Subject: [PATCH 08/10] New syntax: querytype=A accepts now also a list (like querytype=A,AAAA,MX). You can use the exclamation mark as before for inversion (querytype=!A) matches everything BUT type A queries. This has now been extended to be able to invert a list, too (like (querytype=!A,AAAA matches everything BUT A and AAAA queries) Signed-off-by: DL6ER --- src/regex.c | 34 +++++++++++++++++++++++++--------- test/gravity.db.sql | 4 +++- test/pdns/setup.sh | 10 ++++++++++ test/test_suite.bats | 44 ++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 78 insertions(+), 14 deletions(-) diff --git a/src/regex.c b/src/regex.c index ad0f5ea47..e938ff8c1 100644 --- a/src/regex.c +++ b/src/regex.c @@ -159,9 +159,13 @@ static bool compile_regex(const char *regexin, const enum regex_type regexid, co // Analyze FTL-specific parts while((part = strtok_r(NULL, FTL_REGEX_SEP, &saveptr)) != NULL) { - char extra[17] = { 0 }; - // options ";querytype=!AAAA" and ";querytype=AAAA" - if(sscanf(part, "querytype=%16s", extra)) + char extra[64] = { 0 }; + // options like + // - ";querytype=A" (only match type A queries) + // - ";querytype=!AAAA" (match everything but AAAA queries) + // - ";querytype=A,AAAA" (match only A and AAAA queries) + // - ";querytype=!A,AAAA" (match everything but A and AAAA queries) + if(sscanf(part, "querytype=%63s", extra)) { // Warn if specified more than one querytype option if(regex[index].ext.query_type != 0) @@ -169,6 +173,19 @@ static bool compile_regex(const char *regexin, const enum regex_type regexid, co "Overwriting previous querytype setting", dbidx, regexin); + // Check if the first letter is a "!" + // This means that the query type matching is inverted + bool inverted = false; + if(extra[0] == '!') + { + // Set inverted mode (will be applied + // after parsing all query types) + inverted = true; + + // Remove the "!" from the string + memmove(extra, extra+1, strlen(extra)); + } + // Split input string into individual query types char *saveptr2 = NULL; char *token = strtok_r(extra, ",", &saveptr2); @@ -183,12 +200,6 @@ static bool compile_regex(const char *regexin, const enum regex_type regexid, co regex[index].ext.query_type ^= 1 << type; break; } - // Check for INVERTED querytype - else if(token[0] == '!' && strcasecmp(token + 1u, querytypes[type]) == 0) - { - regex[index].ext.query_type ^= ~(1 << type); - break; - } } // Check if we found a valid query type if(regex[index].ext.query_type == 0) @@ -203,6 +214,11 @@ static bool compile_regex(const char *regexin, const enum regex_type regexid, co // Get next token token = strtok_r(NULL, ",", &saveptr2); } + + // Invert query types if requested + if(inverted) + regex[index].ext.query_type = ~regex[index].ext.query_type; + if(regex[index].ext.query_type != 0 && config.debug & DEBUG_REGEX) { logg(" Hint: This regex matches only specific query types:"); diff --git a/test/gravity.db.sql b/test/gravity.db.sql index c317a2dc8..22133c848 100644 --- a/test/gravity.db.sql +++ b/test/gravity.db.sql @@ -208,9 +208,11 @@ INSERT INTO domainlist VALUES(11,3,'^regex-REPLYv6$;reply=fe80::1234',1,15599288 INSERT INTO domainlist VALUES(12,3,'^regex-REPLYv46$;reply=1.2.3.4;reply=fe80::1234',1,1559928803,1559928803,''); INSERT INTO domainlist VALUES(13,3,'^regex-A$;querytype=A',1,1559928803,1559928803,''); INSERT INTO domainlist VALUES(14,3,'^regex-notA$;querytype=!A',1,1559928803,1559928803,''); +INSERT INTO domainlist VALUES(15,3,'^regex-multiple.ftl$;querytype=ANY,HTTPS,SVCB;reply=refused',1,1559928803,1559928803,''); +INSERT INTO domainlist VALUES(16,3,'^regex-notMultiple.ftl$;querytype=!ANY,HTTPS,SVCB;reply=refused',1,1559928803,1559928803,''); /* Other special domains */ -INSERT INTO domainlist VALUES(15,1,'blacklisted-group-disabled.com',1,1559928803,1559928803,'Entry disabled by a group'); +INSERT INTO domainlist VALUES(17,1,'blacklisted-group-disabled.com',1,1559928803,1559928803,'Entry disabled by a group'); INSERT INTO adlist VALUES(1,'https://hosts-file.net/ad_servers.txt',1,1559928803,1559928803,'Migrated from /etc/pihole/adlists.list',1559928803,2000,2,1); diff --git a/test/pdns/setup.sh b/test/pdns/setup.sh index ea53a51b2..390b90733 100644 --- a/test/pdns/setup.sh +++ b/test/pdns/setup.sh @@ -98,9 +98,19 @@ pdnsutil add-record ftl. mx MX "50 ns1.ftl." # SVCB + HTTPS pdnsutil add-record ftl. svcb SVCB '1 port="80"' +pdnsutil add-record ftl. regex-multiple SVCB '1 port="80"' +pdnsutil add-record ftl. regex-notMultiple SVCB '1 port="80"' # HTTPS pdnsutil add-record ftl. https HTTPS '1 . alpn="h3,h2"' +pdnsutil add-record ftl. regex-multiple HTTPS '1 . alpn="h3,h2"' +pdnsutil add-record ftl. regex-notMultiple HTTPS '1 . alpn="h3,h2"' + +# ANY +pdnsutil add-record ftl. regex-multiple A 192.168.3.12 +pdnsutil add-record ftl. regex-multiple AAAA fe80::3f41 +pdnsutil add-record ftl. regex-notMultiple A 192.168.3.12 +pdnsutil add-record ftl. regex-notMultiple AAAA fe80::3f41 # Create reverse lookup zone pdnsutil create-zone arpa ns1.ftl diff --git a/test/test_suite.bats b/test/test_suite.bats index a899cf4df..e143633e5 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -58,7 +58,7 @@ @test "Number of compiled regex filters as expected" { run bash -c 'grep "Compiled [0-9]* whitelist" /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" - [[ ${lines[0]} == *"Compiled 2 whitelist and 9 blacklist regex filters"* ]] + [[ ${lines[0]} == *"Compiled 2 whitelist and 11 blacklist regex filters"* ]] } @test "Blacklisted domain is blocked" { @@ -196,7 +196,7 @@ [[ ${lines[0]} == "2" ]] run bash -c "grep -c 'Regex blacklist ([[:digit:]]*, DB ID [[:digit:]]*) .* NOT ENABLED for client 127.0.0.4' /var/log/pihole/FTL.log" printf "%s\n" "${lines[@]}" - [[ ${lines[0]} == "9" ]] + [[ ${lines[0]} == "11" ]] } @test "Client 5: Client is recognized by MAC address" { @@ -228,7 +228,7 @@ [[ ${lines[0]} == "2" ]] run bash -c "grep -c 'Regex blacklist ([[:digit:]]*, DB ID [[:digit:]]*) .* NOT ENABLED for client 127.0.0.5' /var/log/pihole/FTL.log" printf "%s\n" "${lines[@]}" - [[ ${lines[0]} == "9" ]] + [[ ${lines[0]} == "11" ]] } @test "Client 6: Client is recognized by interface name" { @@ -266,7 +266,7 @@ [[ ${lines[0]} == "2" ]] run bash -c "grep -c 'Regex blacklist ([[:digit:]]*, DB ID [[:digit:]]*) .* NOT ENABLED for client 127.0.0.6' /var/log/pihole/FTL.log" printf "%s\n" "${lines[@]}" - [[ ${lines[0]} == "9" ]] + [[ ${lines[0]} == "11" ]] } @test "Normal query (A) is not blocked" { @@ -1082,6 +1082,42 @@ [[ "${lines[@]}" == *"- HTTPS"* ]] } +@test "Regex Test 52: Option \";querytype=ANY,HTTPS,SVCB;reply=refused\" working as expected (ONLY matching ANY, HTTPS or SVCB queries)" { + run bash -c 'dig A regex-multiple.ftl @127.0.0.1' + printf "dig A: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: NOERROR"* ]] + run bash -c 'dig AAAA regex-multiple.ftl @127.0.0.1' + printf "dig AAAA: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: NOERROR"* ]] + run bash -c 'dig SVCB regex-multiple.ftl @127.0.0.1' + printf "dig SVCB: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: REFUSED"* ]] + run bash -c 'dig HTTPS regex-multiple.ftl @127.0.0.1' + printf "dig HTTPS: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: REFUSED"* ]] + run bash -c 'dig ANY regex-multiple.ftl @127.0.0.1' + printf "dig ANY: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: REFUSED"* ]] +} + +@test "Regex Test 53: Option \";querytype=!ANY,HTTPS,SVCB;reply=refused\" working as expected (NOT matching ANY, HTTPS or SVCB queries)" { + run bash -c 'dig A regex-notMultiple.ftl @127.0.0.1' + printf "dig A: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: REFUSED"* ]] + run bash -c 'dig AAAA regex-notMultiple.ftl @127.0.0.1' + printf "dig AAAA: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: REFUSED"* ]] + run bash -c 'dig SVCB regex-notMultiple.ftl @127.0.0.1' + printf "dig SVCB: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: NOERROR"* ]] + run bash -c 'dig HTTPS regex-notMultiple.ftl @127.0.0.1' + printf "dig HTTPS: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: NOERROR"* ]] + run bash -c 'dig ANY regex-notMultiple.ftl @127.0.0.1' + printf "dig ANY: %s\n" "${lines[@]}" + [[ "${lines[@]}" == *"status: NOERROR"* ]] +} + # x86_64-musl is built on busybox which has a slightly different # variant of ls displaying three, instead of one, spaces between the # user and group names. From f493af3e430a0589536c728dacffdb7fb98ff226 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 27 Jan 2023 16:38:24 +0100 Subject: [PATCH 09/10] Update dnsmasq version to 2.89rc1 Signed-off-by: DL6ER --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 76f7c68fc..1d049915c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,6 @@ cmake_minimum_required(VERSION 2.8.12) project(PIHOLE_FTL C) -set(DNSMASQ_VERSION pi-hole-v2.88) +set(DNSMASQ_VERSION pi-hole-v2.89rc1) add_subdirectory(src) From 6edb071f5407ec74b405eb3e6ddf8a0180c68398 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 7 Feb 2023 19:12:00 +0100 Subject: [PATCH 10/10] Update dnsmasq version to 2.89 Signed-off-by: DL6ER --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d049915c..d468618a4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,6 @@ cmake_minimum_required(VERSION 2.8.12) project(PIHOLE_FTL C) -set(DNSMASQ_VERSION pi-hole-v2.89rc1) +set(DNSMASQ_VERSION pi-hole-v2.89) add_subdirectory(src)