Use EDE data when "proxy-dnssec" is used

[DL6ER]

By submitting this pull request, I confirm the following:

• I have read and understood the contributors guide.
• I have checked that another pull request for this purpose does not exist.
• I have considered, and confirmed that this submission will be valuable to others.
• I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
• I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10

---

When the dnsmasq option proxy-dnssec

proxy-dnssec
Copy the DNSSEC Authenticated Data bit from upstream servers to downstream clients. This is an alternative to having dnsmasq validate DNSSEC, but it depends on the security of the network between dnsmasq and the upstream servers, and the trustworthiness of the upstream servers. Note that caching the Authenticated Data bit correctly in all cases is not technically possible. If the AD bit is to be relied upon when using this option, then the cache should be disabled using --cache-size=0. In most cases, enabling DNSSEC validation within dnsmasq is a better option. See --dnssec for details.

is used, FTL uses the AD bit to determine the INSECURE/SECURE status of queries. In cases where the upstream resolver includes EDE data for errors (e.g. unbound 1.16.0+), FTL also analyses this data to tell apart SERVFAILs caused by a "real" server failure from crafted SERVFAILs due to failed DNSSEC validation.

See linked Discourse Discussion for further details and extensive testing done by @jpgpi250

---

To get proxy-dnssec working, you'll need to add

edns: yes

to unbounds server section and

proxy-dnssec
add-cpe-id=01234

to a custom dnsmasq config file. The add-cpe-id options is necessary to query EDE data from the upstream destination in case a client does not signal EDNS0 support.

---

All in all, this PR also improves the internal handling of DNSSEC status and, e.g., ensures ignored queries (because they are duplicated) are not stamped INSECURE when this is not actually true (they are actually "nothing" as they are ignored). This makes this PR an enhancement even for the vast majority that will never use proxy-dnssec.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/dnssec-discussion/62217/10

[jpgpi250]

ftl crash, using this branch, see https://discourse.pi-hole.net/t/dnssec-discussion-support-for-proxy-dnssec/62217/12 for details, FTL.log with debug flags enabled attached in the topic.

edit
FIXED by DL6ER, thanks
/edit

[jpgpi250]

build failure?

[DL6ER]

PR ready for review + merge. PR text edited to reflect the lastest state

[jpgpi250]

don't know if this can be / needs correcting.
If a domain is blocked by gravity or domainlist entry, the dnssec value is set to 2 (insecure). since no upstream query has been submitted, I think it makes more sense to set the dnssec value to 0 (unknown).
This doesn't affect the web interface (status isn't shown for blocked entries, however, it falsifies reports, much more insecure, hardly any unknown entries.

NOT a big deal, but if it can be changed easily...

examples:

[DL6ER]

@jpgpi250 This is expected and agreement with queries in "normal" DNSSEC mode. Queries are either SECURE (= DNSSEC signed, validation successful), BOGUS (= DNSSEC signed, validation failed) or INSECURE (no DNSSEC signature). As gravity and all locally defined records are not DNSSEC signed, stamping them INSECURE is the logical option. This was previously broken but will now be fixed by this PR when proxy-dnssec is used. When neither dnssec nor proxy-dnssec are enabled, all queries get UNKNOWN DNSSEC status.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/dnssec-discussion-support-for-proxy-dnssec/62217/48