Skip to content

Commit

Permalink
Queries blocked upstream with a known blocking page IP should not be …
Browse files Browse the repository at this point in the history 
…short-circuited locally as this will cause FTL to reply with the locally defined blcoking mode instead of the IP address of the upstream DNS server's blocking page

Signed-off-by: DL6ER <[email protected]>
  • Loading branch information
@DL6ER
DL6ER committed Feb 2, 2025
1 parent e3304ac commit 2137353
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 21 deletions.
37 changes: 26 additions & 11 deletions src/dnsmasq_interface.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1557,13 +1557,13 @@ static bool FTL_check_blocking(const unsigned int queryID, const unsigned int do
force_next_DNS_reply = dns_cache->force_reply;
query_blocked(query, domain, client, QUERY_SPECIAL_DOMAIN);
return true;
break;

case QUERY_EXTERNAL_BLOCKED_IP:
case QUERY_EXTERNAL_BLOCKED_NULL:
case QUERY_EXTERNAL_BLOCKED_NXRA:
case QUERY_EXTERNAL_BLOCKED_EDE15:

{
bool shortcircuit = true;
switch(blocking_status)
{
case QUERY_UNKNOWN:
Expand All @@ -1586,6 +1586,15 @@ static bool FTL_check_blocking(const unsigned int queryID, const unsigned int do
break;
case QUERY_EXTERNAL_BLOCKED_IP:
blockingreason = "blocked upstream with known address";
// We do not want to short-circuit this
// query as to get the address contained
// in the upstream reply being sent
// downstream to the client.
// Otherwise, Pi-hole's short-circuiting
// would reply to the client with the
// configured blocking mode (probably
// NULL)
shortcircuit = false;
break;
case QUERY_EXTERNAL_BLOCKED_NULL:
blockingreason = "blocked upstream with NULL address";
Expand All @@ -1605,8 +1614,8 @@ static bool FTL_check_blocking(const unsigned int queryID, const unsigned int do

force_next_DNS_reply = dns_cache->force_reply;
query_blocked(query, domain, client, blocking_status);
return true;
break;
return shortcircuit;
}

case QUERY_CACHE:
case QUERY_FORWARDED:
Expand All @@ -1626,7 +1635,6 @@ static bool FTL_check_blocking(const unsigned int queryID, const unsigned int do
query->flags.allowed = true;

return false;
break;
}

// Skip all checks and continue if we hit already at least one allowlist in the chain
Expand Down Expand Up @@ -2387,10 +2395,12 @@ static void FTL_reply(const unsigned int flags, const char *name, const union al
const double mean = upstream->rtime / upstream->responses;
upstream->rtuncertainty += (mean - query->response)*(mean - query->response);

// Only proceed if query is not already known
// to have been blocked upstream
if(query->status == QUERY_EXTERNAL_BLOCKED_IP ||
query->status == QUERY_EXTERNAL_BLOCKED_NULL ||
// Only proceed if query is not already known to have been
// blocked upstream AND short-circuited.
// Note: The reply needs to be analyzed further in case of
// QUERY_EXTERNAL_BLOCKED_IP as this is a "normal" upstream
// reply and we need to process it further (DNSSEC status, etc.)
if(query->status == QUERY_EXTERNAL_BLOCKED_NULL ||
query->status == QUERY_EXTERNAL_BLOCKED_NXRA ||
query->status == QUERY_EXTERNAL_BLOCKED_EDE15)
{
Expand Down Expand Up @@ -2989,8 +2999,13 @@ int _FTL_check_reply(const unsigned int rcode, const unsigned short flags,
{
FTL_blocked_upstream_by_addr(new_qstatus, id, file, line);

// Query is blocked
return 1;
// Query is blocked upstream

// Return true for any status except known blocking page
// IP address to short-circut the answer. In the latter case,
// we want to continue processing the query to get the correct
// reply downstream to the requesting client.
return new_qstatus != QUERY_EXTERNAL_BLOCKED_IP;
}
}

Expand Down
26 changes: 16 additions & 10 deletions test/test_suite.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -529,9 +529,10 @@
before="$(grep -c ^ /var/log/pihole/FTL.log)"

# Run test
run bash -c "dig A umbrella.ftl @127.0.0.1"
run bash -c "dig A umbrella.ftl +short @127.0.0.1"
printf "%s\n" "${lines[@]}"
[[ ${lines[@]} == *"EDE: 15 (Blocked): (upstream IP)"* ]]
[[ ${lines[0]} == "146.112.61.104" ]]
[[ ${lines[1]} == "" ]]

# Get number of lines in the log after the test
after="$(grep -c ^ /var/log/pihole/FTL.log)"
Expand All @@ -548,17 +549,17 @@
[[ ${lines[@]} == *"DEBUG_QUERIES: **** forwarded umbrella.ftl to 127.0.0.1#5555"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: blocked upstream with known address (IPv4)"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: DNS cache: A/127.0.0.1/umbrella.ftl -> EXTERNAL_BLOCKED_IP"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: Adding RR: \"umbrella.ftl A 0.0.0.0\""* ]]
}

@test "Upstream blocked domain: IP is recognized (cached)" {
# Get number of lines in the log before the test
before="$(grep -c ^ /var/log/pihole/FTL.log)"

# Run test
run bash -c "dig A umbrella.ftl @127.0.0.1"
run bash -c "dig A umbrella.ftl +short @127.0.0.1"
printf "%s\n" "${lines[@]}"
[[ ${lines[@]} == *"EDE: 15 (Blocked): (upstream IP)"* ]]
[[ ${lines[0]} == "146.112.61.104" ]]
[[ ${lines[1]} == "" ]]

# Get number of lines in the log after the test
after="$(grep -c ^ /var/log/pihole/FTL.log)"
Expand All @@ -572,17 +573,21 @@
done <<< "${log}"
printf "%s\n" "${lines[@]}"
[[ ${lines[@]} == *"DEBUG_QUERIES: umbrella.ftl is known as blocked upstream with known address (expires in"* ]]
# Test for NOT forwarded ...
[[ ${lines[@]} != *"DEBUG_QUERIES: **** forwarded umbrella.ftl to 127.0.0.1#5555"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: Adding RR: \"umbrella.ftl A 0.0.0.0\""* ]]
# ... but cached
[[ ${lines[@]} == *"DEBUG_QUERIES: **** got cache reply: umbrella.ftl is 146.112.61.104"* ]]
}

@test "Upstream blocked domain: IP is recognized (IPv6)" {
# Get number of lines in the log before the test
before="$(grep -c ^ /var/log/pihole/FTL.log)"

# Run test
run bash -c "dig AAAA umbrella.ftl @127.0.0.1"
run bash -c "dig AAAA umbrella.ftl +short @127.0.0.1"
printf "%s\n" "${lines[@]}"
[[ ${lines[0]} == "::ffff:146.112.61.104" ]]
[[ ${lines[1]} == "" ]]

# Get number of lines in the log after the test
after="$(grep -c ^ /var/log/pihole/FTL.log)"
Expand All @@ -599,16 +604,18 @@
[[ ${lines[@]} == *"DEBUG_QUERIES: **** forwarded umbrella.ftl to 127.0.0.1#5555"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: blocked upstream with known address (IPv6)"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: DNS cache: AAAA/127.0.0.1/umbrella.ftl -> EXTERNAL_BLOCKED_IP"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: Adding RR: \"umbrella.ftl AAAA ::\""* ]]
}

@test "Upstream blocked domain: IP is recognized (multi)" {
# Get number of lines in the log before the test
before="$(grep -c ^ /var/log/pihole/FTL.log)"

# Run test
run bash -c "dig A umbrella-multi.ftl @127.0.0.1"
run bash -c "dig A umbrella-multi.ftl +short @127.0.0.1"
printf "%s\n" "${lines[@]}"
[[ "${lines[@]}" == *"146.112.61.104"* ]]
[[ "${lines[@]}" == *"8.8.8.8"* ]]
[[ "${lines[@]}" == *"1.2.3.4"* ]]

# Get number of lines in the log after the test
after="$(grep -c ^ /var/log/pihole/FTL.log)"
Expand All @@ -624,7 +631,6 @@
[[ ${lines[@]} == *"DEBUG_QUERIES: DNS cache: A/127.0.0.1/umbrella-multi.ftl is not blocked (domainlist ID: -1)"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: **** forwarded umbrella-multi.ftl to 127.0.0.1#5555"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: DNS cache: A/127.0.0.1/umbrella-multi.ftl -> EXTERNAL_BLOCKED_IP"* ]]
[[ ${lines[@]} == *"DEBUG_QUERIES: Adding RR: \"umbrella-multi.ftl A 0.0.0.0\""* ]]
}

@test "Upstream blocked domain: EDE 15 is recognized" {
Expand Down

0 comments on commit 2137353

Please sign in to comment.