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.87test4-6 #1230

Merged
merged 16 commits into from
Dec 20, 2021
Merged

Update embedded dnsmasq to v2.87test4-6 #1230

merged 16 commits into from
Dec 20, 2021

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Oct 24, 2021

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


This mainly adds bugfixes for issues reported against the current embedded version of dnsmasq. The first two bug fixes are important for us as we have seen them being reported on our own platforms.

  • 2924c29 fixes query status being reported incorrectly for retired domains and some special edge-cases (see comment message)
  • ea689a2 fixes case-insensitive domain matching, see conditional forwarding does not work anymore #1254 (fix has been confirmed therein)
  • 89a8864 fixes a regression in rebind-domain-ok option recently introduced by dnsmasq v2.86
  • 5129c7e fixes locally generated answers returning an incorrect answer count when the packet gets too large and UDP truncation happens
  • 29261a5 fixes a crash when DNS service is disables and PXE/netboot is used (unlikely to happen for Pi-hole)

DL6ER and others added 9 commits October 24, 2021 09:56
…man page

Signed-off-by: DL6ER <dl6er@dl6er.de>
add_resource_record() returns 1 if the record was added.
Only increment anscount of so.

Thanks to Petr Menšík for spotting the problem.

Signed-off-by: DL6ER <dl6er@dl6er.de>
--address=/münchen.de/ is not accepted unless LOCALEDIR is defined on
build. It is not by default. If LIBIDN1 or 2 is defined, call setlocale
to initialize locale required to translate domains to ascii form.

Signed-off-by: Petr Menšík <pemensik@redhat.com>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Transitional encoding accepts every emoticon you can think about.
Because setlocale were not enabled before, IDN 2003 input was not
accepted by dnsmasq. It makes no sense therefore to maintain backward
compatibility. Accept only proper encoded unicode names and reject
random unicode characters.

Signed-off-by: Petr Menšík <pemensik@redhat.com>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER changed the base branch from master to development October 24, 2021 14:17
simonkelley and others added 6 commits December 1, 2021 16:33
The domain-match rewrite didn't take into account
that domain names are case-insensitive, so things like

--address=/Example.com/.....

didn't work correctly.

Signed-off-by: DL6ER <dl6er@dl6er.de>
The IDs logged when --log-queries=extra is in effect
can be wrong in three cases.

1) When query is retried in response to a a SERVFAIL or REFUSED
answer from upstream. In this case the ID of an unrelated query will
appear in the answer log lines.

2) When the same query arrives from two clients. The query is
sent upstream once, as designed, and the result returned to both clients,
as designed, but the reply to the first client gets the log-ID of the
second query in error.

3) When a query arrives, is sent upstream, and the reply comes back,
but the transaction is blocked awaiting a DNSSEC query needed to validate
the reply. If the client retries the query in this state, the blocking
DNSSEC query will be resent, as designed, but that send will be logged with
the ID of the original, currently blocked, query.

Thanks to  Dominik Derigs for his analysis of this problem.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Commit 32e15c3f458c2e8838a9ecf7d478ecb6750516bf added the following
change:

  --- a/src/dnsmasq/option.c
  +++ b/src/dnsmasq/option.c
  @@ -654,7 +654,7 @@ static char *canonicalise_opt(char *s)
       return 0;

     if (strlen(s) == 0)
  -    return "";
  +    return opt_string_alloc("");

     unhide_metas(s);
     if (!(ret = canonicalise(s, &nomem)) && nomem)

Unfortunately, opt_string_alloc(const char *cp) returns NULL when
strlen(cp) == 0, which in turn causes --rebind-domain-ok='' to crash
with SIGSEGV.

Signed-off-by: DL6ER <dl6er@dl6er.de>
The 2.86 domain-match rewrite changed matching from
whole-labels to substring matching, so example.com
would match example.com and www.example.com, as before,
but also goodexample.com, which is a regression. This
restores the original behaviour.

Also restore the behaviour of --rebind-domain-ok=//
to match domains with onlt a single label and no dots.

Thanks to Sung Pae for reporting these bugs and supplying
an initial patch.

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER marked this pull request as ready for review December 20, 2021 20:35
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER changed the title Update embedded dnsmasq to v2.87test4 Update embedded dnsmasq to v2.87test4-6 Dec 20, 2021
@DL6ER
Copy link
Member Author

DL6ER commented Dec 20, 2021

Ready for review+merge in preparation of a (pre-)Christmas release

@DL6ER DL6ER merged commit 92a27e7 into development Dec 20, 2021
@DL6ER DL6ER mentioned this pull request Dec 20, 2021
5 tasks
@pralor-bot
Copy link

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

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-12-web-v5-9-and-core-v5-7-released/51795/1

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.

6 participants