Not doing so can result in a use after free since the name for DHCP
derived DNS records is represented as a pointer into the DHCP lease
table. Update will only happen when necessary since lease_update_dns
tests internally on dns_dirty and the force argument is zero.
Signed-off-by: Erik Karlsson <erik.karlsson@iopsys.eu>
Timeouts for TCP connections to non-responive servers are very long.
This in not appropriate for DNS connections.
Set timeouts for connection setup, sending data and recieving data.
The timeouts for connection setup and sending data are set at 5 seconds.
For recieving the reply this is doubled, to take into account the
time for usptream to actually get the answer.
Thanks to Petr Menšík for pointing out this problem, and finding a better
and more portable solution than the one in place heretofore.
A relatively common situation is that the reply to a downstream query
will fit in a UDP packet when no DNSSEC RRs are present, but overflows
when the RRSIGS, NSEC ect are added. This extends the automatic
move from UDP to TCP to downstream queries which get truncated replies,
in the hope that once stripped of the DNSSEC RRs, the reply can be returned
via UDP, nwithout making the downstream retry with TCP.
If the downstream sets the DO bit, (ie it wants the DNSSEC RRs, then
this path is not taken, since the downstream will have to get a truncated
repsonse and retry to get a correct answer.
Heretofore, when a validating the result of an external query triggers
a DNSKEY or DS query and the result of that query is truncated, dnsmasq
has forced the whole validation process to move to TCP by returning a
truncated reply to the original requestor. This forces the original
requestor to retry the query in TCP mode, and the DNSSEC subqueries
also get made via TCP and everything works.
Note that in general the actual answer being validated is not large
enough to trigger truncation, and there's no reason not to return that
answer via UDP if we can validate it successfully. It follows that
a substandard client which can't do TCP queries will still work if the
answer could be returned via UDP, but fails if it gets an artifically
truncated answer and cannot move to TCP.
This patch teaches dnsmasq to move to TCP for DNSSEC queries when
validating UDP answers. That makes the substandard clients mentioned
above work, and saves a round trip even for clients that can do TCP.
CAP_NET_ADMIN is needed in the DHCPv4 code to place entries into
the ARP cache. If it's configured to unconditionally broadcast
to unconfigured clients, it never touches the ARP cache and
doesn't need CAP_NET_ADMIN.
Thanks to Martin Ivičič <max.enhanced@gmail.com> for prompting this.
If there are no dynamic configuration directories configured with
dhcp-hostsdir, dhcp-optsdir and hostsdir then we need to use inotify
only to track changes to resolv-files, but we don't need to do
that when DNS is disabled (port=0) or no resolv-files are configured.
It turns out that inotify slots can be a scarce resource, so not
using one when it's not needed is a Goood Thing.
Patch by HL, description above from SRK.
Now we can cache arbirary RRs, give more correct answers when
replying negative answers from cache.
To implement this needed the DNS-doctor code to be untangled from
find_soa(), so it should be under suspicion for any regresssions
in that department.
Similar to local-service, but more strict. Listen only on localhost
unless other interface is specified. Has no effect when interface is
provided explicitly. I had multiple bugs fillen on Fedora, because I have
changed default configuration to:
interface=lo
bind-interfaces
People just adding configuration parts to /etc/dnsmasq.d or appending to
existing configuration often fail to see some defaults are already there.
Give them auto-ignored configuration as smart default.
Signed-off-by: Petr Menšík <pemensik@redhat.com>
Do not add a new parameter on command line. Instead add just parameter
for behaviour modification of existing local-service option. Now it
accepts two optional values:
- net: exactly the same as before
- host: bind only to lo interface, do not listen on any other addresses
than loopback.
At startup, the leases file is read by lease_init(), and
in lease_init() undecorated hostnames are expanded into
FQDNs by adding the domain associated with the address
of the lease.
lease_init() happens relavtively early in the startup, party because
if it calls the dhcp-lease helper script, we don't want that to inherit
a load of sensitive file descriptors. This has implications if domains
are defined using the --domain=example.com,eth0 format since it's long
before we call enumerate_interfaces(), so get_domain fails for such domains.
The patch just moves the hostname expansion function to a seperate
subroutine that gets called later, after enumerate_interfaces().
Add the relevant information to the metrics and to the output of
dump_cache() (which is called when dnsmasq receives SIGUSR1).
Hence, users not collecting metrics will still be able to
troubleshoot with SIGUSR1. In addition to the current usage,
dump_cache() contains the information on the highest usage
since it was last called.
No longer try and fail to open every port when the port range
is in complete use; go straight to re-using an existing socket.
Die at startup if port range is smaller than --port-limit, since
the code behaves badly in this case.
This gives dnsmasq the ability to originate retries for upstream DNS
queries itself, rather than relying on the downstream client. This is
most useful when doing DNSSEC over unreliable upstream network. It
comes with some cost in memory usage and network bandwidth.
Fix a bug found on OpenWrt when IPv4/6 dual stack enabled:
The resolv file is located on tmpfs whose mtime resolution
is 1 second. If the resolv file is updated twice within one
second dnsmasq may can't notice the second update.
netifd updates the resolv file with method: write temp then move,
so adding an inode check fixes this bug.
--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>
If dnsmasq re-reads a resolv file, and it's empty, it will
retry after a delay. In the meantime, the old servers from the
resolv file have been deleted, but the servers_array doesn't
get updated, leading to dangling pointers and crashes.
Thanks to Brad Jorsch for finding and analysing this bug.
This problem was introduced in 2.86.
Error: DEADCODE (CWE-561): [#def12]
dnsmasq-2.86rc3/src/dnsmasq.c:37: assignment: Assigning: "bind_fallback" = "0".
dnsmasq-2.86rc3/src/dnsmasq.c:927: const: At condition "bind_fallback", the value of "bind_fallback" must be equal to 0.
dnsmasq-2.86rc3/src/dnsmasq.c:927: dead_error_condition: The condition "bind_fallback" cannot be true.
dnsmasq-2.86rc3/src/dnsmasq.c:928: dead_error_line: Execution cannot reach this statement: "my_syslog(4, "setting --bin...".
dnsmasq-2.86rc3/src/dnsmasq.c:928: effectively_constant: Local variable "bind_fallback" is assigned only once, to a constant value, making it effectively constant throughout its scope. If this is not the intent, examine the logic to see if there is a missing assignment that would make "bind_fallback" not remain constant.
# 926|
# 927| if (bind_fallback)
# 928|-> my_syslog(LOG_WARNING, _("setting --bind-interfaces option because of OS limitations"));
# 929|
# 930| if (option_bool(OPT_NOWILD))
Error: REVERSE_NEGATIVE (CWE-191): [#def13]
dnsmasq-2.86rc3/src/dnsmasq.c:383: negative_sink_in_call: Passing "dnsmasq_daemon->pxefd" to a parameter that cannot be negative.
dnsmasq-2.86rc3/src/dnsmasq.c:1086: check_after_sink: You might be using variable "dnsmasq_daemon->pxefd" before verifying that it is >= 0.
# 1084| {
# 1085| poll_listen(daemon->dhcpfd, POLLIN);
# 1086|-> if (daemon->pxefd != -1)
# 1087| poll_listen(daemon->pxefd, POLLIN);
# 1088| }
Error: CHECKED_RETURN (CWE-252): [#def18]
dnsmasq-2.86rc3/src/dnsmasq.c:1582: check_return: Calling "fcntl(dnsmasq_daemon->helperfd, 4, i & 0xfffffffffffff7ff)" without checking return value. This library function may fail and return an error code.
# 1580| /* block in writes until all done */
# 1581| if ((i = fcntl(daemon->helperfd, F_GETFL)) != -1)
# 1582|-> fcntl(daemon->helperfd, F_SETFL, i & ~O_NONBLOCK);
# 1583| do {
# 1584| helper_write();
Error: CHECKED_RETURN (CWE-252): [#def22]
dnsmasq-2.86rc3/src/dnsmasq.c:1991: check_return: Calling "fcntl(confd, 4, flags & 0xfffffffffffff7ff)" without checking return value. This library function may fail and return an error code.
# 1989| Reset that here. */
# 1990| if ((flags = fcntl(confd, F_GETFL, 0)) != -1)
# 1991|-> fcntl(confd, F_SETFL, flags & ~O_NONBLOCK);
# 1992|
# 1993| buff = tcp_request(confd, now, &tcp_addr, netmask, auth_dns);
Error: CHECKED_RETURN (CWE-252): [#def26]
dnsmasq-2.86rc3/src/dnssec.c:727: check_return: Calling "extract_name" without checking return value (as is done elsewhere 9 out of 10 times).
dnsmasq-2.86rc3/src/dnssec.c:459: example_checked: Example 1: "extract_name(header, plen, &p, keyname, 1, 0)" has its value checked in "extract_name(header, plen, &p, keyname, 1, 0)".
dnsmasq-2.86rc3/src/dnssec.c:269: example_checked: Example 2: "extract_name(header, plen, &state->ip, state->buff, 1, 0)" has its value checked in "extract_name(header, plen, &state->ip, state->buff, 1, 0)".
dnsmasq-2.86rc3/src/dnssec.c:569: example_checked: Example 3: "extract_name(header, plen, &p, keyname, 1, 0)" has its value checked in "extract_name(header, plen, &p, keyname, 1, 0)".
dnsmasq-2.86rc3/src/rfc1035.c:648: example_checked: Example 4: "extract_name(header, qlen, &p1, name, 1, 0)" has its value checked in "extract_name(header, qlen, &p1, name, 1, 0)".
dnsmasq-2.86rc3/src/rfc1035.c:787: example_checked: Example 5: "extract_name(header, qlen, &p1, name, 1, 0)" has its value checked in "extract_name(header, qlen, &p1, name, 1, 0)".
# 725| /* namebuff used for workspace above, restore to leave unchanged on exit */
# 726| p = (unsigned char*)(rrset[0]);
# 727|-> extract_name(header, plen, &p, name, 1, 0);
# 728|
# 729| if (key)
Error: CHECKED_RETURN (CWE-252): [#def27]
dnsmasq-2.86rc3/src/dnssec.c:1020: check_return: Calling "extract_name" without checking return value (as is done elsewhere 7 out of 8 times).
dnsmasq-2.86rc3/src/auth.c:140: example_checked: Example 1: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)".
dnsmasq-2.86rc3/src/dnssec.c:771: example_checked: Example 2: "extract_name(header, plen, &p, name, 1, 4)" has its value checked in "extract_name(header, plen, &p, name, 1, 4)".
dnsmasq-2.86rc3/src/hash-questions.c:57: example_checked: Example 3: "extract_name(header, plen, &p, name, 1, 4)" has its value checked in "extract_name(header, plen, &p, name, 1, 4)".
dnsmasq-2.86rc3/src/rfc1035.c:1028: example_checked: Example 4: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)".
dnsmasq-2.86rc3/src/rfc1035.c:1438: example_checked: Example 5: "extract_name(header, qlen, &p, name, 1, 4)" has its value checked in "extract_name(header, qlen, &p, name, 1, 4)".
# 1018|
# 1019| p = (unsigned char *)(header+1);
# 1020|-> extract_name(header, plen, &p, name, 1, 4);
# 1021| p += 4; /* qtype, qclass */
# 1022|
Consistently treat a non-NULL return from [ud]bus-init() as a fatal error:
either die() if still starting, or log an error and disable
the relevant module if dnsmasq has already started.
Also rationalise calls to set and check listeners depending on
configuration.
This used to have a global limit, but that has a problem when using
different servers for different upstream domains. Queries which are
routed by domain to an upstream server which is not responding will
build up and trigger the limit, which breaks DNS service for all other
domains which could be handled by other servers. The change is to make
the limit per server-group, where a server group is the set of servers
configured for a particular domain. In the common case, where only
default servers are declared, there is no effective change.
Fix bug which caused dnsmasq to lose track of processes forked
to handle TCP DNS connections under heavy load. The code
checked that at least one free process table slot was
available before listening on TCP sockets, but didn't take
into account that more than one TCP connection could
arrive, so that check was not sufficient to ensure that
there would be slots for all new processes. It compounded
this error by silently failing to store the process when
it did run out of slots. Even when this bug is triggered,
all the right things happen, and answers are still returned.
Only under very exceptional circumstances, does the bug
manifest itself: see
https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q2/014976.html
Thanks to Tijs Van Buggenhout for finding the conditions under
which the bug manifests itself, and then working out
exactly what was going on.
Previously, without min-port or max-port configured, dnsmasq would
default to the compiled in defaults for those, which are 1024 and
65535. Now, when neither are configured, it defaults instead to
the kernel's ephemeral port range, which is typically
32768 to 60999 on Linux systems. This change eliminates the
possibility that dnsmasq may be using a registered port > 1024
when a long-running daemon starts up and wishes to claim it.
This change does likely slighly reduce the number of random ports
and therefore the protection from reply spoofing. The older
behaviour can be restored using the min-port and max-port config
switches should that be a concern.
CVE-2021-3448 applies.
It's possible to specify the source address or interface to be
used when contacting upstream nameservers: server=8.8.8.8@1.2.3.4
or server=8.8.8.8@1.2.3.4#66 or server=8.8.8.8@eth0, and all of
these have, until now, used a single socket, bound to a fixed
port. This was originally done to allow an error (non-existent
interface, or non-local address) to be detected at start-up. This
means that any upstream servers specified in such a way don't use
random source ports, and are more susceptible to cache-poisoning
attacks.
We now use random ports where possible, even when the
source is specified, so server=8.8.8.8@1.2.3.4 or
server=8.8.8.8@eth0 will use random source
ports. server=8.8.8.8@1.2.3.4#66 or any use of --query-port will
use the explicitly configured port, and should only be done with
understanding of the security implications.
Note that this change changes non-existing interface, or non-local
source address errors from fatal to run-time. The error will be
logged and communiction with the server not possible.
This patch fixes a buffer overflow in TCP requests. Since the read is not
actually being retried, the byte written by the child can be left
in the pipe. When that happens, cache_recv_insert() reads the length of the
name, which is now multiplied by 256 due to the extra 0 byte (8 bit shift)
and results in daemon->namebuff being overflowed.
Namebuff is immediately before the daemon struct in memory so it
ends up corrupting the beginning of the daemon struct.
If interface is recreated with the same address but different index, it
would not change any other parameter.
Test also address family on incoming TCP queries.
We call this, which avoids POLLERR returns from netlink on a loaded system,
if the kernel is new enough to support it. Sadly, qemu-user doesn't support
the socket option, so if it fails despite the kernel being new enough to
support it, we just emit a warning, rather than failing hard.
