pi-hole / FTL

The Pi-hole FTL engine
https://pi-hole.net
Other
1.39k stars 196 forks source link

Fix external block detection on NXDOMAIN replies from authoritative upstreams #2087

Closed kaechele closed 1 month ago

kaechele commented 1 month ago

What does this PR aim to accomplish?:

Only consider NXDOMAIN replies as indication of external blocking if both the RA and AA bit are unset. This fixes #2085.

How does this PR accomplish the above?:

When analyzing the DNS packet header we now also check for the absence of the AA bit in addition to the absence of the RA bit.

Link documentation PRs if any are needed to support this PR:

N/A


By submitting this pull request, I confirm the following:

  1. I have read and understood the contributors guide, as well as this entire template. I understand which branch to base my commits and Pull Requests against.
  2. I have commented my proposed changes within the code and I have tested my changes.
  3. I am willing to help maintain this change if there are issues with it later.
  4. It is compatible with the EUPL 1.2 license
  5. I have squashed any insignificant commits. (git rebase)
  6. I have checked that another pull request for this purpose does not exist.
  7. I have considered, and confirmed that this submission will be valuable to others.
  8. I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  9. I give this submission freely, and claim no ownership to its content.

kaechele commented 1 month ago

I've tested this both on my use case (as described in #2085) and an actual blocked domain on Quad9.

My use-case now works correctly with this change.

The results for the blocked domain are also as expected:

With DNSSEC enabled the response from 9.9.9.9 is bogus ``` 2024-10-17 04:49:54.956 UTC [158787M] DEBUG_QUERIES: **** new UDP IPv4 query[A] query "1312services.ru" from eth0/10.0.0.151#54097 (ID 799, FTL 78783, src/dnsmasq/forward.c:1822) 2024-10-17 04:49:54.957 UTC [158787M] DEBUG_QUERIES: Set global cache status to 0 2024-10-17 04:49:54.957 UTC [158787M] DEBUG_QUERIES: 1312services.ru is not known 2024-10-17 04:49:54.979 UTC [158787M] DEBUG_QUERIES: Checking if "1312services.ru" is in antigravity (exact): no 2024-10-17 04:49:54.980 UTC [158787M] DEBUG_QUERIES: Checking if "1312services.ru" is in gravity (exact): no 2024-10-17 04:49:54.980 UTC [158787M] DEBUG_QUERIES: DNS cache: A/10.0.0.151/1312services.ru is not blocked (domainlist ID: -1) 2024-10-17 04:49:54.980 UTC [158787M] DEBUG_QUERIES: **** forwarded 1312services.ru to 9.9.9.11#53 (ID 799, src/dnsmasq/forward.c:559) 2024-10-17 04:49:54.980 UTC [158787M] DEBUG_QUERIES: DNS cache: A/10.0.0.151/1312services.ru -> FORWARDED, no expiry 2024-10-17 04:49:54.993 UTC [158787M] DEBUG_QUERIES: **** new IPv4 dnssec-query[DS] query "ru" from -/#53 (ID 800, FTL 78784, src/dnsmasq/forward.c:1120) 2024-10-17 04:49:54.993 UTC [158787M] DEBUG_QUERIES: **** forwarded ru to 9.9.9.11#53 (ID 800, src/dnsmasq/forward.c:1120) 2024-10-17 04:49:54.994 UTC [158787M] DEBUG_QUERIES: DNS cache: DS/::/ru -> FORWARDED, no expiry 2024-10-17 04:49:55.006 UTC [158787M] DEBUG_QUERIES: **** got upstream reply from 9.9.9.11#53: ru is DNSKEY (ID 800, src/dnsmasq/dnssec.c:1102) 2024-10-17 04:49:55.006 UTC [158787M] DEBUG_QUERIES: Set reply to DNSSEC (11) in src/dnsmasq_interface.c:2369 2024-10-17 04:49:55.006 UTC [158787M] DEBUG_QUERIES: **** new IPv4 dnssec-query[DS] query "1312services.ru" from -/#53 (ID 801, FTL 78785, src/dnsmasq/forward.c:1120) 2024-10-17 04:49:55.007 UTC [158787M] DEBUG_QUERIES: **** forwarded 1312services.ru to 9.9.9.11#53 (ID 801, src/dnsmasq/forward.c:1120) 2024-10-17 04:49:55.007 UTC [158787M] DEBUG_QUERIES: DNS cache: DS/::/1312services.ru -> FORWARDED, no expiry 2024-10-17 04:49:55.019 UTC [158787M] DEBUG_QUERIES: **** DNSSEC 1312services.ru is BOGUS (ID 799, src/dnsmasq/forward.c:1421) 2024-10-17 04:49:55.019 UTC [158787M] DEBUG_QUERIES: EDE: NSEC(3) missing (12) 2024-10-17 04:49:55.019 UTC [158787M] DEBUG_QUERIES: Set reply to NONE (12) in src/dnsmasq_interface.c:2627 ```
With DNSSEC disabled the response from 9.9.9.9 is considered blocked upstream ``` 2024-10-17 04:51:02.817 UTC [158837M] DEBUG_QUERIES: **** new UDP IPv4 query[A] query "1312services.ru" from eth0/10.0.0.151#59362 (ID 55, FTL 78858, src/dnsmasq/forward.c:1822) 2024-10-17 04:51:02.818 UTC [158837M] DEBUG_QUERIES: Set global cache status to 0 2024-10-17 04:51:02.818 UTC [158837M] DEBUG_QUERIES: 1312services.ru is not known 2024-10-17 04:51:02.839 UTC [158837M] DEBUG_QUERIES: Checking if "1312services.ru" is in antigravity (exact): no 2024-10-17 04:51:02.839 UTC [158837M] DEBUG_QUERIES: Checking if "1312services.ru" is in gravity (exact): no 2024-10-17 04:51:02.839 UTC [158837M] DEBUG_QUERIES: DNS cache: A/10.0.0.151/1312services.ru is not blocked (domainlist ID: -1) 2024-10-17 04:51:02.840 UTC [158837M] DEBUG_QUERIES: **** forwarded 1312services.ru to 9.9.9.11#53 (ID 55, src/dnsmasq/forward.c:559) 2024-10-17 04:51:02.840 UTC [158837M] DEBUG_QUERIES: DNS cache: A/10.0.0.151/1312services.ru -> FORWARDED, no expiry 2024-10-17 04:51:02.852 UTC [158837M] DEBUG_QUERIES: **** 1312services.ru externally blocked by header (ID 55, FTL 78858, /opt/src/dnsmasq/rfc1035.c:797) 2024-10-17 04:51:02.853 UTC [158837M] DEBUG_QUERIES: DNS cache: A/10.0.0.151/1312services.ru -> EXTERNAL_BLOCKED_NXRA, expires in 86400s 2024-10-17 04:51:02.853 UTC [158837M] DEBUG_QUERIES: Set reply to NXDOMAIN (2) in src/dnsmasq_interface.c:2821 2024-10-17 04:51:02.853 UTC [158837M] DEBUG_QUERIES: **** got upstream reply from 9.9.9.11#53: 1312services.ru is blocked due to upstream response (header) (ID 55, src/dnsmasq/rfc1035.c:802) 2024-10-17 04:51:02.853 UTC [158837M] DEBUG_QUERIES: Preparing reply for "1312services.ru" 2024-10-17 04:51:02.853 UTC [158837M] DEBUG_QUERIES: Setting EDE: blocked (15) + "upstream NXRA" 2024-10-17 04:51:02.853 UTC [158837M] DEBUG_QUERIES: Adding RR: "1312services.ru A 0.0.0.0" 2024-10-17 04:51:02.854 UTC [158837M] DEBUG_QUERIES: **** got cache reply: 1312services.ru is 0.0.0.0 (ID 55, src/dnsmasq_interface.c:495) ```
DL6ER commented 1 month ago

Thank you!

@pi-hole/ftl-maintainers we need to fix not deploying for builds from foreign repositories.

DL6ER commented 1 month ago

Overwriting failed checks, this is independent and will be fixed by #2088

kaechele commented 1 month ago

Thanks for the review and merge!