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)
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)
c-ares is getting larger these days and we keep adding source files to
the same directory so it can be hard to differentiate core c-ares
implementation from library/utility functions. Lets make some
subdirectories to help with that and shuffle files around.
Fix By: Brad House (@bradh352)
UDP is connectionless, but systems use ICMP unreachable messages to
indicate there is no ability to reach the host or port, which can result
in a `send()` returning an error like `ECONNREFUSED`. We need to handle
non-retryable codes like that to treat it as a connection failure so we
requeue any queries on that connection to another connection/server
immediately. Otherwise what happens is we just wait on the timeout to
expire which can greatly increase the time required to get a definitive
message.
This also adds a test case to verify the behavior.
Fixes#819
Fix By: Brad Houes (@bradh352)
c-ares utilizes recursion for some operations, and some of these
processes can have unintended side effects, such as if a callback
is called that then recurses into the same function. This can cause
strange cleanup conditions that lead to crashes.
Try to disassociate queries with connections as early as possible and
move cleaning up unneeded connections to its own scan rather than
trying to detect each time a query is disassociated from a connection.
Fix By: Brad House (@bradh352)
In c-ares 1.30.0 we started validating strings parsed are printable.
This caused a regression in a pycares test case due to a wrong response
code being returned as the error was being propagated from a different
section of code that was assuming the only possible failure condition
was out-of-memory.
This PR adds a fix for this and also a test case to validate it.
Ref: https://github.com/saghul/pycares/issues/200
Fix By: Brad House (@bradh352)
We've had reports of user-after-free type crashes in Windows cleanup
code for the Event Thread. In evaluating the code, it appeared there
were some memory leaks on per-connection handles that may have remained
open during shutdown, while trying to resolve that it became apparent
the methodology chosen may not have been the right one for interfacing
with the Windows AFD system as stability issues were seen during this
debugging process.
Since this system is completely undocumented, there was no clear
resolution path other than to switch to the *other* methodology which
involves directly opening `\Device\Afd`, rather than spawning a "peer
socket" to use to queue AFD operations.
The original methodology chosen more closely resembled what is employed
by [libuv](https://github.com/libuv/libuv) and given its widespread use
was the reason it was used. The new methodology more closely resembles
[wepoll](https://github.com/piscisaureus/wepoll).
Its not clear if there are any scalability or performance advantages or
disadvantages for either method. They both seem like different ways to
do the same thing, but this current way does seem more stable.
Fixes#798
Fix By: Brad House (@bradh352)
There was a thread passed data for processing that was cleaned up before
thread exit, and it could cause a use-after-free in the test suite. This
doesn't affect c-ares. This was found during trying to reproduce #798,
but appears unrelated, don't use a helper thread as it isn't necessary.
Fix By: Brad House (@bradh352)
As per #738, there are usecases where the DNS TXT record strings should
not be concatenated like RFC 7208 indicates. We cannot break ABI with
those using the new API, so we need to support retrieving the
concatenated version as well as a new API to retrieve the individual
strings which will be used by `ares_parse_text_reply_ext()` to restore
the old behavior prior to c-ares 1.20.
Fixes Issue: #738
Fix By: Brad House (@bradh352)
This PR enables DNS 0x20 as per
https://datatracker.ietf.org/doc/html/draft-vixie-dnsext-dns0x20-00 .
DNS 0x20 adds additional entropy to the request by randomly altering the
case of the DNS question to help prevent cache poisoning attacks.
Google DNS has implemented this support as of 2023, even though this is
a proposed and expired standard from 2008:
https://groups.google.com/g/public-dns-discuss/c/KxIDPOydA5M
There have been documented cases of name server and caching server
non-conformance, though it is expected to become more rare, especially
since Google has started using this.
This can be enabled via the `ARES_FLAG_DNS0x20` flag, which is currently
disabled by default. The test cases do however enable this flag to
validate this feature.
Implementors using this flag will notice that responses will retain the
mixed case, but since DNS names are case-insensitive, any proper
implementation should not be impacted.
There is currently no fallback mechanism implemented as it isn't
immediately clear how this may affect a stub resolver like c-ares where
we aren't querying the authoritative name server, but instead an
intermediate recursive resolver where some domains may return invalid
results while others return valid results, all while querying the same
nameserver. Likely using DNS cookies as suggested by #620 is a better
mechanism to fight cache poisoning attacks for stub resolvers.
TCP queries do not use this feature even if the `ARES_FLAG_DNS0x20` flag
is specified since they are not subject to cache poisoning attacks.
Fixes Issue: #795
Fix By: Brad House (@bradh352)
The header inclusion logic in c-ares is hard to follow. Lets try to
simplify the way it works to make it easier to understand and less
likely to break on new code changes. There's still more work to be done,
but this is a good start at simplifying things.
Fix By: Brad House (@bradh352)
MSVC has been building with /W3 which isn't considered a safe level for
modern code. /W4 is recommended, but it too is lacking some recommended
options, so we enable /W4 and also the recommended options. We do,
however, have to disable a couple of options due to Windows headers not
being fully compliant sometimes as well as some things we do in c-ares
that it doesn't like, but aren't actually bad.
Fix By: Brad House (@bradh352)
The query cache should be enabled by default. This will help with
determining proper timeouts for #736. It can still be disabled by
setting the ttl to 0. There should be no negative consequences of this
in real-world scenarios since DNS is based on the TTL concept and
upstream servers will cache results and not recurse based on this
information anyhow. DNS queries and responses are very small, this
should have negligible impact on memory consumption.
Fix By: Brad House (@bradh352)
Add code annotations for ignoring specific code paths for coverage
calculations. The primary purpose of this is to make it easy to see the
code paths that we could (and probably should) write test cases for, as
these would have the most impact on delivery of a stable product.
The annotations used are:
`LCOV_EXCL_LINE: <designation>`, `LCOV_EXCL_START: <designation>`,
`LCOV_EXCL_STOP`
Unfortunately `LCOV_EXCL_BR_LINE` does not appear to be supported by
coveralls as it would have been a more elegant solution over START/STOP.
We specifically include the `<designation>` not just for future
reference but because it makes it easy to identify in case we want to
address these conditions in a different way in the future.
The main areas designated for exclusion are:
1. `OutOfMemory` - these are hard to test cases, and on modern systems,
are likely to never occur due to optimistic memory allocations, which
can then later cause the kernel to terminate your application due to
memory not actually being available. c-ares does have *some* testing
framework for this, if we wish to expand in the future, we can easily
use sed to get rid of of these annotations.
2. `DefensiveCoding` - these are impossible to reach paths at the point
in time the code was written. They are there for defensive coding in
case code is refactored in the future to prevent unexpected behavior.
3. `UntestablePath` - these are code paths that aren't possible to test,
such as failure of a system call.
4. `FallbackCode` - This is an entire set of code that is untestable
because its not able to simulate a failure of the primary path.
This PR also does add some actual coverage in the test cases where it is
easy to do.
Fix By: Brad House (@bradh352)
With the current c-ares parser, as per PR #765 parsing was broken due to
validation that didn't understand the `SIG` record class. This PR adds
basic, non validating, and incomplete support for the `SIG` record type.
The additional `KEY` and `NXT` which would be required for additional
verification of the records is not implemented. It also does not store
the raw unprocessed RR data that would be required for the validation.
The primary purpose of this PR is to be able to recognize the record and
handle some periphery aspects such as validation of the class associated
with the RR and to not honor the TTL in the RR in the c-ares query cache
since it will always be 0.
Fixes#765
Fix By: Brad House (@bradh352)
As per Issue #760, the use of `struct timeval` is meant for only time
differentials, however it could be used to denote an exact timeout. This
could lead to y2k38 issues on some platforms.
Fixes Issue #760
Fix By: Brad House (@bradh352)
Automatically detect configuration changes and reload. On systems which
provide notification mechanisms, use those, otherwise fallback to
polling. When a system configuration change is detected, it
asynchronously applies the configuration in order to ensure it is a
non-blocking operation for any queries which may still be being
processed.
On Windows, however, changes aren't detected if a user manually
sets/changes the DNS servers on an interface, it doesn't appear there is
any mechanism capable of this. We are relying on
`NotifyIpInterfaceChange()` for notifications.
Fixes Issue: #613
Fix By: Brad House (@bradh352)
**Summary**
This PR adds a server state callback that is invoked whenever a query to
a DNS server finishes.
The callback is invoked with the server details (as a string), a boolean
indicating whether the query succeeded or failed, flags describing the
query (currently just indicating whether TCP or UDP was used), and
custom userdata.
This can be used by user applications to gain observability into DNS
server health and usage. For example, alerts when a DNS server
fails/recovers or metrics to track how often a DNS server is used and
responds successfully.
**Testing**
Three new regression tests `MockChannelTest.ServStateCallback*` have
been added to test the new callback in different success/failure
scenarios.
Fix By: Oliver Welsh (@oliverwelsh)
Improve reliability in the server retry delay regression tests by
increasing the retry delay and sleeping for a little more than the retry
delay when attempting to force retries.
This helps to account for unreliable timing (e.g. NTP slew)
intermittently breaking pipelines.
Fix By: Oliver Welsh (@oliverwelsh)
**Summary**
By default c-ares will select the server with the least number of
consecutive failures when sending a query. However, this means that if a
server temporarily goes down and hits failures (e.g. a transient network
issue), then that server will never be retried until all other servers
hit the same number of failures.
This is an issue if the failed server is preferred to other servers in
the list. For example if a primary server and a backup server are
configured.
This PR adds new server failover retry behavior, where failed servers
are retried with small probability after a minimum delay has passed. The
probability and minimum delay are configurable via the
`ARES_OPT_SERVER_FAILOVER` option. By default c-ares will use a
probability of 10% and a minimum delay of 5 seconds.
In addition, this PR includes a small change to always close out
connections to servers which have hit failures, even with
`ARES_FLAG_STAYOPEN`. It's possible that resetting the connection can
resolve some server issues (e.g. by resetting the source port).
**Testing**
A new set of regression tests have been added to test the new server
failover retry behavior.
Fixes Issue: #717
Fix By: Oliver Welsh (@oliverwelsh)
As per Issue #734 some people use `ndots:0` in their configuration which
is allowed by the system resolver but not by c-ares. Add support for
`ndots:0` and add a test case to validate this behavior.
Fixes Issue: #734
Fix By: Brad House (@bradh352)
Brad House
Gregor Jasny
Oliver Welsh