The previous implementation would redirect a query to a failed server
based on a timeout and random chance per query. This could lead to
issues of having to deal with server timeout scenarios when the server
isn't back online yet causing latency issues. Instead, we should
continue to use the known good servers for the query itself, but spawn a
second query with the same question to a different downed server. That
query will be able to be processed in the background and potentially
bring the server back online.
Also, when using the `rotate` option, servers were previously chosen at
random from the complete list. This PR changes that to choose only from
the servers that share the same highest priority.
Authored-By: Brad House (@bradh352)
Split socket from connection functions for a more clear delineation in
functionality. This is needed to support future DNS over TLS.
Also makes the new ares_process_fds() return a status code so we can
detect out of memory conditions, this too is going to be needed for
handling TLS which is more likely to run into such a situation.
Finally, add in a new ares_htable_vpstr_t type and functions which will
be needed as part of TLS session lookups.
Authored-By: Brad House (@bradh352)
Add a new public function of `ares_process_fds()` to work around the
shortcomings of `ares_process_fd()` and `ares_process()`.
This new function allows integrators to specify more than one file
descriptor at a time in an array, with an event mask per file
descriptor. It also adds a flags parameter which can be used to skip
extra expensive processing if they may be calling the function multiple
times before waiting on more events.
This new function is used by the event thread, and all the other
functions internally call this new function simplifying the code
structure.
Authored-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)
At some point in time, internal non-public functions were prefixed with
`ares__` where as functions that may become public were prefixed with
just `ares_`. This was never very consistent. Organizing the code better
typically provides more benefit in this way, which we've made great
progress on.
All non-static symbols must contain the `ares_` prefix, and that should
be more than sufficient in preventing any sort of issues.
Authored-By: Brad House (@bradh352)
We've started to get into instances of implementing new features where
we define data types in ares_private.h but then need to reference them
in another header. This is just bad design in ares_private.h and
eventually everything should be moved out of there that might need to be
used in different code segments.
Authored-By: Brad House (@bradh352)
Though the fuzzing tests are in the tests directory, we should move
the FUZZING.md file itself to the toplevel to make it more visible.
Signed-off-by: Brad House (@bradh352)
We have been missing basic documentation on some of the features
in c-ares for a while. This document should be used to document any
features that need explanation for behavior analysis and how to use
said feature.
Authored-By: Brad House (@bradh352)
We modified the include path with recent changes, that isn't good for
OSS-Fuzz. Lets make the path relative for including some semi-public
headers to allow OSS-Fuzz to build again.
Fix By: Brad House (@bradh352)
The test case changes introduced a memory leak in the test itself
(not in c-ares). Fix this memory leak. Also move prior fuzzing
information into the new fuzzing document.
Fix By: Brad House (@bradh352)
The coverage of the fuzzer is fairly low as it was only testing the
parsing, not trying to read the data from the datastructure or writing
it back out as a DNS message. Lets extend, and also provide a
FUZZING.md to help people new to fuzzing get up and running.
Authored-By: Brad House (@bradh352)
We recently turned on a feature of SonarCloud to notify when the
ISO C limit of 31 character identifiers had been exceeded. This
commit fixes all non-public instances of such violations.
Authored-By: Brad House (@bradh352)
The new notify pending write feature name should be reduced as ISO C
specifies compilers can support unique identifiers of as few as 31
characters.
Since this feature has not yet been made public in a release we
should go ahead and fix this issue so we aren't stuck with it.
Authored-By: Brad House (@bradh352)
The legacy c-ares parsers such as ares_parse_txt_reply() and
ares_parse_srv_reply() historically all used different logic and
parsing.
As of c-ares 1.21.0 these are simply wrappers around a single parser, and
simply convert the parsed DNS response into the data structures the legacy
parsers used which is a small amount of code and not likely going to vary
based on the input data.
Instead, these days, it makes more sense to test the new parser directly
instead of calling it 10 or 11 times with the same input data to speed up
the number of iterations per second the fuzzer can perform.
We are keeping this legacy fuzzer test for historic reasons or if someone
finds them of use hidden behind a define of USE_LEGACY_PARSERS.
Authored-By: Brad House (@bradh352)
During compiling using MSVC and Ninja, its not uncommon to see errors
such as:
```
C:\projects\c-ares\test\ares_queryloop.c: fatal error C1041: cannot open program database 'C:\projects\c-ares\build\bin\vc140.pdb'; if multiple CL.EXE write to the same .PDB file, please use /FS
```
We see this as build failures in our AppVeyor runs.
The suggestion of using `/FS` is misleading as it already exists in the
compiler flags (and previous attempts to append them again were wrong).
The real issue I believe is that different output targets are being
compiled in parallel, but trying to use the same pdb file, but clearly
shouldn't be since they're different targets.
This CMake issue may be related:
https://gitlab.kitware.com/cmake/cmake/-/issues/20188
Authored-By: Brad House (@bradh352)
AppVeyor reports issues like "fatal error C1041: cannot open program database"
randomly. The /FS flag is supposed to correct this on highly parallel builds
like performed when using Ninja, but it looks like CMake already adds this.
Maybe appending it at the end will work as maybe some prior flag is
counteracting its effectiveness.
Fix By: Brad House (@bradh352)
This allows for default arguments to be provided through a user
configuration file, useful for avoiding repetitive passing of flags on
the command line when using adig frequently. The syntax within
$XDG_CONFIG_HOME/adigrc resembles the one from dig's ~/.digrc, meaning
any command line arguments can be provided, separated either by line
breaks or whitespaces.
---------
Authored-By: Georg Pfuetzenreuter <mail@georg-pfuetzenreuter.net>
We don't want people using ares_process() anymore due to issues/limitations
in select(). Lets also remove the example and reword the manpage.
Authored-By: Brad House (@bradh352)
For non-cmake/autotools builds WIN32_LEAN_AND_MEAN was being defined
in ares_build.h.dist, this can cause conflicts with projects that import c-ares
and already define this.
The Node.js upgrade to c-ares v1.32.2, causes compilation failures like the following:
```
../../third_party/electron_node/deps/cares/include\ares_build.h(168,11): err
or: 'WIN32_LEAN_AND_MEAN' macro redefined [-Werror,-Wmacro-redefined]
168 | # define WIN32_LEAN_AND_MEAN
| ^
<command line>(25,9): note: previous definition is here
25 | #define WIN32_LEAN_AND_MEAN 1
| ^
1 error generated.
[287 processes, 49437/51449 @ 48.5/s : 1018.562s] CC obj/third_party/electro
n_node/deps/cares/cares/ares__socket.obj
FAILED: obj/third_party/electron_node/deps/cares/cares/ares__socket.obj
```
Authored-By: Shelley Vohr (@codebytere)
* ares__buf_split_str: Ability to split a buf object into a C array of C
strings
* ares__array_insertdata_{at,last,first}: copy data provided into array
member as a helper
* ares_free_array: Free a normal C array with custom callback to free
each element
Authored-By: Brad House (@bradh352)
Basic character checks are mostly done with defines, but some were
functions. Move the outliers to defines as well.
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)
The tools like `adig` and `ahost` can take advantage of some of the
library functions within c-ares. We can't really split these library
functions into a helper library without possibly breaking users.
Technically if we could guarantee they rely on pkg-config or cmake
helpers we could make it work ... we can revisit that in the future,
maybe if we make a c-ares 2.0 where people might expect more breakage
(but I still wouldn't want to break API/ABI).
So what this does is it just exports some symbols in headers that aren't
distributed so end users aren't meant to use the symbols, but any
utilities or tests built by c-ares can. It does clutter up some of the
namespace, but all these symbols are guaranteed to be prefixed with
`ares_` so there shouldn't ever be symbol clashes due to this for end
users and its not so many symbols that it should affect any load times.
There will be **zero** API/ABI guarantees for these symbols. They can
change from release to release, this is ok since they are not public.
I'm not entirely thrilled with doing this solution, but I want to avoid
thing like hand-written parsers, such as is used in #856.
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)
The FNV1a implementation used in the hashtable threw away the
algorithm's offset basis in preference of the seed value to
ensure different instances of the hashtable produced different
output for the same keys to prevent against hash collision
attacks.
Throwing away the offset basis completely could lead to some
non-uniformity, so instead we are now XORing the seed and the
algorithm-defined offset basis as the new offset basis to use
which should rectify this.
In the current c-ares use cases, this will probably have little
to no impact, but future hash table uses may benefit from the
better distribution.
Authored-By: Brad House (@bradh352)
As per https://llvm.org/docs/LibFuzzer.html#fuzzer-friendly-build-mode
it is recommended to add a compiler flag of FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
to make it deterministic. This can aid in generating a smaller corpus
and make it more efficient in finding actual flaws.
Right now this primarily impacts:
- Anything using the ares random number generator, like ares__slist_*(),
DNS transaction ids, and dns client cookie values.
- ares__htable_*()
Authored-By: Brad House (@bradh352)
The main purpose of this PR is to modularize the communication library,
and streamline the code paths for TCP and UDP into a single flow. This
will help us add additional flows such as TLS, and also make sure these
communication functions return a known set of error codes so that
additional error codes can be added for new flows in the future.
It also adds a new optional callback of
`ares_set_notify_pending_write_callback()` that will assist in
aggregating data from multiple queries into a single write operation.
This doesn't apply to UDP, but on TCP and especially TLS in the future
this can be a significant win. This is automatically applied for the
Event Thread.
It also fixes a long standing issue if UDP connection became saturated
and started returning `EWOULDBLOCK` or `EAGAIN` it was treated as a
failure. Since this is more inline with the TCP code now, it will wait
on a write event to retry.
Finally there are additional cleanups due to not needing to be able to
retrieve socket errnos all over the place.
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)
Brad House
Georg
Shelley Vohr
Georg