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 dnsmasq to v2.91test6 #2155

Merged
merged 10 commits into from
Jan 8, 2025
Merged

Update dnsmasq to v2.91test6 #2155

merged 10 commits into from
Jan 8, 2025

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Jan 8, 2025

What does this implement/fix?

Changelog:

Handle queries with EDNS client subnet fields better. If dnsmasq is configured to add an EDNS client subnet to a query, it is careful to suppress use of the cache, since a cached answer may not be valid for a query with a different client subnet. Extend this behaviour to queries which arrive a dnsmasq already carrying an EDNS client subnet.


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 10 commits January 1, 2025 18:50
If dnsmasq is configured to add an EDNS client subnet to a query,
it is careful to suppress use of the cache, since a cached answer may
not be valid for a query with a different client subnet.
Extend this behaviour to queries which arrive a dnsmasq
already carrying an EDNS client subnet.

This change is rather more involved than may seem necessary at first sight,
since the existing code relies on all queries being decorated by dnsmasq
and therefore not cached, so there is no chance that an incoming query
might hit the cache and cache lookup don't need to be suppressed, just
cache insertion. When downstream queries may be a mix of client-subnet
bearing and plain vanilla, it can't be assumed that the answers are never
in the cache, and queries with subnets must not do lookups.

Signed-off-by: DL6ER <[email protected]>
I misread the man page for socket(7) and TCP timeouts.

A timeout generates a -1 return and EAGAIN errno, NOT a short read.

Short reads are legit, and aborting when they are seen creates
hard-to-reproduce errors.

Signed-off-by: DL6ER <[email protected]>
Print a specific INFO message instead of a generic WARNING message,
so users aren't inconvenienced and maintainers know what to do.

Debian currently runs this service as part of NetworkManager,
in a systemd service without CAP_CHOWN.  Other distributions may
have the same problem, or might add the issue in future.
This fix should communicate the issue clearly to them.

Signed-off-by: DL6ER <[email protected]>
I have no memory for why this was ever there. It breaks DNSSEC
validation of large RRsets.

I can't see any DoS potential that is exposed by removing it.

Signed-off-by: DL6ER <[email protected]>
@DL6ER DL6ER requested a review from a team January 8, 2025 16:41
@DL6ER DL6ER marked this pull request as ready for review January 8, 2025 16:41
@DL6ER DL6ER merged commit 41db148 into development Jan 8, 2025
20 checks passed
@yubiuser
Copy link
Member

yubiuser commented Jan 8, 2025

Does this also apply to EDNS carrying mac information?

@DL6ER
Copy link
Member Author

DL6ER commented Jan 8, 2025

dnsmasq does not treat DNS packets with MAC information any special unless either add-mac or strip-mac is in your configs. If add-mac is defined (and a MAC address is actually known for the querying client), the query is marked as being non-cachable. EDNS client subnet is a much more involved feature.

@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.

4 participants