During a reconfiguration named wasn't reconfiguring catalog zones'
member zones. This has been fixed.
Closesisc-projects/bind9#4733
Merge branch '4733-fix-catz-member-zones-reconfigure' into 'v9.21.0-release'
See merge request isc-private/bind9!695
This condition in catalog zone processing functions is checked only
for sanity, so there is no meaning for a log message that normally
shouldn't be ever logged.
Catalog zone member zones should be reconfigured as all the other
zones during a reconfiguration. Test it by checking whether the newly
added allow-query setting affects a member zone.
The fcount_incr() was not increasing counter->count when force was set
to true, but fcount_decr() would try to decrease the counter leading to
underflow and assertion failure. Swap the order of the arguments in the
condition, so the !force is evaluated after incrementing the .count.
Closes#4846
Merge branch '4786-forced-fcount_incr-should-still-increment-count-and-allowed-fix' into 'main'
Closes#4846
See merge request isc-projects/bind9!9298
The fcount_incr() was not increasing counter->count when force was set
to true, but fcount_decr() would try to decrease the counter leading to
underflow and assertion failure. Swap the order of the arguments in the
condition, so the !force is evaluated after incrementing the .count.
LaTeX in CI and on ReadTheDocs [fails][1] to render a PDF version of ARM if
the Changelog section is included. The running theory is that the
verbatim section of more than twenty thousand lines is too big to meet
LaTeX self-imposed constraints, and it fails with:
! TeX capacity exceeded, sorry [main memory size=5000000].
Or it just hangs if extra_mem_bot=30000000 is set in
/etc/texmf/texmf.d/01main_memory_bump.cnf:
! Dimension too large.
\fb@put@frame ...p \ifdim \dimen@ >\ht \@tempboxa
\fb@putboxa #1\fb@afterfra...
l.56913 \end{sphinxVerbatim}
Make each BIND 9 release a separate code block to work around the issue.
Further split up the sections for some exceptionally large releases, for
the same reason.
[1]: https://gitlab.isc.org/isc-projects/bind9/-/jobs/4584011
Merge branch 'mnowak/fix-arm-changelog-section' into 'main'
See merge request isc-projects/bind9!9266
LaTeX in CI and on ReadTheDocs fails to render a PDF version of ARM if
the Changelog section is included. The running theory is that the
verbatim section of more than twenty thousand lines is too big to meet
LaTeX self-imposed constraints, and it fails with:
! TeX capacity exceeded, sorry [main memory size=5000000].
Or it just hangs if extra_mem_bot=30000000 is set in
/etc/texmf/texmf.d/01main_memory_bump.cnf:
! Dimension too large.
\fb@put@frame ...p \ifdim \dimen@ >\ht \@tempboxa
\fb@putboxa #1\fb@afterfra...
l.56913 \end{sphinxVerbatim}
Make each BIND 9 release a separate code block to work around the issue.
Further split up the sections for some exceptionally large releases, for
the same reason.
Since the enable_fips_mode() now resides inside the isc_tls unit, BIND 9
would fail to compile when FIPS mode was enabled as the DST subsystem
logging functions were missing.
Move the crypto library logging functions from the openssl_link unit to
isc_tls unit and enhance it, so it can now be used from both places
keeping the old dst__openssl_toresult* macros alive.
Merge branch 'ondrej/move-openssl-logging-to-isc_tls-unit' into 'main'
See merge request isc-projects/bind9!9286
Since the enable_fips_mode() now resides inside the isc_tls unit, BIND 9
would fail to compile when FIPS mode was enabled as the DST subsystem
logging functions were missing.
Move the crypto library logging functions from the openssl_link unit to
isc_tls unit and enhance it, so it can now be used from both places
keeping the old dst__openssl_toresult* macros alive.
There were cases in resolver.c when the `max-recursion-queries` quota was ineffective. It was possible to craft zones that would cause a resolver to waste resources by sending excessive queries while attempting to resolve a name. This has been addressed by correcting errors in the implementation of `max-recursion-queries`, and by reducing the default value from 100 to 32.
In addition, a new `max-query-restarts` option has been added which limits the number of times a recursive server will follow CNAME or DNAME records before terminating resolution. This was previously a hard-coded limit of 16, and now defaults to 11.
Closes#4741
Merge branch '4741-reclimit-restarts' into 'main'
Closes#4741
See merge request isc-projects/bind9!9281
implement, document, and test the 'max-query-restarts' option
which specifies the query restart limit - the number of times
we can follow CNAMEs before terminating resolution.
MAX_RESTARTS is no longer hard-coded; ns_server_setmaxrestarts()
and dns_client_setmaxrestarts() can now be used to modify the
max-restarts value at runtime. in both cases, the default is 11.
the number of steps that can be followed in a CNAME chain
before terminating the lookup has been reduced from 16 to 11.
(this is a hard-coded value, but will be made configurable later.)
there were cases in resolver.c when queries for NS records were
started without passing a pointer to the parent fetch's query counter;
as a result, the max-recursion-queries quota for those queries started
counting from zero, instead of sharing the limit for the parent fetch,
making the quota ineffective in some cases.
Use `[[noreturn]]` when compiling with C23 or greater.
The attribute macro name has been capitalized as `NORETURN` as defining it as `noreturn` breaks external headers.
`#define noreturn __attribute__((noreturn))` wasn't used as C11's `stdnoreturn.h`/`_Noreturn` is required to build BIND9 in the first place.
Merge branch 'aydin/noreturn-c23' into 'main'
See merge request isc-projects/bind9!9149
Instead of calling dst_lib_init() and dst_lib_destroy() explicitly by
all the programs, create a separate memory context for the DST subsystem
and use the library constructor and destructor to initialize the DST
internals.
Merge branch 'ondrej/move-dst_lib_init-and-destroy-to-lib-ctor-dtor' into 'main'
See merge request isc-projects/bind9!9254
Instead of calling dst_lib_init() and dst_lib_destroy() explicitly by
all the programs, create a separate memory context for the DST subsystem
and use the library constructor and destructor to initialize the DST
internals.
The new Fedora 40 TSAN images use libuv, urcu and OpenSSL libraries compiled with ThreadSanitizer. This (in theory) should enable better detection of memory races in those (most important) libraries.
Merge branch 'ondrej/test-new-tsan-images' into 'main'
See merge request isc-projects/bind9!9264
When the SSL object was destroyed, it would invalidate all SSL_SESSION
objects including the cached, but not yet used, TLS session objects.
Properly disassociate the SSL object from the SSL_SESSION before we
store it in the TLS session cache, so we can later destroy it without
invalidating the cached TLS sessions.
Closes#4834
Merge branch '4834-detach-SSL-from-cached-SSL_SESSION' into 'main'
Closes#4834
See merge request isc-projects/bind9!9271
When the SSL object was destroyed, it would invalidate all SSL_SESSION
objects including the cached, but not yet used, TLS session objects.
Properly disassociate the SSL object from the SSL_SESSION before we
store it in the TLS session cache, so we can later destroy it without
invalidating the cached TLS sessions.
Co-authored-by: Ondřej Surý <ondrej@isc.org>
Co-authored-by: Artem Boldariev <artem@isc.org>
Co-authored-by: Aram Sargsyan <aram@isc.org>
When TLS connection (TLSstream) connection was accepted, the children
listening socket was not attached to sock->server and thus it could have
been freed before all the accepted connections were actually closed.
In turn, this would cause us to call isc_tls_free() too soon - causing
cascade errors in pending SSL_read_ex() in the accepted connections.
Properly attach and detach the children listening socket when accepting
and closing the server connections.
Closes#4833
Merge branch '4833-tlssock-needs-to-attach-to-child-tlslistener' into 'main'
Closes#4833
See merge request isc-projects/bind9!9270
When TLS connection (TLSstream) connection was accepted, the children
listening socket was not attached to sock->server and thus it could have
been freed before all the accepted connections were actually closed.
In turn, this would cause us to call isc_tls_free() too soon - causing
cascade errors in pending SSL_read_ex() in the accepted connections.
Properly attach and detach the children listening socket when accepting
and closing the server connections.
Missing file util/dtrace.sh prevented builds on system without dtrace utility.
This has been corrected.
Fixes: #4835
Merge branch 'pspacek/gitattribute-fixes' into 'main'
Closes#4835
See merge request isc-projects/bind9!9262
Ensure that system tests can be executed without Python hypothesis
package.
Closes#4831
Merge branch '4831-isctest-make-hypothesis-optional' into 'main'
Closes#4831
See merge request isc-projects/bind9!9265
Query responses should contain the question section with some exceptions. Dig was not reporting this.
Closes#4808
Merge branch '4808-have-dig-report-missing-question-section-in-axfr-response' into 'main'
Closes#4808
See merge request isc-projects/bind9!9233
The question section should be present in the first AXFR/IXFR
response and in other QUERY responses unless no question was sent.
Issue a warning if the question section is not present.
The OpenSSL 1.x Engines support has been deprecated in the OpenSSL 3.x
and is going to be removed from the upstream OpenSSL. Remove the OpenSSL
Engine support from BIND 9 in favor of OpenSSL 3.x Providers.
Closes#4828
Merge branch '4828-remove-OpenSSL-engine-support' into 'main'
Closes#4828
See merge request isc-projects/bind9!9252
OpenSSL_version() first appeared in OpenSSL 1.1.0 and have been
available since LibreSSL 2.7.1 and OpenBSD 6.3, thus we can remove the
compatibility shims with older versions that are not supported anymore.
Since the support for OpenSSL Engines has been removed, we can now also
remove the checks for OPENSSL_API_LEVEL; The OpenSSL 3.x APIs will be
used when compiling with OpenSSL 3.x, and OpenSSL 1.1.xx APIs will be
used only when OpenSSL 1.1.x is used.
The OpenSSL 1.x Engines support has been deprecated in the OpenSSL 3.x
and is going to be removed. Remove the OpenSSL Engine support in favor
of OpenSSL Providers.
The isc_mem_create() in printversion() was created outside of an #ifdef
HAVE_GEOIP, but destroyed inside the #ifdef; move it to the outside of
the #ifdef where it belongs.
This is really a nit as we immediately exit() after printing the
versions, but I found it and it would bug me for the rest of my life.
Merge branch 'ondrej/fix-missing-isc_mem_destroy-in-printversion' into 'main'
See merge request isc-projects/bind9!9255
The isc_mem_create() in printversion() was created outside of an #ifdef
HAVE_GEOIP, but destroyed inside the #ifdef; move it to the outside of
the #ifdef where it belongs.
This is really a nit as we immediately exit() after printing the
versions, but I found it and it would bug me for the rest of my life.
Fix an assertion failure that could happen as a result of data race between free_gluetable() and addglue() on the same headers.
Closes#4691
Merge branch '4691-fix-data-race-between-free_gluetable-and-addglue' into 'main'
Closes#4691
See merge request isc-projects/bind9!9126
when searching the cache for a node so that we can delete an rdataset, it isn't necessary to set the 'create' flag. if the
node doesn't exist yet, we won't be able to delete anything from it anyway.
Merge branch 'each-minor-findnode-refactor' into 'main'
See merge request isc-projects/bind9!9158
When adding glue to the header, we add header to the wait-free stack to
be cleaned up later which sets wfc_node->next to non-NULL value. When
the actual cleaning happens we would only cleanup the .glue_list, but
since the database isn't locked for the time being, the headers could be
reused while cleaning the existing glue entries, which creates a data
race between database versions.
Revert the code back to use per-database-version hashtable where keys
are the node pointers. This allows each database version to have
independent glue cache table that doesn't affect nodes or headers that
could already "belong" to the future database version.
when searching the cache for a node so that we can delete an
rdataset, it is not necessary to set the 'create' flag. if the
node doesn't exist yet, we then we won't be able to delete
anything from it anyway.
dns_difftuple_create() could only return success, so change
its type to void and clean up all the calls to it.
Merge branch 'each-difftuple-create-cannot-fail' into 'main'
See merge request isc-projects/bind9!9151
dns_difftuple_create() could only return success, so change
its type to void and clean up all the calls to it.
other functions that only returned a result value because of it
have been cleaned up in the same way.
When a priming query is complete, it's currently logged at level ISC_LOG_DEBUG(1), regardless of success or failure. We are now raising it to ISC_LOG_NOTICE in the case of failure. [GL #3516]
Closes#3516
Merge branch '3516-log-priming-errors' into 'main'
Closes#3516
See merge request isc-projects/bind9!9121
when a priming query is complete, it's currently logged at
level ISC_LOG_DEBUG(1), regardless of success or failure. we
are now raising it to ISC_LOG_NOTICE in the case of failure.
There isn't a realistic reason to ever use e = 4294967297. Fortunately
its codepath wasn't reachable to users and can be safetly removed.
Keep in mind the `dns_key_generate` header comment was outdated. e = 3
hasn't been used since 2006 so there isn't a reason to panic. The
toggle was the public exponents between 65537 and 4294967297.
Merge branch 'aydin/rsa-cleanup' into 'main'
See merge request isc-projects/bind9!9133
There isn't a realistic reason to ever use e = 4294967297. Fortunately
its codepath wasn't reachable to users and can be safetly removed.
Keep in mind the `dns_key_generate` header comment was outdated. e = 3
hasn't been used since 2006 so there isn't a reason to panic. The
toggle was the public exponents between 65537 and 4294967297.
CRC-64 has been added for map files. Now that the map file format has
been removed, there isn't a reason to keep the implementation.
Merge branch 'aydin/cleanup-crc' into 'main'
See merge request isc-projects/bind9!9135
The previous work in this area was led by the belief that we might be
calling call_rcu() from within call_rcu() callbacks. After carefully
checking all the current callback, it became evident that this is not
the case and the problem isn't enough rcu_barrier() calls, but something
entirely else.
Call the rcu_barrier() just once as that's enough and the multiple
rcu_barrier() calls will not hide the real problem anymore, so we can
find it.
Merge branch '4607-call-a-single-rcu_barrier' into 'main'
See merge request isc-projects/bind9!9134
The previous work in this area was led by the belief that we might be
calling call_rcu() from within call_rcu() callbacks. After carefully
checking all the current callback, it became evident that this is not
the case and the problem isn't enough rcu_barrier() calls, but something
entirely else.
Call the rcu_barrier() just once as that's enough and the multiple
rcu_barrier() calls will not hide the real problem anymore, so we can
find it.
OpenSSL 1.1.1 or better (or equivalent LibreSSL version) is now required to compile BIND 9.
Closes#2806
Merge branch '2806-remove-ax_check_openssl' into 'main'
Closes#2806
See merge request isc-projects/bind9!9110
Since the minimal OpenSSL version is now OpenSSL 1.1.1, remove all kind
of OpenSSL shims and checks for functions that are now always present in
the OpenSSL libraries.
Co-authored-by: Ondřej Surý <ondrej@isc.org>
Co-authored-by: Aydın Mercan <aydin@isc.org>
OpenSSL supports pkg-config method since the 0.9.8 version and we
already require pkg-config for other mandatory libraries. Also
the way the AX_CHECK_OPENSSL macro was integrated into the configure
script was confusing - the macro would be used only if the libcrypto.pc
and libssl.pc file are not usable, so calling ./configure
--with-openssl=/usr/local would have no effect when PKG_CHECK_MODULES
would be successful.
As BIND 9.20 does not support RHEL/CentOS 7 which just reach
end-of-life, we can safely bump the OpenSSL requirements to version
1.1.1, which in turn will allow us to simplify our OpenSSL integration.
Checking the version of `named-checkconf` would end with assertion failure. This has been fixed.
Closes#4827
Merge branch '4827-cleanup-dst-only-if-initialized' into 'main'
Closes#4827
See merge request isc-projects/bind9!9243
The dst_lib_destroy() should be called only if dst_lib_init() was called
before. In named-checkconf, that is guarded by dst_cleanup variable
that was erroneously set to true by default. Set the dst_cleanup to
'false' by default.
An assertion failure triggers when the TSIG has valid cryptographic signature, but the time is invalid. This can happen when the times between the primary and secondary servers are not synchronised.
Closes#4811
Merge branch '4811-fix-isc_buffer_putuint48-buffer-size-requirement' into 'main'
Closes#4811
See merge request isc-projects/bind9!9234
Add a system test that sets TSIG fudge to 0, waits three seconds and
then sends signed message to the server. This tests the path where the
time difference between the client and the server is outside of the TSIG
fudge value.
The tsig unit test was only testing if everything went ok, but it was
not testing whether the error paths work. Add two more unit tests - one
uses the time outside of the TSIG skew, and the second trashes the
signature with random data.
When putting the 48-bit number into a fixed-size buffer that's exactly 6
bytes, the assertion failure would occur as the 48-bit number is
internally represented as 64-bit number and the code was checking if
there is enough space for `sizeof(val)`. This causes assertion failure
when otherwise valid TSIG signature has a bad timing information.
Specify the size of the argument explicitly, so the 48-bit number
doesn't require 8-byte long buffer.
The fcount_incr() was incorrectly skipping the accounting for the
fetches-per-zone if the force argument was set to true. We want to skip
the accounting only when the fetches-per-zone is completely disabled,
but for individual names we need to do the accounting even if we are
forcing the result to be success.
Closes#4786
Merge branch '4786-forced-fcount_incr-should-still-increment-count-and-allowed' into 'main'
Closes#4786
See merge request isc-projects/bind9!9115
The fcount_incr() was incorrectly skipping the accounting for the
fetches-per-zone if the force argument was set to true. We want to skip
the accounting only when the fetches-per-zone is completely disabled,
but for individual names we need to do the accounting even if we are
forcing the result to be success.
The system tests were overriding the local locale by setting LANG to C.
This does not override the locale in case there are individual LC_<*>
variables like LC_CTYPE explicitly set.
Use LC_ALL=C instead which is the proper way of overriding all currently
set locales.
Merge branch 'ondrej/use-LC_ALL-not-LANG' into 'main'
See merge request isc-projects/bind9!9109
The system tests were overriding the local locale by setting LANG to C.
This does not override the locale in case there are individual LC_<*>
variables like LC_CTYPE explicitly set.
Use LC_ALL=C instead which is the proper way of overriding all currently
set locales.
The tlsstream part of the isc_nmsocket_t gets initialized via designater
initializer and doesn't need the extra memset() later; just remove it.
Merge branch 'ondrej/remove-superfluous-memset-in-isc_nmsocket_init' into 'main'
See merge request isc-projects/bind9!9120
The PTHREAD_MUTEX_ADAPTIVE_NP and PTHREAD_MUTEX_ERRORCHECK_NP are
usually not defines, but enum values, so simple preprocessor check
doesn't work.
Check for PTHREAD_MUTEX_ADAPTIVE_NP from the autoconf AS_COMPILE_IFELSE
block and define HAVE_PTHREAD_MUTEX_ADAPTIVE_NP. This should enable
adaptive mutex on Linux and FreeBSD.
As PTHREAD_MUTEX_ERRORCHECK actually comes from POSIX and Linux glibc
does define it when compatibility macros are being set, we can just use
PTHREAD_MUTEX_ERRORCHECK instead of PTHREAD_MUTEX_ERRORCHECK_NP.
Merge branch 'ondrej/fix-adaptive-mutex-use' into 'main'
See merge request isc-projects/bind9!9111
The PTHREAD_MUTEX_ADAPTIVE_NP and PTHREAD_MUTEX_ERRORCHECK_NP are
usually not defines, but enum values, so simple preprocessor check
doesn't work.
Check for PTHREAD_MUTEX_ADAPTIVE_NP from the autoconf AS_COMPILE_IFELSE
block and define HAVE_PTHREAD_MUTEX_ADAPTIVE_NP. This should enable
adaptive mutex on Linux and FreeBSD.
As PTHREAD_MUTEX_ERRORCHECK actually comes from POSIX and Linux glibc
does define it when compatibility macros are being set, we can just use
PTHREAD_MUTEX_ERRORCHECK instead of PTHREAD_MUTEX_ERRORCHECK_NP.
When automatic-interface-scan is disabled, the route socket was still
being opened. Add new API to connect / disconnect from the route socket
only as needed.
Additionally, move the block that disables periodic interface rescans to
a place where it actually have access to the configuration values.
Previously, the values were being checked before the configuration was
loaded.
Closes#4757
Merge branch '4757-dont-open-routing-socket-if-not-needed' into 'main'
Closes#4757
See merge request isc-projects/bind9!9122
When automatic-interface-scan is disabled, the route socket was still
being opened. Add new API to connect / disconnect from the route socket
only as needed.
Additionally, move the block that disables periodic interface rescans to
a place where it actually have access to the configuration values.
Previously, the values were being checked before the configuration was
loaded.
I split this into two commits, one for the actual newline removal, and one for issues I found, ruining the yaml output when some errors were outputted.
Closes: #4772
Merge branch 'yaml-indent' into 'main'
Closes#4772
See merge request isc-projects/bind9!9112
Add INSIST to fail if the multiplication would cause the variables to overflow.
Closes#4798
Merge branch '4798-cid-498025-and-cid-498031-overflowed-constant-integer_overflow' into 'main'
Closes#4798
See merge request isc-projects/bind9!9131
If bufsize overflows we will have an infinite loop. In practice
this will not happen unless we have made a coding error. Add an
INSIST to detect this condition.
181retry:
182 isc_buffer_allocate(mctx, &b, bufsize);
183 result = dns_rdata_totext(rdata, NULL, b);
184 if (result == ISC_R_NOSPACE) {
185 isc_buffer_free(&b);
CID 498031: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression bufsize, which is equal to 0, overflows
the type that receives it, an unsigned integer 32 bits wide.
186 bufsize *= 2;
187 goto retry;
188 }
If size overflows we will have an infinite loop. In practice
this will not happen unless we have made a coding error. Add
an INSIST to detect this condition.
181 while (!done) {
182 isc_buffer_allocate(mctx, &b, size);
183 result = dns_rdata_totext(rdata, NULL, b);
184 if (result == ISC_R_SUCCESS) {
185 printf("%.*s\n", (int)isc_buffer_usedlength(b),
186 (char *)isc_buffer_base(b));
187 done = true;
188 } else if (result != ISC_R_NOSPACE) {
189 check_result(result, "dns_rdata_totext");
190 }
191 isc_buffer_free(&b);
CID 498025: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression size, which is equal to 0, overflows the type that
receives it, an unsigned integer 32 bits wide.
192 size *= 2;
193 }
Decrementing optlen immediately before calling continue is unneccesary
and inconsistent with the rest of dns_message_pseudosectiontoyaml
and dns_message_pseudosectiontotext. Coverity was also reporting
an impossible false positive overflow of optlen (CID 499061).
4176 } else if (optcode == DNS_OPT_CLIENT_TAG) {
4177 uint16_t id;
4178 ADD_STRING(target, "; CLIENT-TAG:");
4179 if (optlen == 2U) {
4180 id = isc_buffer_getuint16(&optbuf);
4181 snprintf(buf, sizeof(buf), " %u\n", id);
4182 ADD_STRING(target, buf);
CID 499061: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression optlen, which is equal to 65534, underflows
the type that receives it, an unsigned integer 16 bits wide.
4183 optlen -= 2;
4184 POST(optlen);
4185 continue;
4186 }
4187 } else if (optcode == DNS_OPT_SERVER_TAG) {
Merge branch 'marka-remove-unnecessary-operations' into 'main'
See merge request isc-projects/bind9!9130
Decrementing optlen immediately before calling continue is unneccesary
and inconsistent with the rest of dns_message_pseudosectiontoyaml
and dns_message_pseudosectiontotext. Coverity was also reporting
an impossible false positive overflow of optlen (CID 499061).
4176 } else if (optcode == DNS_OPT_CLIENT_TAG) {
4177 uint16_t id;
4178 ADD_STRING(target, "; CLIENT-TAG:");
4179 if (optlen == 2U) {
4180 id = isc_buffer_getuint16(&optbuf);
4181 snprintf(buf, sizeof(buf), " %u\n", id);
4182 ADD_STRING(target, buf);
CID 499061: (#1 of 1): Overflowed constant (INTEGER_OVERFLOW)
overflow_const: Expression optlen, which is equal to 65534, underflows
the type that receives it, an unsigned integer 16 bits wide.
4183 optlen -= 2;
4184 POST(optlen);
4185 continue;
4186 }
4187 } else if (optcode == DNS_OPT_SERVER_TAG) {
The minimum allowed value of 'resolver-query-timeout' was lowered to
301 milliseconds instead of the earlier 10000 milliseconds (which is the
default). As earlier, values less than or equal to 300 are converted to
seconds before applying the limit.
Closes#4320
Merge branch '4320-allow-shorter-resolver-query-timeout-configuration' into 'main'
Closes#4320
See merge request isc-projects/bind9!9091
Before there was a gap from 301 to 9999 which would be converted
to 10000 and now there is no such gap.
This settimeout_belowmin test was checking the behavior of a value
in the gap. As there is now no gap left, the minimum is 301 and
anything below that is converted to seconds as before. In order
for this check to still test the "below minimum" behavior, change
the value from 9000 to 300.
Update the settimeout_overmax value test too so it logically aligns
with the minimum value test.
There are use cases for which shorter timeout values make sense.
For example if there is a load balancer which sets RD=1 and
forwards queries to a BIND resolver which is then configured to
talk to backend servers which are not visible in the public NS set.
WIth a shorter timeout value the frontend can give back SERVFAIL
early when backends are not available and the ultimate client will
not penalize the BIND-frontend for non-response.
A new optional argument '-force' has been added to the command channel
command 'rndc retransfer'. When it is specified, named aborts the
ongoing zone transfer (if there is one), and starts a new transfer.
Closes#2299
Merge branch '2299-implement-rndc-force-retransfer' into 'main'
Closes#2299
See merge request isc-projects/bind9!9102
Use a big zone and the slow transfer mode. Initiate a retransfer, wait
several seconds, then initiate a retransfer using a '-force' argument,
which should cancel the previous transfer and start a new one.
The period between the most significant nibble of the encoded IPv4 address and the 2.0.0.2.IP6.ARPA suffix was missing resulting in the wrong name being checked. Add system test for 6to4-self implementation.
Closes#4766
Merge branch '4766-add-system-test-for-6to4-self' into 'main'
Closes#4766
See merge request isc-projects/bind9!9099
The period between the most significant nibble of the IPv4 address
and the 2.0.0.2.IP6.ARPA suffix was missing resulting in the wrong
name being checked.
Remove the false positive "success resolving" log message when QNAME minimisation is in effect and the final result is NXDOMAIN.
Closes#4784
Merge branch '4784-false-qname-minimisation-error-being-reported' into 'main'
Closes#4784
See merge request isc-projects/bind9!9117
In yaml mode we emit a string for each question and record. Certain
names and data could result in invalid yaml being produced. Use single
quote string for all questions and records. This requires that single
quotes get converted to two quotes within the string.
ALPN are defined as 1*255OCTET in RFC 9460. commatxt_fromtext was not
rejecting invalid inputs produces by missing a level of escaping
which where later caught be dns_rdata_fromwire on reception.
These inputs should have been rejected
svcb in svcb 1 1.svcb alpn=\,abc
svcb1 in svcb 1 1.svcb alpn=a\,\,abc
and generated 00 03 61 62 63 and 01 61 00 02 61 62 63 respectively.
The correct inputs to include commas in the alpn requires double
escaping.
svcb in svcb 1 1.svcb alpn=\\,abc
svcb1 in svcb 1 1.svcb alpn=a\\,\\,abc
and generate 04 2C 61 62 63 and 06 61 2C 2C 61 62 63 respectively.
Add a note that 'rndc reload' and 'rndc reconfig' can't change the
querylog option during the runtime of named.
Closes#4801
Merge branch '4801-arm-querylog-clarification' into 'main'
Closes#4801
See merge request isc-projects/bind9!9136
Replace #define DNS_GETDB_ with struct of bools to make
it easier to pretty-print the attributes in a debugger.
Closes#4559
Merge branch '4559-convert-dns_getdb_x-defines-to-1-bit-long-bools' into 'main'
Closes#4559
See merge request isc-projects/bind9!9093
When cutting a long CNAME chain, named was returning NOERROR
instead of SERVFAIL (alongside with a partial answer). This
has been fixed.
Closes#4449
Merge branch '4449-return-servfail-for-a-long-cname-chain' into 'main'
Closes#4449
See merge request isc-projects/bind9!9090
Update the CNAME chain test to correspond to the changed behavior,
because now named returns SERVFAIL when hitting the maximum query
restarts limit (e.g. happening when following a long CNAME chain).
In the current test auth will hit the limit and return partial data
with a SERVFAIL code, while the resolver will return no data with
a SERVFAIL code after auth returns SERVFAIL to it.
Due to the maximum query restart limitation a long CNAME chain
it is cut after 16 queries but named still returns NOERROR.
Return SERVFAIL instead and the partial answer.
Ensure that the selected algorithms remains stable throughout the entire test session. Crypto support detection was rewritten to python and simplified.
Closes#4202Closes#4422
Related #3810
Merge branch '4202-algorithm-detection-pytest' into 'main'
Closes#4202 and #4422
See merge request isc-projects/bind9!8803
Ensure all the variables are initialized when running the main function
of isctest module. This enables proper environment variables during test
script development when only conf.sh is sourced, rather than the script
being executed by the pytest runner.
Run the crypto support checks when initializing the isctest package and
save those results in environment variable. This removes the need to
repeatedly check for crypto operation support, as it's not something
that would change at test runtime.
Instead of invoking get_algorithms.py script repeatedly (which may yield
different results), move the algorithm configuration to an isctest
module. This ensures the variables are consistent across the entire test
run.
Add two mechanisms to close open log files. The first is `rndc closelogs`. The second is `kill -USR1 <pid>`.
They are intended to be used with external log rotation tools.
Closes#4780
Merge branch '4780-add-support-for-external-log-rotation-tools' into 'main'
Closes#4780
See merge request isc-projects/bind9!9113
Some external log file rotation programs use signals to tell programs
to close log files. SIGHUP can be used to do this but it also does
a full reconfiguration. Configure named to accept SIGUSR1 as a
signal to close log files.
On 32 bit machines isc_meminfo_totalphys could return an incorrect value.
Closes#4799
Merge branch '4799-cid-498034-overflowed-return-value-integer_overflow' into 'main'
Closes#4799
See merge request isc-projects/bind9!9132
On a 32 bit machine casting to size_t can still lead to an overflow.
Cast to uint64_t. Also detect all possible negative values for
pages and pagesize to silence warning about possible negative value.
39#if defined(_SC_PHYS_PAGES) && defined(_SC_PAGESIZE)
1. tainted_data_return: Called function sysconf(_SC_PHYS_PAGES),
and a possible return value may be less than zero.
2. assign: Assigning: pages = sysconf(_SC_PHYS_PAGES).
40 long pages = sysconf(_SC_PHYS_PAGES);
41 long pagesize = sysconf(_SC_PAGESIZE);
42
3. Condition pages == -1, taking false branch.
4. Condition pagesize == -1, taking false branch.
43 if (pages == -1 || pagesize == -1) {
44 return (0);
45 }
46
5. overflow: The expression (size_t)pages * pagesize might be negative,
but is used in a context that treats it as unsigned.
CID 498034: (#1 of 1): Overflowed return value (INTEGER_OVERFLOW)
6. return_overflow: (size_t)pages * pagesize, which might have underflowed,
is returned from the function.
47 return ((size_t)pages * pagesize);
48#endif /* if defined(_SC_PHYS_PAGES) && defined(_SC_PAGESIZE) */
Stop updating `find.result_v4` and `find.result_v4` in `clean_finds_at_name`. The values are supposed to be static.
Closes#4118
Merge branch '4118-data-race-lib-dns-adb-c-1537-in-clean_finds_at_name' into 'main'
Closes#4118
See merge request isc-projects/bind9!9108
Don't verify the just signed zone as the RRSIGs could have expired before the signing process completes
Closes#4781#2476
Merge branch '4781-statschannel-setup-can-fail-due-to-short-validity-interval' into 'main'
Closes#4781 and #2476
See merge request isc-projects/bind9!9114
The updatecheck-kskonly.secure zone is being used to test dynamic
updates while the KSK is offline. It ensures that the DNSKEY RRset
will retain the RRSIG record, while the updated data is being signed
with the currently active ZSK.
When walking through ZSK rollovers, ensure that the newest ZSK (ZSK3)
is published before doing the dynamic update, preventing timing
related test failures.
Also fix the test log line ($ZSK_ID3 was not yet created at the time
of logging).
Adjust key state and timing metadata if dnssec-policy key lifetime configuration is updated, so that it also
affects existing keys.
Closes#4677
Merge branch '4677-dnssec-policy-key-lifetime-reconfigure' into 'main'
Closes#4677
See merge request isc-projects/bind9!9118
The key lifetime should no longer be adjusted if the key is being
retired earlier, for example because a manual rollover was started.
This would falsely be seen as a dnssec-policy lifetime reconfiguration,
and would adjust the retire/removed time again.
This also means we should update the status output, and the next
rollover scheduled is now calculated using (retire-active) instead of
key lifetime.
If dnssec-policy is reconfigured and the key lifetime has changed,
update existing keys with the new lifetime and adjust the retire
and removed timing metadata accordingly.
If the key has no lifetime yet, just initialize the lifetime. It
may be that the retire/removed timing metadata has already been set.
Skip keys which goal is not set to omnipresent. These keys are already
in the progress of retiring, or still unused.
Use a single source of truth, the git log, to generate the list of CHANGES. Use the .rst format and include it in the ARM for a quick reference with proper gitlab links to issues and merge requests.
Closes#75
Merge branch 'nicki/add-gitchangelog' into 'main'
Closes#75
See merge request isc-projects/bind9!9152
The files in contrib/ directory shouldn't be subject to our pylint
check. They can come from external sources and we don't subject these to
the same standards as the rest of the BIND9 code / scripts.
The configuration has been crafted to cater for BIND9 needs:
- Define actions that have an equivalent section in existing Release
Notes
- Assume the commits that have the necessary changelog makrup are merge
commits from GitLab and transform them into messages and proper links
- Put the resulting changelog into the proper place in
doc/arm/changelog.rst
- Have a separate configuration for changelog and release notes. Both of
these should be generated from the `git log`, with release notes being
subject to more scrutiny and further editing
The project hasn't seen any new development/changes since 2018 and it
seems unlikely we'd be able to get any changes into the upstream. Since
it's isolated into a single file and its task is fairly straighforward,
pull the code into our own repository and maintain it here as needed.
This also makes it easier to make any changes that are specific to our
project.
This commit ensures that we are not attempting to accept an expired
TCP connection as we are not interested in any data that could have
been accumulated in its internal buffers. Now we just drop them for
good.
When sending fails, the ns__client_request() would not reset the
connection and continue as nothing is happening. This comes from the
model that we don't care about failed UDP sends because datagrams are
unreliable anyway, but it greatly affects TCP connections with
keep-alive.
The worst case scenario is as follows:
1. the 3-way TCP handshake gets completed
2. the libuv calls the "uv_connection_cb" callback
3. the TCP connection gets queue because of the tcp-clients quota
4. the TCP client sends as many DNS messages as the buffers allow
5. the TCP connection gets dropped by the client due to the timeout
6. the TCP connection gets accepted by the server
7. the data already sent by the client gets read
8. all sending fails immediately because the TCP connection is dead
9. we consume all the data in the buffer in a very tight loop
As it doesn't make sense to trying to process more data on the TCP
connection when the sending is failing, drop the connection immediately
on the first sending error.
As ns_query_init() cannot fail now, remove the error paths, especially
in ns__client_setup() where we now don't have to care what to do with
the connection if setting up the client could fail. It couldn't fail
even before, but now it's formal.
Be more aggressive when throttling the reading - when we can't send the
outgoing TCP synchronously with uv_try_write(), we start throttling the
reading immediately instead of waiting for the send buffers to fill up.
This should not affect behaved clients that read the data from the TCP
on the other end.
Instead of outright refusing to add new RR types to the cache, be a bit
smarter:
1. If the new header type is in our priority list, we always add either
positive or negative entry at the beginning of the list.
2. If the new header type is negative entry, and we are over the limit,
we mark it as ancient immediately, so it gets evicted from the cache
as soon as possible.
3. Otherwise add the new header after the priority headers (or at the
head of the list).
4. If we are over the limit, evict the last entry on the normal header
list.
Add HTTPS, SVCB, SRV, PTR, NAPTR, DNSKEY and TXT records to the list of
the priority types that are put at the beginning of the slabheader list
for faster access and to avoid eviction when there are more types than
the max-types-per-name limit.
Due to omission it was possible to un-throttle a TCP connection
previously throttled due to the peer not reading back data we are
sending.
In particular, that affected DoH code, but it could also affect other
transports (the current or future ones) that pause/resume reading
according to its internal state.
Clear qctx->zversion when clearing qctx->zrdataset et al in
lib/ns/query.c:qctx_freedata. The uncleared pointer could lead to
an assertion failure if zone data needed to be re-saved which could
happen with stale data support enabled.
A different solution in the future might be adopted depending
on feedback and other new information, so it makes sense to mark
these options as EXPERIMENTAL until we have more data.
View matching on an incoming query checks the query's signature,
which can be a CPU-heavy task for a SIG(0)-signed message. Implement
an asynchronous mode of the view matching function which uses the
offloaded signature checking facilities, and use it for the incoming
queries.
Add support for using the offload threadpool to perform message
signature verifications. This should allow check SIG(0)-signed
messages without affecting the worker threads.
This is a tiny helper function which is used only once and can be
replaced with two function calls instead. Removing this makes
supporting asynchronous signature checking less complicated.
In order to protect from a malicious DNS client that sends many
queries with a SIG(0)-signed message, add a quota of simultaneously
running SIG(0) checks.
This protection can only help when named is using more than one worker
threads. For example, if named is running with the '-n 4' option, and
'sig0checks-quota 2;' is used, then named will make sure to not use
more than 2 workers for the SIG(0) signature checks in parallel, thus
leaving the other workers to serve the remaining clients which do not
use SIG(0)-signed messages.
That limitation is going to change when SIG(0) signature checks are
offloaded to "slow" threads in a future commit.
The 'sig0checks-quota-exempt' ACL option can be used to exempt certain
clients from the quota requirements using their IP or network addresses.
The 'sig0checks-quota-maxwait-ms' option is used to define a maximum
amount of time for named to wait for a quota to appear. If during that
time no new quota becomes available, named will answer to the client
with DNS_R_REFUSED.
By default we log a rekey failure on debug level. We should probably
change the log level to error. We make an exception for when the zone
is not loaded yet, it often happens at startup that a rekey is
run before the zone is fully loaded.
when signatures were not added because of too many types already
existing at a node, the diff was not being cleaned up; this led to
a memory leak being reported at shutdown.
kasp-max-types-per-name (named2.conf.in):
An unsigned zone with RR type count on a name right below the
configured limit. Then sign the zone using KASP. Adding a RRSIG would
push it over the RR type limit per name. Signing should fail, but
the server should not crash, nor end up in infinite resign-attempt loop.
kasp-max-records-per-type-dnskey (named1.conf.in):
Test with low max-record-per-rrset limit and a DNSSEC policy requiring
more than the limit. Signing should fail.
kasp-max-types-per-name (named1.conf.in):
Each RRSIG(covered type) is counted as an individual RR type. Test the
corner case where a signed zone, which is just below the limit-1,
adds a new type - doing so would trigger signing for the new type and
thus increase the number of "types" by 2, pushing it over the limit
again.
Add two new masterformat tests that use signing. In the case of
'under-limit-kasp', the signing will keep the number of records in the
RRset under the limit. In the case of 'on-limit-kasp', the signing
will push the number of records in the RRset over the limit, because
of the added RRSIG record.
Send a recursive query for a large number of RRsets, which should
fail when using the default max-types-per-name setting of 100, but
succeed when the cap is disabled.
Previously, the number of RR types for a single owner name was limited
only by the maximum number of the types (64k). As the data structure
that holds the RR types for the database node is just a linked list, and
there are places where we just walk through the whole list (again and
again), adding a large number of RR types for a single owner named with
would slow down processing of such name (database node).
Add a configurable limit to cap the number of the RR types for a single
owner. This is enforced at the database (rbtdb, qpzone, qpcache) level
and configured with new max-types-per-name configuration option that
can be configured globally, per-view and per-zone.
Send a recursive query for a large (2500 record) RRset, which should
fail when using the default max-records-per-type setting of 100, but
succeed when the cap is disabled.
Add two new masterformat tests - the 'huge' zone fits within the ns1
limit and loads on the primary ns1 server, but must not transfer to the
ns2 secondary, and the 'uber' zone should not even load on the primary
ns1 server.
Previously, the number of RRs in the RRSets were internally unlimited.
As the data structure that holds the RRs is just a linked list, and
there are places where we just walk through all of the RRs, adding an
RRSet with huge number of RRs inside would slow down processing of said
RRSets.
Add a configurable limit to cap the number of the RRs in a single RRSet.
This is enforced at the database (rbtdb, qpzone, qpcache) level and
configured with new max-records-per-type configuration option that can
be configured globally, per-view and per-zone.
The changes in this MR prevent the memory used for sending the outgoing
TCP requests to spike so much. That strictly remove the extra need for
own memory context, and thus since we generally prefer simplicity,
remove the extra memory context with own jemalloc arenas just for the
outgoing send buffers.
The single TCP read can create as much as 64k divided by the minimum
size of the DNS message. This can clog the processing thread and trash
the memory allocator because we need to do as much as ~20k allocations in
a single UV loop tick.
Limit the number of the DNS messages processed in a single UV loop tick
to just single DNS message and limit the number of the outstanding DNS
messages back to 23. This effectively limits the number of pipelined
DNS messages to that number (this is the limit we already had before).
As a single thread can process only one TCP send at the time, we don't
really need a memory pool for the TCP buffers, but it's enough to have
a single per-loop (client manager) static buffer that's being used to
assemble the DNS message and then it gets copied into own sending
buffer.
In the future, this should get optimized by exposing the uv_try API
from the network manager, and first try to send the message directly
and allocate the sending buffer only if we need to send the data
asynchronously.
Constantly allocating, reallocating and deallocating 64K TCP send
buffers by 'ns_client' instances takes too much CPU time.
There is an existing mechanism to reuse the ns_clent_t structure
associated with the handle using 'isc_nmhandle_getdata/_setdata'
(see ns_client_request()), but it doesn't work with TCP, because
every time ns_client_request() is called it gets a new handle even
for the same TCP connection, see the comments in
streamdns_on_complete_dnsmessage().
To solve the problem, we introduce an array of available (unused)
TCP buffers stored in ns_clientmgr_t structure so that a 'client'
working via TCP can have a chance to reuse one (if there is one)
instead of allocating a new one every time.
When TCP client would not read the DNS message sent to them, the TCP
sends inside named would accumulate and cause degradation of the
service. Throttle the reading from the TCP socket when we accumulate
enough DNS data to be sent. Currently this is limited in a way that a
single largest possible DNS message can fit into the buffer.
DoH: Avoid potential data races in our DoH implementation related to to HTTP/2 session object management and endpoints set object management
See merge request isc-private/bind9!614
This commit ensures that an HTTP endpoints set reference is stored in
a socket object associated with an HTTP/2 stream instead of
referencing the global set stored inside a listener.
This helps to prevent an issue like follows:
1. BIND is configured to serve DoH clients;
2. A client is connected and one or more HTTP/2 stream is
created. Internal pointers are now pointing to the data on the
associated HTTP endpoints set;
3. BIND is reconfigured - the new endpoints set object is created and
promoted to all listeners;
4. The old pointers to the HTTP endpoints set data are now invalid.
Instead referencing a global object that is updated on
re-configurations we now store a local reference which prevents the
endpoints set objects to go out of scope prematurely.
It was reported that HTTP/2 session might get closed or even deleted
before all async. processing has been completed.
This commit addresses that: now we are avoiding using the object when
we do not need it or specifically check if the pointers used are not
'NULL' and by ensuring that there is at least one reference to the
session object while we are doing incoming data processing.
This commit makes the code more resilient to such issues in the
future.
Changed the default value for 'allow-transfer' to 'none'; zone
transfers now require explicit authorization.
Updated all system tests to specify an allow-transfer ACL when needed.
Revised the ARM to specify that the default is 'none'.
Replace the ISC_LIST based deadnodes implementation with isc_queue which
is wait-free and we don't have to acquire neither the tree nor node lock
to append nodes to the queue and the cleaning process can also
copy (splice) the list into a local copy without acquiring the list.
Currently, there's little benefit to this as we need to hold those
locks anyway, but in the future as we move to RCU based implementation,
this will be ready.
To align the cleaning with our event loop based model, remove the
hardcoded count for the node locks and use the number of the event loops
instead. This way, each event loop can have its own cleaning as part of
the process. Use uniform random numbers to spread the nodes evenly
between the buckets (instead of hashing the domain name).
Add an isc_queue implementation that hides the gory details of cds_wfcq
into more neat API. The same caveats as with cds_wfcq.
TODO: Add documentation to the API.
Setting "Affects v9.x" labels on a merge request duplicates information
already present on the GitLab issue associated with that merge request.
For trivial merge requests that are not associated with any GitLab
issue, setting the "Affects v9.x" label(s) is considered unnecessary.
Trigger a failure for every merge request marked with at least one
"Affects v9.x" label.
Merge request titles auto-generated by GitLab are often a source of
confusion regarding the actual contents of a given merge request. Warn
for merge requests containing titles that look like auto-generated ones.
Using "-v9_x" and "-v9.x" version suffixes for branch names is now
deprecated since some automation logic does not handle these. Fail for
any merge request using such old-style version suffixes.
Backports are not expected to have any "Affects v9.x" labels set since
those are only meant to be set for merge requests that should have
backports created for them.
When in -4 mode check that "IPv6 disabled and no IPv4 primaries"
is logged and when in -6 mode check that "IPv4 disabled and no IPv6
primaries" is logged.
When the cache's memory context was in over memory state when the
cache was flushed it resulted in LRU cleaning removing newly entered
data in the new cache straight away until the old cache had been
destroyed enough to take it out of over memory state. When flushing
the cache create a new memory context for the new db to prevent this.
The `axfr_makedb()` didn't set the loop on the newly created database,
effectively killing delayed cleaning on such database. Move the
database creation into dns_zone API that knows all the gory details of
creating new database suitable for the zone.
The times it takes to run tests CI vary significantly enough
that it makes hypothesis test reach their deadlines and fail randomly
marking the tests as flaky.
This commit disables the deadlines when running in CI.
The most important being `dns_names` that generates dns.name.Name
objects based on given paramaters.
No guarantees are given when it comes the uniformity of generated
samples, however it plays nicely with the hypothesis' shrinking
algorithm.
Once we use hypothesis more widely (in at least one more test) this file
should be moved for it to be reused easily.
Check for more rcodes and various properties needed in the wildcard
test. Add a `name` module for various dns.name.Name operations (with
`prepend_label` function only now).
Expose `timeout` as a parameter of `query.tcp`/`query.udp`.
The system test from the BIND 9.19.24 release does not include the
isctest/vars/autoconf.py file from 9.19.25-dev, and therefore the job
will fail before the 9.19.25 release is published. In the meantime,
consider using the conf.sh file.
The rdataslab.c was full of code like this:
length = raw[0] * 256 + raw[1];
and
count2 = *current2++ * 256;
count2 += *current2++;
Refactor code like this into peek_uint16() and get_uint16 macros
to prevent code repetition and possible mistakes when copy and
pasting the same code over and over.
As a side note for an entertainment of a careful reader of the commit
messages: The byte manipulation was changed from multiplication and
addition to shift with or.
The difference in the assembly looks like this:
MUL and ADD:
movzx eax, BYTE PTR [rdi]
movzx edi, BYTE PTR [rdi+1]
sal eax, 8
or edi, eax
SHIFT and OR:
movzx edi, WORD PTR [rdi]
rol di, 8
movzx edi, di
If the result and/or buffer is then being used after the macro call,
there's more differences in favor of the SHIFT+OR solution.
The detach function declaration in `ISC__REFCOUNT_TRACE_DECL` had an
returned an accidental implicit int. While not allowed since C99, it
became an error by default in GCC 14.
`ISC_REFCOUNT_TRACE_IMPL` and `ISC_REFCOUNT_STATIC_TRACE_IMPL` expanded
into the wrong macros, trying to declare it again with the wrong number
of parameters.
- duplicated question
- duplicated answer
- qtype as an answer
- two question types
- question names
- nsec3 bad owner name
- short record
- short question
- mismatching question class
- bad record owner name
- mismatched class in record
- mismatched KEY class
- OPT wrong owner name
- invalid RRSIG "covers" type
- UPDATE malformed delete type
- TSIG wrong class
- TSIG not the last record
there were TSAN error reports because of conflicting uses of
node->dirty and node->nsec, which were in the same qword.
this could be resolved by separating them, but we could also
make them into atomic values and remove some node locking.
The fix_iterator() function had a lot of bugs in it and while fixing
them, the number of corner cases and the complexity of the function
got out of hand. Rewrite the function with the following modifications:
The function now requires that the iterator is pointing to a leaf node.
This removes the cases we have to deal when the iterator was left on a
dead branch.
From the leaf node, pop up the iterator stack until we encounter the
branch where the offset point is before the point where the search key
differs. This will bring us to the right branch, or at the first
unmatched node, in which case we pop up to the parent branch. From
there it is easier to retrieve the predecessor.
Once we are at the right branch, all we have to do is find the right
twig (which is either the twig for the character at the position where
the search key differs, or the previous twig) and walk down from there
to the greatest leaf or, in case there is no good twig, get the
previous twig from the successor and get the greatest leaf from there.
If there is no previous twig to select in this branch, because every
leaf from this branch node is greater than the one we wanted, we need
to pop up the stack again and resume at the parent branch. This is
achieved by calling prevleaf().
Move the fix_iterator out of the loop and only call it when we found
a leaf node. This leaf node may be the wrong leaf node, but fix_iterator
should correct that.
Also, when we don't need to set the iterator, just get any leaf. We
only need to have a leaf for the qpkey_compare and the end result does
not matter if compare was against an ancestor leaf or any leaf below
that point.
An obscured DNSKEY RRset at a delegation was incorrectly added to
the NSEC/NSEC3 type bit map leading to zone verification failures.
This adds such a RRset to the test zone.
DNSSEC only works when DNSKEYs are self signed. This only occurs
when the DNSKEY RRset is at the apex. Cause dnssec-signzone to
fail if it attempts to sign an non-apex DNSKEY RRset.
An assertion failure would be triggered when sending the TCP data ends
after the TCP reading gets closed. Implement proper reference counting
for the isc_httpd object.
When searching for a requested name in dns_qp_lookup(), we may add
a leaf node to the QP chain, then subsequently determine that the
branch we were on was a dead end. When that happens, the chain can be
left holding a pointer to a node that is *not* an ancestor of the
requested name.
We correct for this by unwinding any chain links with an offset
value greater or equal to that of the node we found.
The code below the if/else construction could only be run if the 'if'
code path was taken. Move the code into the 'if' code block so that
it is more easier to read.
The list isn't exactly maintained but it helped with some BIND history
tracking and is basically harmless so it might be worth holding onto it.
I have adapted the name to ASCII so IDN support won't be necessary.
The high-water allows administrators to better tune the recursive
clients limit without having to to poll the statistics channel in high
rates to get this number.
Returning the value allows for better high-water tracking without
running into edge cases like the following:
0. The counter is at value X
1. Increment the value (X+1)
2. The value is decreased multiple times in another threads (X+1-Y)
3. Get the value (X+1-Y)
4. Update-if-greater misses the X+1 value which should have been the
high-water
To avoid any escaping issues or messing with a language-specific format
when the variable has to be parsed, create a dedicated file for each
variable that is obtained from autoconf.
Make sure all initialization takes place in isctest.vars.__init__ and
export the initial env vars there. Remove the no longer needed env
fixture and use os.environ instead.
If OPENSSL_CONF is exported as an empty string, it will cause issues on
rhel9fips. Allow the environment variables to be set and exported, but
make sure to only export them if they have been set by the user.
The openssl config needs to be parsed for some tests that use SoftHSM2.
Rewrite the parsing to python and ensure the required variables are
properly set test-wide.
Provide a single point of access to all the variables used by tests. Use
a custom dict-like structure to access the underlying data without
making a copy. This allows the individual modules to update the contents
at runtime, which is used for some variables.
While this isn't required for pytest operation and execution of the
system test suite, it can be handy to allow test script development and
debugging. Especially setup scripts often source conf.sh and expect
environment variables to be loaded. If these scripts are executed
stand-alone, the environment variables need to be loaded from the python
package.
Remove conf.sh.in and move the environment variables into isctest/vars
python package. This enabled the removal of an ugly pytest hack which
loaded and parsed these variables from the environment.
qp and rbtdb produce stylistically different backup files. This
was causing the xferquota system test to fail. This has been
addressed by making the test independent of the stylistic differences.
The mechanism was published as RFC 8509. I've briefly looked at diff
between versions -08 and the RFC and did not find significant protocol
change. Quick manual check confirms what we seem to comply with the
published protocol.
Draft was eventually published as RFC 9276 but we did not update our
docs. Also add couple mentions in relevant places in the ARM and
dnssec-signzone man page, mainly around "do not touch" places.
Resolve "SVCB/HTTPS: Bind9 and Dig fail to parse packet if zero SvcPriority and existing SvcParams"
Closes#4704
See merge request isc-projects/bind9!9000
When parsing a zonefile named-checkzone (and others) could loop
infinitely if a directory was $INCLUDED. Record the error and treat
as EOF when looking for multiple errors.
This was found by Eric Sesterhenn from X41.
In nibble mode if the value to be converted was negative the parser
would loop forever. Process the value as an unsigned int instead
of as an int to prevent sign extension when shifting.
This was found by Eric Sesterhenn from X41.
This test is outdated because it tested the 'sig-validity-interval'
option that has been replaced by dnssec-policy's 'signatures-validity',
'signatures-refresh', and 'signatures-jitter' options.
Nevertheless, it tests if the jitter is spread correctly.
Update the test to make use of 'signatures-jitter', set the value
to 1 day (meaning resign in 499 days since 'signatures-validity' is
set to 500 days).
Note that this previously changed erroneously the refresh value to
449 days (should have been 499 days, but that is not allowed by
checkconf, since it is above 90% of 'signatures-validity').
After we have changed the maximum allowed iterations to 51 for signing,
the NSEC3 chain has changed and requires one more NSEC to be returned
in the answer (plus corresponding RRSIG). So the expected number or
records in the authority section is now 8.
If the key is offline and the keymgr runs, it will treat it as a missing key,
and generate a new key (according to the policy). Fix the test by putting
back the KSK temporarily when we run 'rndc loadkeys'.
1. When generating keys, don't set timing metadata. Otherwise keys
are considered to be in use and won't be selected when dnssec-policy
starts a new key rollover.
2. Add an extra check to make sure the new ZSK (zsk2) is prepublished.
Also add a check to make sure it has become active.
3. When using dnssec-settime, add -s to also write to key state files.
The config was recently modified to ensure ns4 won't leak any queries to
root servers. However, the test wasn't executed and it turns out the way
this was handled actually broke the test case. Add our custom root hint
to both of the views to ensure the test can still pass without leaking
any queries.
An RPZ response's SOA record TTL is set to 1 instead of the SOA TTL,
a boolean value is passed on to query_addsoa, which is supposed to be
a TTL value. I don't see what value is appropriate to be used for
overriding, so we will pass UINT32_MAX.
Now that this function also creates the DNSKEY record for the KSKs,
as well as other associated records such as CDS and CDNSKEY, rename
the function to something slightly better.
Creating the KSR happens on the "ZSK side". The KSK is offline and while
the public key and state file may be present, draft-icann-dnssec-keymgmt-01.txt
suggest that the KSR only contains ZSKs.
This is also what knot dns does, so it would also be in the spirit of
interoperability.
The final line in a KSR ";; KeySigningRequest generated at ..." was
missing the version number, that has now been fixed.
Thanks Libor Peltan for reporting.
The expireheader() call in the expire_ttl_headers() function
is erroneous as it passes the 'nlocktypep' and 'tlocktypep'
arguments in a wrong order, which then causes an assertion
failure.
Fix the order of the arguments so it corresponds to the function's
prototype.
in QP keys, characters that are not common in DNS names are
encoded as two-octet sequences. this caused a glitch in iterator
positioning when some lookups failed.
consider the case where we're searching for "\009" (represented
in a QP key as {0x03, 0x0c}) and a branch exists for "\000"
(represented as {0x03, 0x03}). we match on the 0x03, and continue
to search down. at the point where we find we have no match,
we need to pop back up to the branch before the 0x03 - which may
be multiple levels up the stack - before we position the iterator.
there were some structure names used in qpcache.c and qpzone.c that
were too similar to each other and could be confusing when debugging.
they have been changed as follows:
in qcache.c:
- changed_t was unused, and has been removed
- search_t -> qpc_search_t
- qpdb_rdatasetiter_t -> qpc_rditer_t
- qpdb_dbiterator_t -> qpc_dbiter_t
in qpzone.c:
- qpdb_changed_t -> qpz_changed_t
- qpdb_changedlist_t -> qpz_changedlist_t
- qpdb_version_t -> qpz_version_t
- qpdb_versionlist_t -> qpz_versionlist_t
- qpdb_search_t -> qpz_search_t
- qpdb_load_t -> qpz_search_t
when calling dns_qp_lookup() from qpcache, instead of passing
'foundname' so that a name would be constructed from the QP key,
we now just use the name field in the node data. this makes
dns_qp_lookup() run faster.
the same optimization has also been added to qpzone.
the documentation for dns_qp_lookup() has been updated to
discuss this performance consideration.
when the cache is over memory, we purge from the LRU list until
we've freed the approximate amount of memory to be added. this
approximation could fail because the memory allocated for nodenames
wasn't being counted.
add a dns_name_size() function so we can look up the size of nodenames,
then add that to the purgesize calculation.
in a cache database, unlike zones, NSEC3 records are stored in
the main tree. it is not necessary to maintain a separate 'nsec3'
tree, nor to have code in the dbiterator implementation to traverse
from one tree to another.
(if we ever implement synth-from-dnssec using NSEC3 records, we'll
need to revert this change. in the meantime, simpler code is better.)
the QP database doesn't support relative names as the RBTDB did, so
there's no need for a 'new_origin' flag or to handle `DNS_R_NEWORIGIN`
result codes.
- remove unneeded struct members and misleading comments.
- remove unused parameters for static functions.
- rename 'find_callback' to 'delegating' for consistency with qpzone;
the find callback mechanism is not used in QP databases.
- change dns_qpdata_t to qpcnode_t (QP cache node), and dns_qpdb_t to
qpcache_t, as these types are only accessed locally.
- also change qpdata_t in qpzone.c to qpznode_t (QP zone node), for
consistency.
- make the refcount declarations for qpcnode_t and qpznode_t static,
using the new ISC_REFCOUNT_STATIC macros.
In qpcache (and rbtdb), there are some functions that acquire
neither the tree lock nor the node lock when calling newref().
In theory, this could lead to a race in which a new reference
is added to a node that was about to be deleted.
We now detect this condition by passing the current tree and node
lock status to newref(). If the node was previously unreferenced
and we don't hold at least one read lock, we will assert.
The FreeBSD autoscaler has been configured to utilize the new "instance"
GitLab Runner executor to spawn "stress" test CI jobs on AWS EC2
dynamically. A shared GitLab Runner named "freebsd-instance-autoscaler"
has been set up in GitLab CI/CD to communicate with EC2, provisioning VM
instances on demand based on a FreeBSD 13 AMI image. This image is the
same as the one previously used for FreeBSD "stress" tests before the
implementation of autoscaling (specifically, the
"freebsd13-amd64-bind9stress.aws.lab.isc.org" GitLab Runner in CI/CD).
Since the BIND 9 QA repository has been made public, adjust the relevant
URLs and paths used in .gitlab-ci.yml so that they work with the public
version of that repository.
Because some tests don't have a legtimate handle, provide a temporary
return early that should be fixed and removed before squashing. This
short circuiting is still correct until DoQ/DoH3 support is introduced.
GCC might fail to compile because it expects a return after UNREACHABLE.
It should ideally just work anyway since UNREACHABLE is either a
noreturn or UB (__builtin_unreachable / C23 unreachable).
Either way, it should be optimized almost always so the fallback is
free or basically free anyway when it isn't optimized out.
the previous commit introduced a possible race in getsigningtime()
where the rdataset header could change between being found on the
heap and being bound.
getsigningtime() now looks at the first element of the heap, gathers the
locknum, locks the respective lock, and retrieves the header from the
heap again. If the locknum has changed, it will rinse and repeat.
Theoretically, this could spin forever, but practically, it almost never
will as the heap changes on the zone are very rare.
we simplify matters further by changing the dns_db_getsigningtime()
API call. instead of passing back a bound rdataset, we pass back the
information the caller actually needed: the resigning time, owner name
and type of the rdataset that was first on the heap.
in RBTDB, the heap was used by zone databases for resigning, and
by the cache for TTL-based cache cleaning. the cache use case required
very frequent updates, so there was a separate heap for each of the
node lock buckets.
qpzone is for zones only, so it doesn't need to support the cache
use case; the heap will only be touched when the zone is updated or
incrementally signed. we can simplify the code by using only a single
heap.
fix_iterator() and related functions are quite difficult to read.
perhaps it would be a little clearer if we didn't assign values
to variables that won't subsequently be used, or unnecessarily
pop the stack and then push the same value back onto it.
also, in dns_qp_lookup() we previously called fix_iterator(),
removed the leaf from the top of the iterator stack, and then
added it back on. this would be clearer if we just push the leaf
onto the stack when we need to, but leave the stack alone when
it's already complete.
under some circumstances it was possible for the iterator to
be set to the first leaf in a set of twigs, when it should have
been set to the last.
a unit test has been added to test this scenario. if there is a
a tree containing the following values: {".", "abb.", "abc."}, and
we query for "acb.", previously the iterator would be positioned at
"abb." instead of "abc.".
the tree structure is:
branch (offset 1, ".")
branch (offset 3, ".ab")
leaf (".abb")
leaf (".abc")
we find the branch with offset 3 (indicating that its twigs differ
from each other in the third position of the label, "abB" vs "abC").
but the search key differs from the found keys at position 2
("aC" vs "aB"). we look up the bit value in position 3 of the
search key ("B"), and incorrectly follow it onto the wrong twig
("abB").
to correct for this, we need to check for the case where the search
key is greater than the found key in a position earlier than the
branch offset. if it is, then we need to pop from the current leaf
to its parent, and get the greatest leaf from there.
a further change is needed to ensure that we don't do this twice;
when we've moved to a new leaf and the point of difference between
it and the search key even earlier than before, then we're definitely
at a predecessor node and there's no need to continue the loop.
The previous value of 30 minutes used to cache the ADB names and entries
was quite long. Change the value to 60 seconds for faster recovery
after cached intermittent failure of the remote nameservers.
The algorithm from the previous commit[1] is now used to calculate all
the expiration values through the code (ncache results, cname/dname
targets).
1. ISC_MIN(cur, ISC_MAX(now + ADB_ENTRY_WINDOW, now + rdataset->ttl))
Correct the logic to set the expiration period of expire_{v4,v6} as
follows:
1. If the trust is ultimate (local entry), immediately set the entry as
expired, so the changes to the local zones have immediate effect.
3. If the expiration is already set and smaller than the new value, then
leave the expiration value as it is.
2. Otherwise pick larger of `now + ADB_ENTRY_WINDOW` and `now + TTL` as
the new expiration value.
When ADB entry was created it was set to never expire. If we never
called any of the functions that adjust the expiration, it could linger
in the ADB forever.
Set the expiration (.expires) to now + ADB_ENTRY_WINDOW when creating
the new ADB entry to ensure the ADB entry will always expire.
In the past, our CI infrastructure was more sensitive to the number of
CI jobs running on it. We tried to limit long-running jobs in merge
request-triggered pipelines, as there are many of them, and spawned them
only in daily scheduled ones. Moving most of the CI infrastructure to
AWS has made it way better to run jobs in parallel, and the existence of
short respdiff jobs has lost its original merit. It can also be harmful
as some problems are detected only by the longer respdiff variant when a
faulty merge request has already been merged. We should run all long
respdiff tests in merge request-triggered pipelines.
Also, move the former respdiff-long job (now just "respdiff") to AWS as
old instance memory constraints (see
f09cf69594) are no longer an issue.
We now have ctx.kskflag, ctx.zskflag, and ctx.revflag, but zskflag is
not quite like the other two, as it doesn't have a special bit in the
DNS packet, and is used as a boolean.
This patch changes so that we use booleans for all three, and
construct the flags based on which ones are set.
patch by @aram
Add test cases for the 'request' command. Reuse the earlier
pregenerated ZSKs. We also need to set up some KSK files, that can
be done with 'dnssec-keygen -k <policy> -fK' now.
The 'check_keys()' function is adjusted such that the expected active
time of the successor key is set to the inactive time of the
predecessor. Some additional information is saved to make 'request'
testing easier.
Add code that can create a Key Signing Request (KSR) given a DNSSEC
policy, a set of keys and an interval.
Multiple keys that match the bundle and kasp parameters are sorted by
keytag, mainly for testing purposes.
Create some helper functions for code that is going to be reused by the
other commands (request, sign), such as setting and checking the context
parameters, and retrieving the dnssec-policy/kasp.
The 'dnssec-keygen' tool now allows the options '-k <dnssec-policy>'
and '-f <flags>' together to create keys from a DNSSEC policy that only
match the given role. Allow setting '-fZ' to only create ZSKs, while
'-fK' will only create KSKs.
Add a system test for testing dnssec-ksr, initally for the keygen
command. This should be able to create or select key files given a
DNSSEC policy and a time window.
Introduce a new DNSSEC tool, dnssec-ksr, for creating signed key
response (SKR) files, given one or more key signing requests (KSRs).
For now it is just a dummy tool, but the future purpose of this utility
is to pregenerate ZSKs and signed RRsets for DNSKEY, CDNSKEY, and CDS
for a given period that a KSK is to be offline.
Previously, only a single controlconf message would be processed from a
single TCP read even if the TCP read buffer contained multiple messages.
Refactor the isccc_ccmsg unit to store the extra buffer in the internal
buffer and use the already read data first before reading from the
network again.
Co-authored-by: Ondřej Surý <ondrej@isc.org>
Co-authored-by: Dominik Thalhammer <dominik@thalhammer.it>
The xmlGetGlobalState allocates per-thread memory that is not properly
cleaned up when the libxml2 is used from offloaded threads. Add the
function the the LeakSanitizer suppression list.
Prepare the statistics channel data in the offloaded worker thread, so
the networking thread is not blocked by the process gathering data from
various data structures. Only the netmgr send is then run on the
networkin thread when all the data is already there.
Protect the access to the trust byte in the ncache data with relaxed
atomic operation to mimick the current behaviour. This will teach
TSAN that the concurrent access is fine.
This prevents TSAN errors with the ncache code where the trust byte
access needs to be protected by a lock. The old code copied the
entire region before determining where the name ended. We now
determine where the name ends then copy just that data and in doing
so avoid reading the trust byte.
This commit makes the dispatch_test use the same timeouts that network
manager tests. We do that because the old values appear to be too
small for our heavy loaded CI machines, leading to spurious failures
on them. The network manager tests are much more stable in this
situation and they use somewhat larger timeout values.
We use a smaller connection timeouts for the tests which are expected
to timeout to not wait for too long.
The source address field of 'newnotify' was not updated from the
default (0.0.0.0) when the destination address was an IPv6 address.
This resulted in the messages failing to be sent. Set the source
address to :: when the destination address is an IPv6 address.
- [ ] [:link:][step_backports] **(SwEng)** Prepare backports of the merge request addressing the problem for all affected (and still maintained) branches of a given product
- [ ] [:link:][step_finish_advisory] **(Support)** Finish preparing the Security Advisory
- [ ] [:link:][step_meta_issue] **(QA)** Create (or update) the private issue containing links to fixes & reproducers for all CVEs fixed in a given release cycle
- [ ] [:link:][step_changes] **(QA)** (BIND 9 only) Reserve a block of `CHANGES` placeholders once the complete set of vulnerabilities fixed in a given release cycle is determined
- [ ] [:link:][step_merge_fixes] **(QA)** Merge the CVE fixes in CVE identifier order
- [ ] [:link:][step_patches] **(QA)** Prepare a standalone patch for the last stable release of each affected (and still maintained) product branch
- [ ] [:link:][step_asn_releases] **(QA)** Prepare ASN releases (as outlined in the Release Checklist)
| `-DOPENSSL_API_COMPAT=10100` | Build using the deprecated OpenSSL APIs so that the `engine` API is available when building with OpenSSL 3.0.0 for PKCS#11 support |
| [security] | Fix for a significant security flaw |
| [experimental] | Used for new features when the syntax or other aspects of the design are still in flux and may change |
| [port] | Portability enhancement |
| [maint] | Updates to built-in data such as root server addresses and keys |
| [tuning] | Changes to built-in configuration defaults and constants to improve performance |
| [performance] | Other changes to improve server performance |
| [protocol] | Updates to the DNS protocol such as new RR types |
| [test] | Changes to the automatic tests, not affecting server functionality |
| [cleanup] | Minor corrections and refactoring |
| [doc] | Documentation |
| [contrib] | Changes to the contributed tools and libraries in the 'contrib' subdirectory |
| [placeholder] | Used in the main development branch to reserve change numbers for use in other branches, e.g., when fixing a bug that only exists in older releases |
In general, [func] and [experimental] tags only appear in new-feature
releases (i.e., those with version numbers ending in zero). Some new
functionality may be backported to older releases on a case-by-case basis.
All other change types may be applied to all currently supported releases.
#### Bug report identifiers
Most notes in the CHANGES file include a reference to a bug report or
``$INCLUDE``. This option cannot be combined with :option:`-O raw <-O>`
or serial-number updating.
..option:: -E engine
This option specifies the hardware to use for cryptographic
operations, such as a secure key store used for signing, when applicable.
When BIND 9 is built with OpenSSL, this needs to be set to the OpenSSL
engine identifier that drives the cryptographic accelerator or
hardware service module (usually ``pkcs11``).
..option:: -F
This options turns on FIPS (US Federal Information Processing Standards)
@@ -89,6 +80,11 @@ Options
where ``digest-type`` is an allowed algorithm such as SHA-256 (2), or SHA-384 (4).
Only works in combination with smart signing (``-S``).
..option:: -J filename
This option tells :program:`dnssec-signzone` to read the journal from the given file
when loading the zone file.
..option:: -K directory
This option specifies the directory to search for DNSSEC keys. If not
@@ -273,7 +269,7 @@ Options
with cached copies of the old DNSKEY RRset. The :option:`-Q` option forces
:program:`dnssec-signzone` to remove signatures from keys that are no longer
active. This enables ZSK rollover using the procedure described in
:rfc:`4641#4.2.1.1` ("Pre-Publish Key Rollover").
:rfc:`6781#4.1.1.1` ("Pre-Publish Key Rollover").
..option:: -q
@@ -290,7 +286,7 @@ Options
This option is similar to :option:`-Q`, except it forces
:program:`dnssec-signzone` to remove signatures from keys that are no longer
published. This enables ZSK rollover using the procedure described in
:rfc:`4641#4.2.1.2` ("Double Signature Zone Signing Key
:rfc:`6781#4.1.1.2` ("Double Signature Zone Signing Key
Rollover").
..option:: -S
@@ -374,6 +370,7 @@ Options
..note::
``-3 -`` is the recommended configuration. Adding salt provides no practical benefits.
See :rfc:`9276`.
..option:: -H iterations
@@ -382,6 +379,7 @@ Options
..warning::
Values greater than 0 cause interoperability issues and also increase the risk of CPU-exhausting DoS attacks.
See :rfc:`9276`.
..option:: -A
@@ -390,6 +388,7 @@ Options
..warning::
Do not use this option unless all its implications are fully understood. This option is intended only for extremely large zones (comparable to ``com.``) with sparse secure delegations.
; Copyright (C) Internet Systems Consortium, Inc. ("ISC")
;
; SPDX-License-Identifier: MPL-2.0
;
; This Source Code Form is subject to the terms of the Mozilla Public
; License, v. 2.0. If a copy of the MPL was not distributed with this
; file, you can obtain one at https://mozilla.org/MPL/2.0/.
;
; See the COPYRIGHT file distributed with this work for additional
; information regarding copyright ownership.
$TTL 600
@ SOA ns hostmaster 2011012708 3600 1200 604800 1200
NS ns
ns A 192.0.2.1
; invalid zero length alpn (at end) due to missing double escape
svcb SVCB 1 . alpn=abc\,
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Petr Špaček
Michał KępieńandPetr Špaček
Aram SargsyanandPetr Špaček
Ondřej Surý
Michal Nowak
Evan Hunt
Aydın Mercan
Nicki KřížekandPetr Špaček
Mark Andrews
Yedaya KatsmanandMark Andrews
Tom KrizekandNicki Křížek
Matthijs Mekking
Suzanne GoldlustandMatthijs Mekking
Emanuel PetrandNicki Křížek
Artem Boldariev
Štěpán BalážikandPetr Špaček
Petr MenšíkandPetr Špaček
Dominik ThalhammerandOndřej Surý