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..e938ff8c1 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; @@ -154,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) @@ -164,38 +173,60 @@ 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++) + // 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); + 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(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); + + // Get next token + token = strtok_r(NULL, ",", &saveptr2); } - // Debug output - else if(config.debug & DEBUG_REGEX) + // 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(" 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 +411,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 +466,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/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 d5e73b3f2..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" { @@ -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,50 @@ [[ ${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"* ]] +} + +@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.