Commit graph

15476 commits

Author SHA1 Message Date
Ondřej Surý
acdc57259f Fix the assertion failure in the isc_hashmap iterator
When the round robin hashing reorders the map entries on deletion, we
were adjusting the iterator table size only when the reordering was
happening at the internal table boundary.  The iterator table size had
to be reduced by one to prevent seeing the entry that resized on
position [0] twice because it migrated to [iter->size - 1] position.

However, the same thing could happen when the same entry migrates a
second time from [iter->size - 1] to [iter->size - 2] position (and so
on) because the check that we are manipulating the entry just in the [0]
position was insufficient.  Instead of checking the position [pos == 0],
we now check that the [pos % iter->size == 0], thus ignoring all the
entries that might have moved back to the end of the internal table.
2024-08-14 15:19:04 +00:00
Ondřej Surý
86f1ec34dc Silence all warnings that stem from the default config
As we now setup the logging very early, parsing the default config would
always print warnings about experimental (and possibly deprecated)
options in the default config.  This would even mess with commands like
`named -V` and it is also wrong to warn users about using experimental
options in the default config, because they can't do anything about
this.  Add CFG_PCTX_NODEPRECATED and CFG_PCTX_NOEXPERIMENTAL options
that we can pass to cfg parser and silence the early warnings caused by
using experimental options in the default config.
2024-08-14 12:50:31 +00:00
Aydın Mercan
596903a6b7
use deterministic ecdsa for openssl >= 3.2
OpenSSL has added support for deterministic ECDSA (RFC 6979) with
version 3.2.

Use it by default as derandomization doesn't pose a risk for DNS
usecases and is allowed by FIPS 186-5.
2024-08-14 14:34:44 +03:00
Ondřej Surý
8e86e55af1
Don't skip the counting if fcount_incr() is called with force==true (v2)
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.
2024-08-13 12:51:22 +02:00
Ondřej Surý
39aef50b9b
Move the dst__openssl_toresult to isc_tls unit
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.
2024-08-08 11:59:41 +02:00
Evan Hunt
104f3b82fb implement 'max-query-restarts'
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.
2024-08-07 13:20:05 -07:00
Evan Hunt
7e3b425dc2 reduce the max-recursion-queries default to 32
the number of iterative queries that can be sent to resolve a
name now defaults to 32 rather than 100.
2024-08-07 13:19:57 -07:00
Evan Hunt
c5588babaf make "max_restarts" a configurable value
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.
2024-08-07 13:03:08 -07:00
Evan Hunt
05d78671bb reduce MAX_RESTARTS to 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.)
2024-08-07 13:00:42 -07:00
Evan Hunt
825f3d68c5 add debug logging when creating or attaching to a query counter
fctx_create() now logs at debug level 9 when the fctx attaches
to an existing counter or creates a new one.
2024-08-07 11:21:44 -07:00
Evan Hunt
af7db89513 apply max-recursion-queries quota to validator queries
previously, validator queries for DNSKEY and DS records were
not counted toward the quota for max-recursion-queries; they
are now.
2024-08-07 11:21:44 -07:00
Evan Hunt
d3b7e92783 attach query counter to NS fetches
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.
2024-08-07 11:21:44 -07:00
Aydın Mercan
f58ed932d8
use only c23 or c11 noreturn specifiers
Since we require C11 or greater, we can depend on using either _Noreturn
or [[noreturn]].
2024-08-07 18:27:40 +03:00
Ondřej Surý
e6f2f2a5e6
Initialize the DST subsystem implicitly
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.
2024-08-07 17:03:27 +02:00
Ondřej Surý
c11b736e44 Disassociate the SSL object from the cached SSL_SESSION
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>
2024-08-07 14:25:11 +00:00
Ondřej Surý
684f3eb8e6 Attach/detach to the listening child socket when accepting TLS
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.
2024-08-07 14:17:43 +00:00
Ondřej Surý
495cf18c75
Remove checks for OPENSSL_API_LEVEL define
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.
2024-08-06 15:17:48 +02:00
Ondřej Surý
ef7aba7072
Remove OpenSSL Engine support
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.
2024-08-06 15:17:48 +02:00
Ondřej Surý
5beae5faf9
Fix the glue table in the QP and RBT zone databases
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.
2024-08-05 15:36:54 +02:00
Evan Hunt
6b720bfe1a minor findnode optimization
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.
2024-08-05 13:36:41 +00:00
Evan Hunt
a68a77ca86 dns_difftuple_create() cannot fail
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.
2024-08-05 13:31:38 +00:00
Evan Hunt
a84d54c6ff
raise the log level of priming failures
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.
2024-08-05 13:56:13 +02:00
Aydın Mercan
2a76352b37 fix the rsa exponent to 65537
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.
2024-08-05 11:21:59 +00:00
Aydın Mercan
5dbb560747 remove the crc64 implementation
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.
2024-08-05 11:21:25 +00:00
Ondřej Surý
13941c8ca7 Call rcu_barrier() in the isc_mem_destroy() just once
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.
2024-08-05 10:24:47 +00:00
Ondřej Surý
8ccfbcfe72 Remove no longer needed OpenSSL shims and checks
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>
2024-08-05 10:23:59 +00:00
Ondřej Surý
37dbd57c16
Fix the assertion failure when putting 48-bit number to buffer
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.
2024-08-05 09:55:18 +02:00
Ondřej Surý
a513d4c07f Don't skip the counting if fcount_incr() is called with force==true
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.
2024-08-05 07:33:20 +00:00
Ondřej Surý
827a153d99 Remove superfluous memset() in isc_nmsocket_init()
The tlsstream part of the isc_nmsocket_t gets initialized via designater
initializer and doesn't need the extra memset() later; just remove it.
2024-08-05 07:32:12 +00:00
Ondřej Surý
cc4f99bc6d Fix PTHREAD_MUTEX_ADAPTIVE_NP and PTHREAD_MUTEX_ERRORCHECK_NP usage
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.
2024-08-05 07:31:39 +00:00
Ondřej Surý
f158884344 Remove ISC_MUTEX_INITIALIZER
It's hard to get it right on different platforms and it's unused
in BIND 9 anyway.
2024-08-05 07:31:39 +00:00
Ondřej Surý
b26079fdaf Don't open route socket if we don't need it
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.
2024-08-05 07:31:02 +00:00
Ondřej Surý
912eaf6cb9 Clarify that cds_wfcq_dequeue_blocking() doesn't block if empty 2024-08-05 07:30:10 +00:00
Mark Andrews
47338c2c87 Remove unnecessary operations
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) {
2024-08-02 03:44:04 +00:00
Aram Sargsyan
5f47c2b567 Allow shorter resolver-query-timeout configuration
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.
2024-08-01 18:30:35 +00:00
Aram Sargsyan
63b8a75de9 Rename dns_zone_forcereload() to dns_zone_forcexfr()
The new name describes the function more accurately.
2024-08-01 11:01:17 +00:00
Aram Sargsyan
3d1179501a Make dns_xfrin_shutdown() safe to run from a different loop
If the current loop is different than the zone transfer's loop then
run the shutdown operation asynchronously.
2024-08-01 10:43:47 +00:00
Aram Sargsyan
402ca316ae Implement rndc retransfer -force
With this new optional argument if there is an ongoing zone
transfer it will be aborted before a new zone transfer is scheduled.
2024-08-01 10:43:47 +00:00
Aram Sargsyan
b156531b29 Do not automatically restart a canceled zone transfer
If a zone transfer is canceled there is no need to try the
next primary or retry with AXFR.
2024-08-01 10:43:47 +00:00
Mark Andrews
bca63437a1 Add missing period to generated IPv4 6to4 name
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.
2024-08-01 15:17:30 +10:00
Mark Andrews
6d1c7beb15 Cleanup old clang-format string splitting 2024-08-01 14:17:57 +10:00
Mark Andrews
f78beca942 Remove false positive qname minimisation error
Don't report qname minimisation NXDOMAIN errors when the result is
NXDOMAIN.
2024-08-01 14:17:57 +10:00
Mark Andrews
393d7fa78e Fix yaml output
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.
2024-08-01 12:30:57 +10:00
Mark Andrews
b51c9eb797 Properly reject zero length ALPN in commatxt_fromtext
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.
2024-08-01 10:20:55 +10:00
Aram Sargsyan
cb5238cc62 Replace #define DNS_GETDB_ with struct of bools
This makes it easier to pretty-print the attributes in a debugger.
2024-07-31 11:52:52 +00:00
Aram Sargsyan
b621f1d88e Return SERVFAIL for a too long CNAME chain
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.
2024-07-31 10:54:10 +00:00
Mark Andrews
48d39f7c30 Check that FILE_STREAM(channel) is not already closed
isc_log_closefilelogs can also close log files.  isc_log_doit failed
to check if the file handle was still valid before closing it.
2024-07-31 17:36:38 +10:00
Mark Andrews
e8dbc5db92 Properly compute the physical memory size
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) */
2024-07-31 05:55:30 +00:00
Mark Andrews
53a5f50e9d Do not update find.result_v4 and find.result_v6
These values are supposed to be static for the life of the find and
clean_finds_at_name was updating them resulting in TSAN error
reports.

    WARNING: ThreadSanitizer: data race
    Write of size 4 at 0x000000000001 by thread T1 (mutexes: write M1, write M2):
      #0 clean_finds_at_name lib/dns/adb.c:1537
      #1 fetch_callback lib/dns/adb.c:4009
      #2 task_run lib/isc/task.c:815
      #3 isc_task_run lib/isc/task.c:896
      #4 isc__nm_async_task netmgr/netmgr.c:848
      #5 process_netievent netmgr/netmgr.c:920
      #6 process_queue netmgr/netmgr.c:1013
      #7 process_all_queues netmgr/netmgr.c:767
      #8 async_cb netmgr/netmgr.c:796
      #9 uv__async_io /usr/src/libuv-v1.44.1/src/unix/async.c:163
      #10 isc__trampoline_run lib/isc/trampoline.c:189

    Previous read of size 4 at 0x000000000001 by thread T2:
      #0 findname lib/dns/resolver.c:3749
      #1 fctx_getaddresses lib/dns/resolver.c:3993
      #2 fctx_try lib/dns/resolver.c:4390
      #3 rctx_nextserver lib/dns/resolver.c:10356
      #4 rctx_done lib/dns/resolver.c:10503
      #5 resquery_response lib/dns/resolver.c:8511
      #6 udp_recv lib/dns/dispatch.c:638
      #7 isc__nm_async_readcb netmgr/netmgr.c:2885
      #8 isc__nm_readcb netmgr/netmgr.c:2858
      #9 udp_recv_cb netmgr/udp.c:650
      #10 isc__nm_udp_read_cb netmgr/udp.c:1057
      #11 uv__udp_recvmsg /usr/src/libuv-v1.44.1/src/unix/udp.c:303
      #12 isc__trampoline_run lib/isc/trampoline.c:189
2024-07-31 14:46:45 +10:00
Mark Andrews
14a76ae498 Log key calculation overflows 2024-07-30 10:58:54 +02:00