Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update embedded dnsmasq to v2.91test8 #2165

Merged
merged 9 commits into from
Jan 19, 2025
Merged

Update embedded dnsmasq to v2.91test8 #2165

merged 9 commits into from
Jan 19, 2025

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Jan 19, 2025

What does this implement/fix?

Add most recent dnsmasq commits. They are mostly concerned with mixing a local unsigned authoritative zone (e.g., home.example.com) with a DNSSEC signed upstream zone (e.g. example.com`). The first commit improves handling of duplicate DNS answers via UDP avoiding sending duplicate information to the client downstream in edge-cases. As the client would have anyway ignored it, this is just a tweak, not a fix for anything.


Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): 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.
  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)

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repositories developmental branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

simonkelley and others added 9 commits January 14, 2025 04:21
If we get a duplicate answer for a query via UDP which we have
either already received and started DNSSEC validation, or was
truncated and we've passed to TCP, then just ignore it.

The code was already in place, but had evolved wonky and
only worked for error replies which would otherwise prompt
a retransmit.

Signed-off-by: DL6ER <[email protected]>
If the value of CFLAGS is changed between builds, the makefile
will rebuid, in the same way as for COPTS.

Signed-off-by: DL6ER <[email protected]>
When dnsmasq is configured to act as an authoritative server and has
an authoritative zone configured, and recieves a query for
that zone _as_forwarder_ it answers the query directly rather
than forwarding it. This doesn't affect the answer, but it
saves dnsmasq forwarding the query to the recusor upstream,
whch then bounces it back to dnsmasq in auth mode. The
exception should be when the query is for the root of zone, for a DS
RR. The answer to that has to come from the parent, via the
recursor, and will typically be a proof-of-nonexistence since
dnsmasq doesn't support signed zones. This patch suppresses
local answers and forces forwarding to the upstream recursor
for such queries. It stops breakage when a DNSSEC validating
client makes queries to dnsmasq acting as forwarder for a zone
for which it is authoritative.

Signed-off-by: DL6ER <[email protected]>
If the client asks for DNSSEC RRs via the do bit, and
we have an answer cached, we can only return the cached
answer if the RR was not validated. This is because
we don't the extra info (RRSIGS, NSECs) for a complete
validated answer. In that case we have to forward again.

This bug was that the "is the cache entry validated" test was
in an outer loop rather than an inner one. A cache hit on
a different RRtype that wasn't validated would satify the
condition to use the cache, even if the cache entry for
the required RRtype didn't. The only time when there can be a mix
of validated and non validated cache entries for the same domain
is when most are not validated, but one is a negative cache for
a DS record.

This bug took a long time to find.

Signed-off-by: DL6ER <[email protected]>
Let's give the poor programmers a chance.

Signed-off-by: DL6ER <[email protected]>
@DL6ER DL6ER merged commit 84e2ad1 into development Jan 19, 2025
19 checks passed
@PromoFaux PromoFaux mentioned this pull request Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants