In PRIVATEOID keys, the key data begins with a length byte followed
by an ASN.1 object identifier that indicates the cryptographic algorithm
to use. Previously, the length byte was not accounted for when
checking the contents of keys and signatures, which could have led
to interoperability problems with any zones signed using PRIVATEOID.
This has been fixed.
Closes#5270
Merge branch '5270-fix-check-private' into 'main'
See merge request isc-projects/bind9!10372
Reformat the section to be more consistent with the rest of the rndc
documentation and avoid using :program: directive which would needlessly
break rst links.
We were failing to account for the length byte before the OID.
See RFC 4034.
Algorithm number 254 is reserved for private use and will never be
assigned to a specific algorithm. The public key area in the DNSKEY
RR and the signature area in the RRSIG RR begin with an unsigned
length byte followed by a BER encoded Object Identifier (ISO OID) of
that length. The OID indicates the private algorithm in use, and the
remainder of the area is whatever is required by that algorithm.
Entities should only use OIDs they control to designate their private
algorithms.
When multiple nested DNS validations were destroyed out of order,
the EDE context could be freed before all EDE codes were copied,
which could cause an assertion failure. This has been fixed.
Closes#5213
Merge branch '5213-use-dns_ede_copy-in-dns_validator' into 'main'
See merge request isc-projects/bind9!10365
If the nested DNS validator ends up in the same fetch because of the
loops, the code could be copying the EDE codes from the same source EDE
context as the destination EDE context. Skip copying the EDE codes if
the source and the destination is the same.
Instead of passing the edectx from the fetchctx into all subvalidators,
make the ede context ownership explict for dns_resolver_createfetch()
callers, and copy the ede result codes from the children validators to
the parent when finishing the validation process.
Profiles show that an high amount of CPU time spent in memset.
By removing zero initalization of certain large buffers we improve
performance in certain authoritative workloads.
Closes#5159
Merge branch '5159-do-not-zero-qp-search-buffers' into 'main'
See merge request isc-projects/bind9!10058
Profiles show that an high amount of CPU time spent in memset.
By removing zero initalization of certain large buffers we improve
performance in certain authoritative workloads.
TSAN reports a lock-order-inversion (potential deadlock) issue in
`add_trace_entry()`.
While it is true that in one case a lock in the `isc_mem_t` structure is
locked first, and then a lock in the `FILE` structure is locked second,
and in the the second case it is the other way around, this isn't an
issue, because those are `FILE` structures for totally different files,
used in different parts of the code.
Closes#5266
Merge branch '5266-freebsd-suppress-tsan-lock-order-inversion-false-positive' into 'main'
See merge request isc-projects/bind9!10355
TSAN reports a lock-order-inversion (potential deadlock) issue in
add_trace_entry():
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock)
Cycle in lock order graph: M0001 (0x000000000001) => M0002 (0x000000000002) => M0001
Mutex M0002 acquired here while holding mutex M0001 in main thread:
#0 _pthread_mutex_lock /usr/src/contrib/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:1342:3
#1 add_trace_entry lib/isc/mem.c:210:2
#2 isc__mem_get lib/isc/mem.c:606:2
#3 isc_buffer_allocate lib/isc/./include/isc/buffer.h:1080:23
#4 pushandgrow lib/isc/lex.c:321:3
#5 isc_lex_gettoken lib/isc/lex.c:445:22
#6 cfg_gettoken lib/isccfg/parser.c:3490:11
#7 cfg_parse_mapbody lib/isccfg/parser.c:2230:3
#8 cfg_parse_obj lib/isccfg/parser.c:247:11
#9 parse2 lib/isccfg/parser.c:628:11
#10 cfg_parse_file lib/isccfg/parser.c:668:11
#11 load_configuration bin/named/server.c:8069:13
#12 run_server bin/named/server.c:9518:2
#13 isc__async_cb lib/isc/async.c:110:3
#14 uv__async_io /tmp/libuv-1.50.0/src/unix/async.c:208:5
#15 uv__io_poll /tmp/libuv-1.50.0/src/unix/kqueue.c:369:9
#16 uv_run /tmp/libuv-1.50.0/src/unix/core.c:460:5
#17 loop_thread lib/isc/loop.c:327:6
#18 thread_body lib/isc/thread.c:89:8
#19 isc_thread_main lib/isc/thread.c:124:2
#20 isc_loopmgr_run lib/isc/loop.c:513:2
#21 main bin/named/main.c:1469:2
Mutex M0001 previously acquired by the same thread here:
#0 _pthread_mutex_lock /usr/src/contrib/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:1342:3
#1 _flockfile /usr/src/lib/libc/stdio/_flock_stub.c:72:3
#2 cfg_gettoken lib/isccfg/parser.c:3490:11
#3 cfg_parse_mapbody lib/isccfg/parser.c:2230:3
#4 cfg_parse_obj lib/isccfg/parser.c:247:11
#5 parse2 lib/isccfg/parser.c:628:11
#6 cfg_parse_file lib/isccfg/parser.c:668:11
#7 load_configuration bin/named/server.c:8069:13
#8 run_server bin/named/server.c:9518:2
#9 isc__async_cb lib/isc/async.c:110:3
#10 uv__async_io /tmp/libuv-1.50.0/src/unix/async.c:208:5
#11 uv__io_poll /tmp/libuv-1.50.0/src/unix/kqueue.c:369:9
#12 uv_run /tmp/libuv-1.50.0/src/unix/core.c:460:5
#13 loop_thread lib/isc/loop.c:327:6
#14 thread_body lib/isc/thread.c:89:8
#15 isc_thread_main lib/isc/thread.c:124:2
#16 isc_loopmgr_run lib/isc/loop.c:513:2
#17 main bin/named/main.c:1469:2
Mutex M0001 acquired here while holding mutex M0002 in main thread:
#0 _pthread_mutex_lock /usr/src/contrib/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:1342:3
#1 _flockfile /usr/src/lib/libc/stdio/_flock_stub.c:72:3
#2 print_active lib/isc/mem.c:629:3
#3 isc_mem_stats lib/isc/mem.c:694:2
#4 main bin/named/main.c:1498:4
Mutex M0002 previously acquired by the same thread here:
#0 _pthread_mutex_lock /usr/src/contrib/llvm-project/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:1342:3
#1 isc_mem_stats lib/isc/mem.c:668:2
#2 main bin/named/main.c:1498:4
SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) lib/isc/mem.c:210:2 in add_trace_entry
In the first stack frame ('M0001'->'M0002' lock order) cfg_gettoken()
uses flockfile() to lock 'M0001' for the 'FILE' object associated with
the configuration file (e.g. the configuration file itself and
whatever it includes, like a zone database), then it locks a memory
context mutex M0002.
In the other stack frmae ('M0002'->'M0001' lock order) isc_mem_stats()
locks a memory context mutex M0002, then it uses fprintf(), which
internally locks a 'M0001' mutex with flockfile() to write into the
'named.memstats' memory statistics file.
While it is true that in one case a lock in the 'isc_mem_t' structure is
locked first, and then a lock in the 'FILE' structure is locked second,
and in the the second case it is the other way around, this isn't an
issue, because those are 'FILE' structures for totally different files,
used in different parts of the code.
It was also manually confirmed that 'named.memstats' doesn't get
processed by cfg_gettoken(), and is used only in the second stack
frame's code flow when named is exiting.
For pipelines in the private repository, add an optional manual job,
which allows the current branch to be pushed into the specified
customer's git repository. This can be useful to provide patch previews
for early testing.
For tags created in a private repository, add a manual job which pushes
the created tag to all entitled customers.
Merge branch 'nicki/ci-customer-git-automation' into 'main'
See merge request isc-projects/bind9!10323
For pipelines in the private repository, add an optional manual job,
which allows the current branch to be pushed into the specified
customer's git repository. This can be useful to provide patch previews
for early testing.
For tags created in a private repository, add a manual job which pushes
the created tag to all entitled customers.
TSAN reports a data race in FreeBSD's memset(), called by its
__crt_calloc() memory allocation function. There is a very similar
bug report [1] in FreeBSD bug tracker, and an existing code-review [2]
that tries to address an issue, the description of which is very
similar to what we are seeing.
Suppress this report by adding its signature to '.tsan-suppress'.
[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=282794
[2] https://reviews.freebsd.org/D28536?id=86694Closes#5267
Merge branch '5267-freebsd-suppress-tsan-data-race-false-positive' into 'main'
See merge request isc-projects/bind9!10353
TSAN reports a data race in FreeBSD's memset(), called by its
__crt_calloc() memory allocation function. There is a very similar
bug report [1] in FreeBSD bug tracker, and an existing code-review [2]
that tries to address an issue, the description of which is very
similar to what we are seeing.
Suppress this report by adding its signature to '.tsan-suppress'.
[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=282794
[2] https://reviews.freebsd.org/D28536?id=86694
The pattern `for (x = ISC_LIST_HEAD(...); x != NULL; ISC_LIST_NEXT(...)` has been changed to `ISC_LIST_FOREACH` throughout BIND, except in a few
cases where the change would be excessively complex.
In most cases this was a straightforward change. In some places, however, the list element variable was referenced after the loop ended. Where possible, code has now been refactored to avoid this necessity.
`ISC_LIST_FOREACH` has also been modified to use `typeof(list.head)` to declare list elements automatically. When the list object to be iterated is declared with a `const` qualifier, the qualifier is passed along to the element declaration, causing a compilation failure. To avoid this problem, some `const` qualifiers have been removed; where that was not possible, `UNCONST` was used.
Merge branch 'each-isc-list-foreach' into 'main'
See merge request isc-projects/bind9!10310
use the ISC_LIST_FOREACH pattern in places where lists had
been iterated using a different pattern from the typical
`for` loop: for example, `while (!ISC_LIST_EMPTY(...))` or
`while ((e = ISC_LIST_HEAD(...)) != NULL)`.
the pattern `for (x = ISC_LIST_HEAD(...); x != NULL; ISC_LIST_NEXT(...)`
has been changed to `ISC_LIST_FOREACH` throughout BIND, except in a few
cases where the change would be excessively complex.
in most cases this was a straightforward change. in some places,
however, the list element variable was referenced after the loop
ended, and the code was refactored to avoid this necessity.
also, because `ISC_LIST_FOREACH` uses typeof(list.head) to declare
the list elements, compilation failures can occur if the list object
has a `const` qualifier. some `const` qualifiers have been removed
from function parameters to avoid this problem, and where that was not
possible, `UNCONST` was used.
ISC_LIST_FOREACH and related macros now use 'typeof(list.head)' to
declare the list elements automatically; the caller no longer needs
to do so.
ISC_LIST_FOREACH_SAFE also now implicitly declares its own 'next'
pointer, so it only needs three parameters instead of four.
Clean up dead code following merge of !10302.
Closes#5262
Merge branch '5262-cleanup-dead-code-cid-548247' into 'main'
See merge request isc-projects/bind9!10346
Libedit is now ubiquitous and has a license compatible with
MPL 2.0. We are now dropping readline (GPL 3.0) and editline (obsolete) support
in favor of libedit.
Merge branch 'ondrej/cleanup-various-readline-libraries' into 'main'
See merge request isc-projects/bind9!10202
Previously the code would not check if the string intended to be used
for SNI is a hostname.
See also: !9923Closes#5225
Merge branch '5225-dig-sni-fix' into 'main'
See merge request isc-projects/bind9!10254
Add a function that checks if a 'hostname' is not a valid IPv4 or IPv6
address. Returns 'true' if the hostname is likely a domain name, and
'false' if it represents an IP address.
Previously changed mem_test (!10320) introduces a test which checks for
the value of `__FILE__`, which is different if the build is done
out-of-tree or not, even though this is not relevant for the test (only
the base filename is). This result in a broken test for out-of-tree
builds. Fix this by changing the way the "grep" is done in the test,
ignoring the optional path prefix in the filename.
Merge branch 'colin-fix-outoftree-memtest' into 'main'
See merge request isc-projects/bind9!10343
Previously changed mem_test (!10320) introduces a test which checks for
the value of `__FILE__`, which is different if the build is done
out-of-tree or not, even though this is not relevant for the test (only
the base filename is). This result in a broken test for out-of-tree
builds. Fix this by changing the way the "grep" is done in the test,
ignoring the optional path prefix in the filename.
A recent change to the dnssec system test depended on a file
that is only in the source tree, not in the build tree, and was
therefore not available in out-of-tree builds.
Merge branch 'each-fix-dnssec-oot' into 'main'
See merge request isc-projects/bind9!10342
A recent change to the dnssec system test depended on a file
that is only in the source tree, not in the build tree, and was
therefore not available in out-of-tree builds.
libsystemd, despite being useful, adds a huge surface area for just
using the sd_notify API. libsystemd's surface has been exploited in the
past [1].
Implement the systemd notification protocol by hand since it is just
sending newline-delimited datagrams to a UNIX socket. The code shouldn't
need more attention in the future since the notification protocol is
covered under systemd's stability promise [2].
We don't need to support VSOCK-backed service notifications since they
are only intended for virtual machine inits.
[1]: https://www.openwall.com/lists/oss-security/2024/03/29/4
[2]: https://systemd.io/PORTABILITY_AND_STABILITY/
When allocating memory under -m trace|record, the __FILE__ pointer is
stored, so it can be printed out later in order to figure out in which
file an allocation leaked. (among others, like the line number).
However named crashes when called with -m record and using a plugin
leaking memory. The reason is that plugins are unloaded earlier than
when the leaked allocations are dumped (obviously, as it's done as late
as possible). In such circumstances, `__FILE__` is dangling because the
dynamically loaded library (the plugin) is not in memory anymore.
Fix the crash by systematically copying the `__FILE__` string
instead of copying the pointer. Of course, this make each allocation to
consume a bit more memory (and longer, as it needs to calculate the
length of `__FILE__`) but this occurs only under -m trace|record debugging
flags.
Merge branch 'colin-memdump-plugins' into 'main'
See merge request isc-projects/bind9!10320
When allocating memory under -m trace|record, the __FILE__ pointer is
stored, so it can be printed out later in order to figure out in which
file an allocation leaked. (among others, like the line number).
However named crashes when called with -m record and using a plugin
leaking memory. The reason is that plugins are unloaded earlier than
when the leaked allocations are dumped (obviously, as it's done as late
as possible). In such circumstances, __FILE__ is dangling because the
dynamically loaded library (the plugin) is not in memory anymore.
Fix the crash by systematically copying the __FILE__ string
instead of copying the pointer. Of course, this make each allocation to
consume a bit more memory (and longer, as it needs to calculate the
length of __FILE__) but this occurs only under -m trace|record debugging
flags.
In term of unit test, because grepping in C is not fun, and because the
whole "syntax" of the dump output is tested in other tests, this simply
search for a substring in the whole buffer to make sure the expected
allocations are found.
The new ``reset-stats`` command for ``rndc`` allows some statistics
counters to be reset during runtime. At the moment only two "high-water"
counters are supported, so the ability to reset them after the
initial peaks during the server's "warm-up" phase may be useful for
some operators.
Closes#5251
Merge branch '5251-feature-rndc-reset-high-water-statistics' into 'main'
See merge request isc-projects/bind9!10318
This new rndc option allows to reset some statistics counters during
runtime. At this moment only the high-water type counters are supported
as such an ability to reset them after the initial peaks during the
server's "warm-up" phase can be useful for some operators.
