Diego Fronza [Thu, 10 Sep 2020 18:33:15 +0000 (15:33 -0300)]
Added test for the proposed fix
This test is very simple, two nameserver instances are created:
- ns4: master, with 'minimal-responses yes', authoritative
for example. zone
- ns5: slave, stub zone
The first thing verified is the transfer of zone data from master
to slave, which should be saved in ns5/example.db.
After that, a query is issued to ns5 asking for target.example.
TXT, a record present in the master database with the "test" string
as content.
If that query works, it means stub zone successfully request
nameserver addresses from master, ns4.example. A/AAAA
The presence of both A/AAAA records for ns4 is also verified in the
stub zone local file, ns5/example.db.
Diego Fronza [Thu, 10 Sep 2020 18:09:14 +0000 (15:09 -0300)]
Fix transfer of glue records in stub zones if master has minimal-responses set
Stub zones don't make use of AXFR/IXFR for the transfering of zone
data, instead, a single query is issued to the master asking for
their nameserver records (NS).
That works fine unless master is configured with 'minimal-responses'
set to yes, in which case glue records are not provided by master
in the answer with nameservers authoritative for the zone, leaving
stub zones with incomplete databases.
This commit fix this problem in a simple way, when the answer with
the authoritative nameservers is received from master (stub_callback),
for each nameserver listed (save_nsrrset), a A and AAAA records for
the name is verified in the additional section, and if not present
a query is created to resolve the corresponsing missing glue.
A struct 'stub_cb_args' was added to keep relevant information for
performing a query, like TSIG key, udp size, dscp value, etc, this
information is borrowed from, and created within function 'ns_query',
where the resolving of nameserver from master starts.
A new field was added to the struct 'dns_stub', an atomic integer,
namely pending_requests, which is used to keep how many queries are
created when resolving nameserver addresses that were missing in
the glue.
When the value of pending_requests is zero we know we can release
resources, adjust zone timers, dump to zone file, etc.
Michal Nowak [Mon, 19 Oct 2020 05:47:57 +0000 (07:47 +0200)]
Run unit tests on OpenBSD in GitLab CI
Unlike other maintained BIND branches, the "main" BIND branch does not
require Kyua for running unit tests, which has been an obstacle for
adding an OpenBSD unit test job to GitLab CI. Experiments show that a
complete BIND unit test suite completes in a few minutes on OpenBSD and
that unit tests are not as severely affected by OpenBSD performance
issues as system tests are. Add a GitLab CI job which runs unit tests
on OpenBSD to every pipeline.
Diego Fronza [Thu, 1 Oct 2020 17:04:05 +0000 (14:04 -0300)]
Fix dnstap system test on FreeBSD
This commit ensures that dnstap output files captured
by fstrm_capture are properly flushed before any attempt
on reading them with dnstap-read is done.
By reading fstrm-capture source code it was noticed that
signal SIGHUP is used to flush the capture file.
Matthijs Mekking [Tue, 20 Oct 2020 08:57:16 +0000 (10:57 +0200)]
Don't increment network error stats on UV_EOF
When networking statistics was added to the netmgr (in commit 5234a8e00a6ae1df738020f27544594ccb8d5215), two lines were added that
increment the 'STATID_RECVFAIL' statistic: One if 'uv_read_start'
fails and one at the end of the 'read_cb'. The latter happens
if 'nread < 0'.
According to the libuv documentation, I/O read callbacks (such as for
files and sockets) are passed a parameter 'nread'. If 'nread' is less
than 0, there was an error and 'UV_EOF' is the end of file error, which
you may want to handle differently.
In other words, we should not treat EOF as a RECVFAIL error.
Mark Andrews [Wed, 16 Sep 2020 02:40:52 +0000 (12:40 +1000)]
Try to improve rrl timing
Add a +burst option to mdig so that we have a second to setup the
mdig calls then they run at the start of the next second.
RRL uses 'queries in a second' as a approximation to
'queries per second'. Getting the bursts of traffic to all happen in
the same second should prevent false negatives in the system test.
We now have a second to setup the traffic in. Then the traffic should
be sent at the start of the next second. If that still fails we
should move to +burst=<now+2> (further extend mdig) instead of the
implicit <now+1> as the trigger second.
Mark Andrews [Mon, 12 Oct 2020 06:51:09 +0000 (17:51 +1100)]
Complete the isc_nmhandle_detach() in the worker thread.
isc_nmhandle_detach() needs to complete in the same thread
as shutdown_walk_cb() to avoid a race. Clear the caller's
pointer then pass control to the worker if necessary.
WARNING: ThreadSanitizer: data race
Write of size 8 at 0x000000000001 by thread T1:
#0 isc_nmhandle_detach lib/isc/netmgr/netmgr.c:1258:15
#1 control_command bin/named/controlconf.c:388:3
#2 dispatch lib/isc/task.c:1152:7
#3 run lib/isc/task.c:1344:2
Clone the csock in accept_connection(), not in callback
If we clone the csock (children socket) in TCP accept_connection()
instead of passing the ssock (server socket) to the call back and
cloning it there we unbreak the assumption that every socket is handled
inside it's own worker thread and therefore we can get rid of (at least)
callback locking.
Ondřej Surý [Fri, 2 Oct 2020 07:28:29 +0000 (09:28 +0200)]
Change the isc__nm_tcpdns_stoplistening() to be asynchronous event
The isc__nm_tcpdns_stoplistening() would call isc__nmsocket_clearcb()
that would clear the .accept_cb from non-netmgr thread. Change the
tcpdns_stoplistening to enqueue ievent that would get processed in the
right netmgr thread to avoid locking.
Mark Andrews [Fri, 2 Oct 2020 06:17:51 +0000 (16:17 +1000)]
Fix the data race on shutdown/reconfig in control channel
The controllistener could be freed before the event posted by
isc_nm_stoplistening() has been processed. This commit adds
a reference counter to the controllistener to determine when
to free the listener.
Adjust legacy and digdelv tests for default 1232 EDNS Buffer Size
* the legacy test with -T maxudp512 will just fail, e.g. if the packets
larger than 512 octets are dropped along the path, the proper response
is to fail
* digdelv test was just expecting default server EDNS buffer size to be
4096, the test needed only slight adjustment
Simplify the EDNS buffer size logic for DNS Flag Day 2020
The DNS Flag Day 2020 aims to remove the IP fragmentation problem from
the UDP DNS communication. In this commit, we implement the required
changes and simplify the logic for picking the EDNS Buffer Size.
1. The defaults for `edns-udp-size`, `max-udp-size` and
`nocookie-udp-size` have been changed to `1232` (the value picked by
DNS Flag Day 2020).
2. The probing heuristics that would try 512->4096->1432->1232 buffer
sizes has been removed and the resolver will always use just the
`edns-udp-size` value.
3. Instead of just disabling the PMTUD mechanism on the UDP sockets, we
now set IP_DONTFRAG (IPV6_DONTFRAG) flag. That means that the UDP
packets won't get ever fragmented. If the ICMP packets are lost the
UDP will just timeout and eventually be retried over TCP.
Ondřej Surý [Mon, 5 Oct 2020 11:14:04 +0000 (13:14 +0200)]
Split reusing the addr/port and load-balancing socket options
The SO_REUSEADDR, SO_REUSEPORT and SO_REUSEPORT_LB has different meaning
on different platform. In this commit, we split the function to set the
reuse of address/port and setting the load-balancing into separate
functions.
The libuv library already have multiplatform support for setting
SO_REUSEADDR and SO_REUSEPORT that allows binding to the same address
and port, but unfortunately, when used after the load-balancing socket
options have been already set, it overrides the previous setting, so we
need our own helper function to enable the SO_REUSEADDR/SO_REUSEPORT
first and then enable the load-balancing socket option.
Ondřej Surý [Mon, 5 Oct 2020 10:25:19 +0000 (12:25 +0200)]
Use uv_os_sock_t instead of uv_os_fd_t for sockets
On POSIX based systems both uv_os_sock_t and uv_os_fd_t are both typedef
to int. That's not true on Windows, where uv_os_sock_t is SOCKET and
uv_os_fd_t is HANDLE and they differ in level of indirection.
Ondřej Surý [Mon, 5 Oct 2020 09:17:52 +0000 (11:17 +0200)]
Refactor isc__nm_socket_freebind() to take fd and sa_family as args
The isc__nm_socket_freebind() has been refactored to match other
isc__nm_socket_...() helper functions and take uv_os_fd_t and
sa_family_t as function arguments.
Matthijs Mekking [Mon, 24 Aug 2020 08:05:28 +0000 (10:05 +0200)]
Various rndc dnssec -checkds fixes
While working on 'rndc dnssec -rollover' I noticed the following
(small) issues:
- The key files where updated with hints set to "-when" and that
should always be "now.
- The kasp system test did not properly update the test number when
calling 'rndc dnssec -checkds' (and ensuring that works).
- There was a missing ']' in the rndc.c help output.
Matthijs Mekking [Fri, 21 Aug 2020 13:31:57 +0000 (15:31 +0200)]
Add manual key rollover logic
Add to the keymgr a function that will schedule a rollover. This
basically means setting the time when the key needs to retire,
and updating the key lifetime, then update the state file. The next
time that named runs the keymgr the new lifetime will be taken into
account.
After backporting #1870 to 9.11-S I saw that the condition check there
is different than in the main branch. In 9.11-S "stale" can mean
stale and serve-stale, or not active (awaiting cleanup). In 9.16 and
later versions, "stale" is stale and serve-stale, and "ancient" means
not active (awaiting cleanup). An "ancient" RRset is one that is not
active (TTL expired) and is not eligble for serve-stale.
Update the condition for rndc dumpdb -expired to closer match what is
in 9.11-S.
Sometimes, not all keys have been created in time before 'check_keys'
is called. Run a 'retry_quiet' on checking the number of keys before
continuing checking the key data.
The kasp code had bad implicit size values for the cryptographic
algorithms Ed25519 and Ed448. When creating keys they would never
match the dnssec-policy, leading to new attempts to create keys.
These algorithms were previously not yet added to the system tests,
due to lack of availability on some systems.
Use the testcrypto script to see if these algorithms are supported by
openssl. If so, add the specific configuration to the named.conf file
and touch a file to indicate support. If the file exists, the
corresponding setup and tests are performed.
Michał Kępień [Fri, 2 Oct 2020 06:41:43 +0000 (08:41 +0200)]
Rework "rrset-order" documentation
Certain parts of the existing documentation for the "rrset-order"
statement are incorrect, others are ambiguous. Rework the relevant
section of the ARM to make it clear and up-to-date with the source code.
Michał Kępień [Fri, 2 Oct 2020 06:41:43 +0000 (08:41 +0200)]
Add tests for "order none" RRset ordering rules
Make sure "order none" RRset ordering rules are tested in the
"rrsetorder" system test just like all other rule types are. As the
check for the case of no "rrset-order" rule matching a given RRset also
tests "order none" (rather than "order random", as the test code may
suggest at first glance), replace the test code for that case so that it
matches other "order none" tests.
Add DNS_RDATASETATTR_NONE to the list of RRset ordering modes accepted
by dns_order_add() to allow "order none" to be used in "rrset-order"
rules. This both prevents the aforementioned crashes and addresses the
discrepancy between named-checkconf and named.
Remove the .key from the beginning of the line in rst file
The handling of . (dot) characted at the beginning of the line has
changed between the sphinx-doc versions, and it was constantly giving us
trouble when generating man pages when using different sphinx-doc. This
commit just changes the source rst file, so there's no more . (dot) the
beginning of the line.
Mark Andrews [Mon, 28 Sep 2020 02:54:17 +0000 (12:54 +1000)]
Always clean sig0name in msgresetsigs() and dns_message_renderreset()
The fuzzing harness operates on dns_message_t in non-standard ways
and if 'sig0name' is non-NULL when msgresetsigs() and
dns_message_renderreset() are called it should be cleaned up.
The dns_message_create() cannot fail, change the return to void
The dns_message_create() function cannot soft fail (as all memory
allocations either succeed or cause abort), so we change the function to
return void and cleanup the calls.
Diego Fronza [Mon, 21 Sep 2020 21:19:49 +0000 (18:19 -0300)]
cocci: Add semantic patch to refactor dns_message_destroy()
dns_message_t objects are now being handled using reference counting
semantics, so now dns_message_destroy() is not called directly anymore,
dns_message_detach must be called instead.
Diego Fronza [Mon, 21 Sep 2020 20:44:29 +0000 (17:44 -0300)]
Properly handling dns_message_t shared references
This commit fix the problems that arose when moving the dns_message_t
object from fetchctx_t to the query structure.
Since the lifetime of query objects are different than that of a fetchctx
and the dns_message_t object held by the query may be being used by some
external module, e.g. validator, even after the query may have been destroyed,
propery handling of the references to the message were added in this commit to
avoid accessing an already destroyed object.
Specifically, in rctx_done(), a reference to the message is attached at
the beginning of the function and detached at the end, since a possible call
to fctx_cancelquery() would release the dns_message_t object, and in the next
lines of code a call to rctx_nextserver() or rctx_chaseds() would require
a valid pointer to the same object.
In valcreate() a new reference is attached to the message object, this
ensures that if the corresponding query object is destroyed before the
validator attempts to access it, no invalid pointer access occurs.
In validated() we have to attach a new reference to the message, since
we destroy the validator object at the beginning of the function,
and we need access to the message in the next lines of the same function.
rctx_nextserver() and rctx_chaseds() functions were adapted to receive
a new parameter of dns_message_t* type, this was so they could receive a
valid reference to a dns_message_t since using the response context respctx_t
to access the message through rctx->query->rmessage could lead to an already
released reference due to the query being canceled.
projects / thirdparty / bind9.git / log
