The next() function is broken for any TFTP packet with padding
which doesn't end with a zero.
Rewrite to handle such packets.
Thanks to Helge Deller <deller@gmx.de> for persisting in finding the
actual problem and proposing a solution. This patch is modelled on his,
but rewritten for personal preference by Simon Kelley, who is
responsible for all bugs.
To complement the previous one, which fixed the retry path
when the query is retried from a different id/source address, this
fixes retries from the same id/source address.
A retry to upstream DNS servers triggered by the following conditions
1) A query asking for the same data as a previous query which has not yet been answered.
2) The second query arrives more than two seconds after the first.
3) Either the source of the second query or the id field differs from the first.
fails to set the case of the retry to the same pattern as the first attempt.
However dnsmasq expects the reply from upstream to have the case
pattern of the first attempt.
If the answer to the retry arrives before the answer to the first
query, dnsmasq will notice the case mismatch, log an error, and
ignore the answer.
The worst case scenario would be the first upstream query or reply is
lost and there would follow a short period where all queries for that
particular domain would fail.
This is a 2.91 development issue, it doesn't apply to previous stable releases.
Fix a case sensitivity problem which has been lurking for a long while.
When we get example.com and Example.com and combine them, we send whichever
query arrives first upstream and then later answer it, and we also
answer the second with the same answer. That means that if example.com
arrives first, it will get the answer example.com - good - but Example.com
will _also_ get the answer example.com - not so good.
In theory, fixing this is simple without having to keep seperate
copies of all the queries: Just use the bit-vector representation
of case flipping that we have for 0x20-encoding to keep the
differences in case. The complication comes from the fact that
the existing bit-vector code only holds data on the first 32 alpha
letters, because we only flip that up to many for 0x20 encoding.
In practise, the delta between combined queries can almost always
be represented with that data, since almost all queries are
all lower case and we only purturb the first 32 letters with
0x20 encoding. It's therefore worth keeping the existing,
efficient data structure for the 99.9% of the time it works.
For the 0.1% it doesn't, however, one needs an arbitrary-length data
structure with the resource implications of that.
Thanks to Peter Tirsek for the well researched bug report which set me
on to these problems.
We must only compare case when mapping an answer from upstream
to a forwarding record, not when checking a query to see if it's a
duplicate. Since the saved query name is scrambled, that ensures
that almost all such checks will wrongly fail.
Thanks to Peter Tirsek for an exemplary bug report for this.
The "bit 0x20 encoding" implemented in 995a16ca0c
can interact badly with (hopefully) rare broken upstream servers. Provide
an option to turn it off and a log message to give a clue as to why DNS service
is non-functional.
When a new IPv6 address is being added to a dhcp_config
struct, if there is anything invalid regarding the prefix
it looks like there is a potential memory leak.
ret_err_free() should be used to free it.
Also, the new addrlist struct is being linked into
the existing addr6 list in the dhcp_config before the
validity check, it is best to defer this insertion
until later so an invalid entry is not present, since
the CONFIG_ADDR6 flag might not have been set yet.
Signed-off-by: Brian Haley <haleyb.dev@gmail.com>
This provides extra protection against reply-spoof attacks.
Since DNS queries are case-insensitive, it's possible to randomly flip
the case of letters in a query and still get the correct answer back.
This adds an extra dimension for a cache-poisoning attacker to guess
when sending replies in-the-blind since it's expected that the
legitimate answer will have the same pattern of upper and lower case
as the query, so any replies which don't can be ignored as
malicious.
The amount of extra entropy clearly depends on the number
of a-z and A-Z characters in the query, and this implementation puts a
hard limit of 32 bits to make rescource allocation easy. This about
doubles entropy over the standard random ID and random port
combination.
When checking that an answer is the answer to the question that
we asked, compare the name in a case-sensitive manner.
Clients can set the letters in a query to a random pattern of
uppercase and lowercase to add more randomness as protection against
cache-poisoning attacks, and we don't want to nullify that.
This actually restores the status quo before
commit ed6d29a784
since matching questions and answers using a checksum
can't help but be case sensitive.
This patch is a preparation for introducing DNS-0x20
in the dnsmasq query path.
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.
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.
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.
If a child process dies unexpectedly, log the error and
try and tidy up so the main process continues to run and
doesn't block awaiting the dead child.
Print a specific INFO message instead of a generic WARNING message,
so users know what to do.
Starting dnsmasq without upstream servers indicates a problem by default,
but is perfectly normal with D-Bus enabled. For example, NetworkManager
starts dnsmasq with no upstream servers, then immediately populates it
over D-Bus.
Handling events on file descriptors can result in new file
descriptors being created or old ones being deleted. As such
the results of the last call to poll() become invalid in subtle
ways.
After handling each file descriptor in check_dns_listeners()
return, to go around the poll() loop again and get valid data
for the new situation.
Thanks to Dominik Derigs for his indefatigable sleuthing of this one.
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.
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.
