this change addresses an issue with DNS cookie invalidation for users of
the legacy API and custom network stacks (e.g., Seastar's DNS).
in 02745437, we added an optional `agetsockname` to
`ares_socket_functions_ex` struct, and intended to skip calling this
function in DNS cookie invalidation if this function is not available
for legacy API users.
but we are still calling this system call directly, this fails Seastar's
DNS which uses c-ares to resolve names, and it uses the socket function
table to override the network stack to use its own userspace TCP/IP
stack, in that case, the fd passed to c-ares is not a valid file
descriptor, hence the call fails.
in this change, we
- replace direct getsockname() call with sock_funcs.agetsockname()
- skip the call if agetsockname() is not configured
Refs 02745437
Signed-off-by: Kefu Chai (@tchaikov)
Some external users swap out the entire IP stack, such as
[Seastar](https://github.com/scylladb/seastar) that uses DPDK. So we
need to allow them to override all network stack functions used by
c-ares. Since we don't know what network functions may be needed in the
future, we need to make a versioned structure that can be expanded.
We'll need to disable any features in the future if the versioned
interface is less than is required to support any new features.
We recently had a regression when we added DNS Cookie support as we
relied on `getsockname()` to get the local source ip address, but we
were calling the native version since there was no registered callback
available. We will mark this as optional and just skip this step in DNS
Cookie invalidation if its not available for the legacy api users.
Supersedes #893
Authored-By: Brad House (@bradh352)
Invalid arguments might be silently ignored if they were determined
they might be valid hostnames. Also it was common for people to
specify `-s server` in old c-ares versions out of habbit, lets
re-add that functionality since it doesn't conflict with bind dig
options.
Authored-By: Brad House (@bradh352)
When got a truncated UDP packet and switch the query to TCP, use the
same port.
Fallback TCP port was always default 53.
Tested on e037002. Also existed before a41b85e.
To reproduce it, on Windows:
1. Build and launch a "parrot" DNS server, which just echo back the
query with `data[3] |= 0x82`, meaning with flag truncated is true. For
example on: `127.0.0.2:33053`.
2. Launch another DNS server, on `127.0.0.2:53`. Or use a packet capture
tool, like Wireshark.
3. Run `adig @127.0.0.2 -p 33053 google.com`. You will see the log
appears on TCP port 53.
Using a non-default port DNS server, `adig @127.0.0.2 -p 33053 -t txt
gitlab.com` is very likely to trigger it, as the answer would be very
long (rcvd: 1465).
Authored-By: @lifenjoiner
The c-ares dig utility outputs a similar format to the bind dig utility,
we should also make it take arguments that mimic theirs for
compatibility.
This PR starts down that path supporting some of the major options.
Fixes#885
Authored-By: Brad House (@bradh352)
If you bind to a local address, you now only have approx 32k possible
source ports to initiate connections.
In modern days that can quickly run out.
setting IP_BIND_ADDRESS_NO_PORT let's the kernel choose a port at
connect time, increasing the limit of combinations to around a million.
Authored-By: Cristian Rodríguez (@crrodriguez)
Some of the new library functions didn't have any specific test cases
or were very limited. Lets add a few more to improve code coverage.
There's still a lot to go, but this is a good start.
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)
The DNS server format is insufficient for future configurations, such as
supporting DNS over TLS (DoT) and DNS over HTTPS (DoH), as well as
additional functionality such as domain-specific servers. Already, in
the case where different UDP and TCP ports are used, it is impossible to
represent in the current format.
In order to try to use some standardized format, we are going to define
our own URI schemes that should be parse-able by any URI parser. The new
scheme will only be used when the configuration cannot otherwise be
expressed using the current `ipaddr%iface:port` format, which is the
format used as the nameserver configuration in `/etc/resolv.conf`.
However, the parser `ares_set_servers_csv()` shall accept the new URI
scheme format even when it is not necessary.
This PR implements a URI parser and writer and hooks the basic usage
into `ares_set_servers_csv()` and `ares_get_servers_csv()` as well as
provides updated documentation in the relevant manpages.
We will define these URI schemes:
* `dns://` - Normal DNS server (UDP + TCP). We need to be careful not to
conflict with query params defined in
https://datatracker.ietf.org/doc/html/rfc4501 since we'd technically be
extending this URI scheme. Port defaults to `53`.
* `dns+tls://` - DNS over TLS. Port defaults to `853`.
* `dns+https://` - DNS over HTTPS. Port defaults to `443`.
We initially will define these query parameters (additional arguments
may be required in the future to specify options such as TLS certificate
validation rules):
* `tcpport` - TCP port to use, only for `dns://` scheme. The `port`
specified as part of the `authority` component of the URI will be used
for both UDP and TCP by default, this option will override the TCP port.
* `ipaddr` - Only for `dns+tls://` and `dns+https://`. If the
`authority` component of the URI contains a hostname, this is used to
specify the ip address of the hostname. If not specified, will need to
use a non-secure server to perform a DNS lookup to retrieve this
information. It is always recommended to have both the ip address and
fully qualified domain name specified.
* `hostname` - Only for `dns+tls://` and `dns+https://`. If the
`authority` component of the URI contains an ip address, this is used to
specify the fully qualified domain name of the server. If not specified,
will need to use a non-secure server to perform a DNS reverse lookup to
retrieve this information. It is always recommended to have both the ip
address and fully qualified domain name specified.
* `domain` - If specified, this server is a domain-specific server. Any
queries for this domain will be routed to this server. Multiple servers
may be tagged with the same domain.
Examples:
```
dns://8.8.8.8
dns://[2001:4860:4860::8888]
dns://[fe80::b542:84df:1719:65e3%en0]
dns://192.168.1.1:55
dns://192.168.1.1?tcpport=1153
dns://10.0.1.1?domain=myvpn.com
dns+tls://8.8.8.8?hostname=dns.google
dns+tls://one.one.one.one?ipaddr=1.1.1.1
```
NOTE: While we are defining the scheme for things like domain-specific
servers, DNS over TLS and DNS over HTTPS, the underlying implementations
for those features do not yet exist and therefore will result in errors
if they are attempted to be used.
### Non-compliance in implementation
All these could be easily implemented/fixed if desired, however any such
changes would be of no use to the current c-ares usage of URIs:
* Does not currently support relative references
* Requires use of the authority section, blank is not allowed
* The query string is interpreted to be in
[application/x-www-form-urlencoded](https://en.wikipedia.org/wiki/Application/x-www-form-urlencoded)
format only and will result in parse errors if it is not. This is the
most common format used, however technically not valid to mandate this
format is used. We could add flags in the future to treat the query
string as opaque and leave it to the user to process. Or we could
internally have a list of schemes that use this format.
* [IDNA](https://en.wikipedia.org/wiki/Internationalized_domain_name) is
not supported.
* Does not support hex-encoded IPv4 addresses (this is compliant with RFC3986, but not WHATWG)
Authored-By: Brad House (@bradh352)
The solaris images have been updated to be a bit more modernized. Switch
to using the new image and enable the use of running tests through GDB
to be able to print backtraces in case of failures.
Authored-By: Brad House (@bradh352)
ares_platform.c contains some legacy code to support things like Windows
9x and Windows CE 6, those have long since been discontinued.
Lets kill these legacy files to clean up some cruft.
Signed-off-by: Brad House (@bradh352)
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)
Brad House
Kefu Chai
lifenjoiner
Cristian Rodríguez