When using Stubby as a system DNS over TLS resolver with a Internet
connection that disconnects and reconnects from time to time there is often
a long waiting time (~20 minutes) after the connection reconnects before
DNS queries start to work again.
This is because in this particular case all the upstream TLS TCP
connections in Stubby are stuck waiting for upstream server response.
Which will never arrive since the host external IP address might have
changed and / or NAT router connection tracking entries for these TCP
connections might have been removed when the Internet connection
reconnected.
By default Linux tries to retransmit data on a TCP connection 15 times
before finally terminating it.
This takes 16 - 20 minutes, which is obviously a very long time to wait for
system DNS resolving to work again.
This is a real problem on weak mobile connections.
Thankfully, there is a "TCP_USER_TIMEOUT" per-socket option that allows
explicitly setting how long the network stack will wait in such cases.
Let's add a matching "tcp_send_timeout" option to getdns that allows
setting this option on outgoing TCP sockets.
For backward compatibility the code won't try to set it by default.
With this option set to, for example, 15 seconds Stubby recovers pretty
much instantly in such cases.
Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Most of the time we only need a read _or_ a write callback registered
with libuv - for example, on a UDP request a write callback is
registered, when executed the write callback performs the write,
deregisters itself, and registers a read callback.
However there is one case where getdns registers both read and write
callbacks: when a backlog of TCP requests is going to the same upstream
resolver, we use a single fd and queue the requests. In this instance we
want to listen for both read (to get responses for requests we've
already sent) and write (to continue to send our pending requests).
libuv, like most event libraries, only allows one callback to be
registered per fd. To get notification for both reads and writes, you
should examine the event flags and have appropriate conditional logic
within the single callback. Today getdns incorrectly tries to register
two separate poll_t with libuv, one for read and one for write - this
results in a crash (internal libuv assertion guaranteeing that only a
single poll_t is registered per fd).
Testing was done by using flamethrower
(https://github.com/DNS-OARC/flamethrower) to toss queries at a program
that embeds getdns.
Note that a higher qps trigger a _different_ getdns/libuv crashing bug
that occurs when the TCP backlog grows so large that requests start to
time out. That crash is not addressed in this PR, and will be more
involved to fix.
for the query, just like connection setup timeout was, so fallback transport have a chance too when TCP connection setup is less well detectable (as with TCP_FASTOPEN on MacOS).
Now GnuTLS 3.6.5 and later are in the field, we've run into problems handshaking with TLS1.3 servers with a GnuTLS build. OpenSSL works fine. Comparing the client handshake of GnuTLS and OpenSSL, we found GnuTLS was being considerably more restrictive. This change loosens the restriction so GnuTLS presents nearly the same set of cipher and other options and OpenSSL. OpenSSL provides more signature algorithms. The change gets GetDNS working against Quad1, Quad8, Quad9 and the getdnsapi servers.
On the first call to tls_create_object (stub.c), tls_fallback_ok is read
before being initialized. This patch initializes tls_fallback_ok to 0 in
upsteam_init (context.c)
Valgrind complains about the uninitialized value:
==14774== Conditional jump or move depends on uninitialised value(s)
==14774== at 0x1528C3: tls_create_object (stub.c:900)
==14774== by 0x1556AD: upstream_connect (stub.c:2065)
==14774== by 0x15582E: upstream_find_for_transport (stub.c:2109)
==14774== by 0x1558B7: upstream_find_for_netreq (stub.c:2130)
==14774== by 0x156027: _getdns_submit_stub_request (stub.c:2296)
==14774== by 0x1421C8: _getdns_submit_netreq (general.c:478)
==14774== by 0x14261D: getdns_general_ns (general.c:636)
==14774== by 0x142905: _getdns_general_loop (general.c:731)
==14774== by 0x1432FB: getdns_general (general.c:888)
==14774== by 0x118B94: incoming_request_handler (stubby.c:692)
==14774== by 0x14F46B: udp_read_cb (server.c:762)
==14774== by 0x15C86B: poll_read_cb (poll_eventloop.c:295)
==14774== Uninitialised value was created by a heap allocation
==14774== at 0x483877F: malloc (vg_replace_malloc.c:309)
==14774== by 0x123CCF: upstreams_create (context.c:581)
==14774== by 0x128B24: getdns_context_set_upstream_recursive_servers (context.c:2760)
==14774== by 0x12DBFE: _getdns_context_config_setting (context.c:4646)
==14774== by 0x12FF47: getdns_context_config (context.c:4769)
==14774== by 0x1178C2: parse_config (stubby.c:297)
==14774== by 0x117B24: parse_config_file (stubby.c:343)
==14774== by 0x11919F: main (stubby.c:833)
Before (FreeBSD 11), poll could be used to wait for the socket to
be writeable immediately. Now (since FreeBSD 12) this results in
infinite wait, so we just have to write immediately to work around
this.
Tweak libcheck/windows logic.
Hacks to make tests pass with strange bionic system resolver behaviour
Add to README that xenial doesn’t have libunbound-dev 1.5.9 packaged
sys/poll.h seems to be some GNU extension. musl warns about this:
warning redirecting incorrect #include <sys/poll.h> to <poll.h>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Add an include of stdlib.h to various files that were relying on config.h to drag it in. I don't think config.h should be pulling in standard C headers.
The change mostly consists of removing or replacing non-standard (usually POSIX) header includes.
Guards for replacements for inet_ntop(), inet_pton() and gettimeofday() are updated; the first two are macros on Windows, so the guards are changed to HAVE_DECL. gettimeofday() is present on MinGW builds but not Visual Studio, so that has a function check.
uClibc 0.9.30rc1 - 0.9.32rc5 has bug - getaddrinfo() does not accept numeric
service without any hints. As the related side effect, hint struct with
ai_socktype == 0 (unspec) and ai_protocol == 0 (unpsec) gives the same
EAI_SERVICE error instead of same address with different proto enumebration.
For more details please refer https://bugs.busybox.net/show_bug.cgi?id=3841 and
https://git.uclibc.org/uClibc/commit/?id=bc3be18145e4d57e7268506f123c0f0f373a15e2
Since 0.9.3x uClibc versions are still not somewhat unique in embedded (issue
https://github.com/getdnsapi/stubby/issues/124 as example) and non-zero
ai_socktype allows to avoid address dups for each supported UDP/TCP/etc proto,
seems worth to have it specified, as a minor memory allocation optimization at
least.
SOCK_DGRAM vs SOCK_STREAM choice doesn't really matter here, both are actually
used for DNS and both are non-zero, no difference is expected on *nix. So
SOCK_DGRAM selected due original comment only.
Typedefs sha256_pin_t & getdns_log_config multiple declaration in context.h,
tls.h and tls_internal.h causes build error with some gnu99 compilers, even
if the redefinition is identical.
One possible way is to protect each occurence with ifdefs, but it seems too
brute, other one is to keep typedef in context.h only and use struct types
in recently added tls* scope.
Error example:
../libtool --quiet --tag=CC --mode=compile arm-brcm-linux-uclibcgnueabi-gcc
-std=gnu99 -I. -I. -I./util/auxiliary -I./tls -I./openssl -I./../stubby/src
-Wall -Wextra -D_BSD_SOURCE -D_DEFAULT_SOURCE ... -c ./convert.c -o convert.lo
In file included from ./context.h:53:0,
from ./util-internal.h:42,
from ./convert.c:50:
./tls.h:45:27: error: redefinition of typedef 'sha256_pin_t'
./openssl/tls-internal.h:57:27: note: previous declaration of 'sha256_pin_t' was here
In file included from ./util-internal.h:42:0,
from ./convert.c:50:
./context.h:133:3: error: redefinition of typedef 'sha256_pin_t'
./tls.h:45:27: note: previous declaration of 'sha256_pin_t' was here
./context.h:267:3: error: redefinition of typedef 'getdns_log_config'
./openssl/tls-internal.h:58:34: note: previous declaration of 'getdns_log_config' was here
When calculating HTTP request buffer size tas_connect() unnecessarily adds
an extra octet for the terminating NULL byte.
The terminating NULL was already accounted for by sizeof(fmt), however,
since sizeof("123") = 4.
The extra NULL byte at the end of the anchor fetch HTTP request resulted
in an extra "501 Not implemented" HTTP response from the trust anchor
server.
tas_doc_read() uses a very short 50 msec network read timeout which makes
fetching trust anchors pretty much impossible on high-latency connections
like 3G.
Use a 2 second read timeout, just like the other tas_read_cb() callback
setter does.
Rosen Penev
Willem Toorop
Maciej S. Szmigiero
Eli Lindsey
Jim Hague
Renaud Allard
Jim Hague
Zane Zakraisek
Sara Dickinson
Vladislav Grishenko