The autotools rewrite in c-ares 1.25 might have broken detection
of __system_property_get() which is needed on Android versions
prior to v8 in order to read DNS server settings.
This might fix#907
Authored-By: Brad House (@bradh352)
Some DNS servers don't properly ignore unknown EDNS options as the spec says they must, and instead will return EFORMERR.
See discussion roughly starting here: https://github.com/alpinelinux/docker-alpine/issues/366#issuecomment-2462530681
In this case the DNS server is known to support EDNS in general (as version prior to c-ares 1.33 worked which used EDNS), but when adding the EDNS DNS Cookie extension, they return EFORMERR. This is in violation of [RFC6891 6.1.2](https://datatracker.ietf.org/doc/html/rfc6891#section-6.1.2):
> Any OPTION-CODE values not understood by a responder or requestor MUST be ignored.
The server in this example actual echo's back the EDNS record further causing confusion that makes you think they might understand the record.
We need to catch an EFORMERR and re-attempt the query without EDNS completely since they are really non-compliant with EDNS. We may support additional EDNS extensions in the future and don't want to have to probe each individual extension with a braindead server.
Fixes#911
Authored-By: Brad House (@bradh352)
TSAN is warning about a thread concurrency issue that doesn't actually matter if the operation isn't atomic as its an optimization in this code path to skip timeout processing if we're shutting down due to ares_destroy().
Fix By: Jiwoo Park (@jimmy-park)
We added an optimization to stop retries on other address classes
on failures if one address class was received successfully. In
production, however, some odd misconfigured use cases could mean
an ipv6 address would be returned but the host was really only
capable of connecting to ipv4 machines.
We want to modify this optimization now to continue retries on
ipv4 even if ipv6 was received, but NOT the other way around.
It was always more likely that ipv6 resolution would cause the
delays due to system issues, as the world still really only
runs on ipv4...
Authored-By: Brad House (@bradh352)
Due to the way record duplication works, we might sometimes get a
misleading error code. Rewrite the error code to make better
sense.
Authored-By: Brad House (@bradh352)
As reported on Android 14, which recently added a fortify check for fcntl
F_SETFD to make sure only FD_CLOEXEC is ever passed:
dfe67d266c
We can't pass anything else to it. Even though on glibc, O_CLOEXEC
and FD_CLOEXEC are the same value, this isn't defined to be
portable in any way.
Fixes#900
Reported-By: Yuki San (@RealYukiSan)
Authored-By: Brad House (@bradh352)
As per Issue #883, an incorrect `libcares.pc` can be generated when
`CMAKE_THREAD_LIBS_INIT` contains a value like `-lpthread` because it
gets added to `libcares.pc` with another `-l` prefix. We can't control
the behavior of `CMAKE_THREAD_LIBS_INIT` since its set by `FIND_PACKAGE
(Threads)`.
Lets strip the `-l` prefix from the library before adding it to
`CARES_DEPENDENT_LIBS` which is used in the generation of `libcares.pc`.
Fixes#883
Reported-By: 前进，前进，进 (@leleliu008)
Fix By: Brad House (@bradh352)
Due to running containerized tests we had to cut and paste the `TEST_P`
macro definition in `googletest/include/gtest/gtest-param-test.h`, and
modify it as it isn't designed to be wrapped in any way. Unfortunately
this tends to change from release to release, usually in minor ways ...
but also google test specifically doesn't advertise its own version so
it can be hard to work around.
Lets try to fix compatibility with google test 1.15
Fixes#873
Authored-By: Brad House (@bradh352)
systemd-resolved will return `ESERVFAIL` or `EREFUSED` by default on
single label domain names. See
https://github.com/systemd/systemd/issues/34101
They've basically labeled this as a non-issue even though it appears to
be in violation of the RFCs for downstream systems being able to support
negative caching. I haven't tested with the suggested
`ResolveUnicastSingleLabel=yes`, but that's off by default so its
unlikely people even knows this exists.
Since systemd is very prevalent these days, we really don't have much of
a choice but to work around their design decisions. This PR implements
this support, but *only* on single label names as it likely isn't valid
otherwise. It also adds test cases to confirm this behavior.
Fixes#852
Authored-By: Brad House (@bradh352)
As per #852 searching is failing, partially it is due to the ndots value
not defaulting to a proper value on linux, and partially due to
systemd-resolved returning the wrong error codes.
This PR fixes the first issue and adds containerized test cases to
validate the behavior and prevent issues in the future.
Reported-By: Hans-Christian Egtvedt (@egtvedt) and Mikael Lindemann(@mikaellindemann)
Authored-By: Brad House (@bradh352)
If a blank DNS name is used, the DNS query cache would fail due to an
invalid sanity check. This can be legitimate such as:
adig -t SOA .
This fixes that situation as well as a few other spots that were
uncovered and adds a test case to validate the behavior to ensure
it won't regress in the future.
Fixes#858
Reported-By: Nodar Chkuaselidze (@nodech)
Authored-By: Brad House (@bradh352)
pkg-config uses the .private suffix for static linking to c-ares,
set the libraries and cflags necessary for this.
Authored-By: Christoph Reiter (@lazka)
Approved-By: Brad House (@bradh352)
During development we were initially using MSG_FASTOPEN but then
switched to TCP_FASTOPEN_CONNECT for Linux, but we left in the
build-time dependency on just MSG_FASTOPEN which predates
TCP_FASTOPEN_CONNECT. This causes build issues on legacy platforms
like CentOS 7.
Fixes#850
Reported-By: Erik Lax (@eriklax)
When native thread is attached to JVM, then it's name is taken from
JavaVMAttachArgs. When no JavaVMAttachArgs, or no JavaVMAttachArgs::name
passed, then JVM on its own decides on taming thread. Those names are
not descriptive. To preserve thread name, pass the currently set thread
name in JavaVMAttachArgs::name. pthread_getname_np was introduced in API
26, hence use more generic approach.
Fixes#837
Authored By: Yauheni Khnykin (@Hsilgos)
GitHub actions supports running tests on various docker containers, move
Ubuntu 20.04 and Alpine tests to containers. Also move iOS testing to
GitHub actions since that runs on MacOS which is supported.
This should take additional load off of Cirrus-CI which consumes credits
like crazy. This leaves only FreeBSD and Linux ARM testing on Cirrus-CI.
Authored-By: Brad House (@bradh352)
Create a new data structure for a basic growable indexable array,
supporting the features you'd normally expect such as insert (at, last,
first), remove (at, last, first), and get (at, last, first). Internally
all data is stored in an appropriately sized array that can directly be
returned to the caller as a C array of the data type provided. The array
grows by powers of two, and has optimizations for head and tail
removals.
Array modifications can be risky (e.g. wrong reallocation sizes,
mis-sized memory moves, out-of-bounds access), so it makes sense to have
standardized code that is well tested.
The arrays used by the dns record parser / writer have been converted to
the new array data structure as have a few other instances.
Authored-By: Brad House (@bradh352)
TCP Fast Open (TFO) allows TCP connection establishment in 0-RTT when a
client and server have previously communicated. The SYN packet will also
contain the initial data packet from the client to the server. This
means there should be virtually no slowdown over UDP when both sides
support TCP FastOpen, which is unfortunately not always the case. For
instance, `1.1.1.1` appears to support TFO, however `8.8.8.8` does not.
This implementation supports Linux, Android, FreeBSD, MacOS, and iOS.
While Windows does have support for TCP FastOpen it does so via
completion APIs only, and that can't be used with polling APIs like used
by every other OS. We could implement it in the future if desired for
those using `ARES_OPT_EVENT_THREAD`, but it would probably require
adopting IOCP completely on Windows.
Sysctls are required to be set appropriately:
- Linux: `net.ipv4.tcp_fastopen`:
- `1` = client only (typically default)
- `2` = server only
- `3` = client and server
- MacOS: `net.inet.tcp.fastopen`
- `1` = client only
- `2` = server only
- `3` = client and server (typically default)
- FreeBSD: `net.inet.tcp.fastopen.server_enable` (boolean) and
`net.inet.tcp.fastopen.client_enable` (boolean)
This feature is always-on, when running on an OS with the capability
enabled. Though some middleboxes have impacted end-to-end TFO and caused
connectivity errors, all modern OSs perform automatic blackholing of IPs
that have issues with TFO. It is not expected this to cause any issues
in the modern day implementations.
This will also help with improving latency for future DoT and DoH
implementations.
Authored-By: Brad House (@bradh352)
Refactor some connection handling to reduce code duplication and to
unify the TCP and UDP codepaths a bit more. This will make some future
changes easier to make.
This also does some structure renaming to better conform with current
standards:
- `struct server_state` -> `ares_server_t`
- `struct server_connection` -> `ares_conn_t`
- `struct query` -> `ares_query_t`
Authored-by: Brad House (@bradh352)
When implementing ares_dns_rr_del_opt_byid() for DNS cookie support,
memory wasn't being zero'd out after removing an option, and when
ares_dns_rr_set_opt{_own}() was called, it would try to free the
address that still existed. Add a couple memset()'s to prevent
this issue.
Fixes#836
Fix By: Brad House (@bradh352)
During the implementation of server cookies, test cases were missing
to validate the server cookie in a prior reply was passed back, and
it turns out they were not.
This also adds tests for verification.
Fix By: Brad House (@bradh352)
DNS cookies are a simple form of learned mutual authentication supported
by most DNS server implementations these days and can help prevent DNS
Cache Poisoning attacks for clients and DNS amplification attacks for
servers.
Fixes#620
Fix By: Brad House (@bradh352)
When using EventThreads, the config change cleanup code might manipulate
the event update list if it uses file descriptors (such as on Linux).
This was being done without a lock. Rework the event enqueuing to handle
locking internally to prevent this and to simplify where it is used.
This was found by chance during an ASAN CI run.
Fix By: Brad House (@bradh352)
Brad House
Jiwoo Park
Yauheni Khnykin