Caching an answer which has more that one RR, with at least
one answer being <=13 bytes and at least one being >13 bytes
can screw up the F_KEYTAG flag bit, resulting in the wrong
type of the address union being used and either a bad value
return or a crash in the block code.
Thanks to Dominik Derigs and the Pi-hole project for finding
and characterising this.
By default TCP connect takes minutes to fail when trying to
connect a server which is not responding and for which the
network layer doesn't generate HOSTUNREACH errors.
This is doubled because having failed to connect in FASTOPEN
mode, the code then tries again with a call to connect().
We set TCP_SYNCNT to 2, which make the timeout about 10 seconds.
This in an unportable Linux feature, so it doesn't work on other
platforms.
No longer try connect() if sendmsg in fastopen mode fails with
ETIMEDOUT or EHOSTUNREACH since the story will just be the same.
On 15/5/2023 8.8.8.8 was returning SERVFAIL for a query on ec.europa.eu
ec.europa.eu is not a domain cut, that happens at jrc.ec.europa.eu. which
does return a signed proof of non-existance for a DS record.
Abandoning the search for a DS or proof of non existence at ec.europa.eu
renders everything within that domain BOGUS, since nothing is signed.
This code changes behaviour on a SERVFAIL to continue looking
deeper for a DS or proof of its nonexistence.
After replying with stale data, dnsmasq sends the query upstream to
refresh the cache asynchronously and sometimes sends the wrong packet:
packet length can be wrong, and if an EDE marking stale data is added
to the answer that can end up in the query also. This bug only seems
to cause problems when the usptream server is a DOH/DOT proxy. Thanks
to Justin He for the bug report.
If I configure dnsmasq to use dbus and then restart dbus.service with watchers present,
it crashes dnsmasq. The reason is simple, it uses loop to walk over watchers to call
dbus handling code. But from that code the same list can be modified and watchers removed.
But the list iteration continues anyway.
Restart the loop if list were modified.
A NXDOMAIN answer recieved over TCP by a child process would
be correctly sent back to the master process which would then
fail to insert it into the cache.
the compiler padding it with an extra 8 bytes.
Use the F_KEYTAG flag in a a cache record to discriminate between
an arbitrary RR stored entirely in the addr union and one
which has a point to block storage.
If a NODATA answer is returned instead of actual data for A or AAAA
queries because of the existence of --filter-A or --filter-AAAA
config options, then mark the replies with an EDE "filtered" tag.
Basic patch by Petr Menšík, tweaked by Simon Kelley to apply onto
the preceding caching patches.
If --filter-AAAA is set and we have cached entry for
the domain in question fpr any RR type that allows us to
return a NODATA reply when --filter-AAAA is set without
going upstream. Similarly for --filter-A.
Dynamic-host was implemented to ignore interface addresses with /32
(or /128 for IPv6) prefix lengths, since they are not useful for
synthesising addresses.
Due to a bug before 2.88, this didn't work for IPv4, and some have
used --dynamic-host=example.com,0.0.0.0,eth0 to do the equivalent of
--interface-name for such interfaces. When the bug was fixed in 2.88
these uses broke.
Since this behaviour seems to violate the principle of least surprise,
and since the 2.88 fix is breaking existing imstallations, this
commit removes the check on /32 and /128 prefix lengths to solve both
problems.
If there exists a --address=/<domain>/ or --server=/<domain>/#
configuration but no upstream server config unqualified by
domain then when a query which doesnt match the domain is
recieved it will use the qualfied server config and in the process
possibly make an out-of-bounds memory access.
Thanks to Daniel Danzberger for spotting the bug.
As defined in the C standard:
In all cases the argument is an int, the value of which shall
be representable as an unsigned char or shall equal the value
of the macro EOF. If the argument has any other value, the
behavior is undefined.
This is because they're designed to work with the int values returned
by getc or fgetc; they need extra work to handle a char value.
If EOF is -1 (as it almost always is), with 8-bit bytes, the allowed
inputs to the ctype(3) functions are:
{-1, 0, 1, 2, 3, ..., 255}.
However, on platforms where char is signed, such as x86 with the
usual ABI, code like
char *arg = ...;
... isspace(*arg) ...
may pass in values in the range:
{-128, -127, -126, ..., -2, -1, 0, 1, ..., 127}.
This has two problems:
1. Inputs in the set {-128, -127, -126, ..., -2} are forbidden.
2. The non-EOF byte 0xff is conflated with the value EOF = -1, so
even though the input is not forbidden, it may give the wrong
answer.
Casting char to int first before passing the result to ctype(3)
doesn't help: inputs like -128 are unchanged by this cast. It is
necessary to cast char inputs to unsigned char first; you can then
cast to int if you like but there's no need because the functions
will always convert the argument to int by definition. So the above
fragment needs to be:
char *arg = ...;
... isspace((unsigned char)*arg) ...
This patch inserts unsigned char casts where necessary, and changes
int casts to unsigned char casts where the input is char.
I left alone int casts where the input is unsigned char already --
they're not immediately harmful, although they would have the effect
of suppressing some compiler warnings if the input is ever changed to
be char instead of unsigned char, so it might be better to remove
those casts too.
I also left alone calls where the input is int to begin with because
it came from getc; casting to unsigned char here would be wrong, of
course.
