Commit Graph
3352 Commits
Author SHA1 Message Date
Mark Andrews ed0cb2e9e4 Use ns_g_server->recursionquota instead of client->recursionquota
as the later will be NULL on ISC_R_QUOTA.

*** CID 306732:  Null pointer dereferences  (FORWARD_NULL)
/bin/named/query.c: 4359 in query_recurse()
4353     			static _Atomic(isc_stdtime_t) last = 0;
4354     #else
4355     			static isc_stdtime_t last = 0;
4356     #endif
4357     			isc_stdtime_t now;
4358     			isc_stdtime_get(&now);
   CID 306732:  Null pointer dereferences  (FORWARD_NULL)
   Passing "client" to "log_quota", which dereferences null "client->recursionquota".
4359     			log_quota(client, &last, now,
4360     				  "no more recursive clients (%d/%d/%d): %s",
4361     				  isc_result_totext(result));
4362     			ns_client_killoldestquery(client);
4363     		}
4364     		if (result == ISC_R_SUCCESS && !client->mortal &&

** CID 306731:  Program hangs  (LOCK)
/lib/dns/resolver.c: 8943 in spillattimer_countdown()
2020-09-09 06:01:54 +00:00
Mark Andrews d6c727d669 ISC_QUEUE_POP is not tsan safe. Suppress warnings
ret->link.next is tested to see if it is NULL unlocked
to avoid obtaining taillock when it is not nexessary then
retested once the taillock is obtained.
2020-09-08 17:41:33 +10:00
Mark Andrews 6d9b1ed70c Address data race over query_recurse.last
WARNING: ThreadSanitizer: data race
  Read of size 4 at 0x000000000001 by thread T1:
    #0 query_recurse bin/named/query.c:4291:15
    #1 query_find bin/named/query.c
    #2 ns_query_start bin/named/query.c:9675:8
    #3 client_request bin/named/client.c:3133:3
    #4 dispatch lib/isc/task.c:1157:7
    #5 run lib/isc/task.c:1331:2

  Previous write of size 4 at 0x000000000001 by thread T2:
    #0 query_recurse bin/named/query.c:4292:10
    #1 query_find bin/named/query.c
    #2 ns_query_start bin/named/query.c:9675:8
    #3 client_request bin/named/client.c:3133:3
    #4 dispatch lib/isc/task.c:1157:7
    #5 run lib/isc/task.c:1331:2

  Location is global 'query_recurse.last' of size 4 at 0x000000000001
2020-09-08 09:25:43 +10:00
Mark Andrews 72cbe648c4 Test if linked while holding the queue lock
WARNING: ThreadSanitizer: data race
  Read of size 8 at 0x000000000001 by thread T1:
    #0 client_shutdown bin/named/client.c:849:6
    #1 dispatch lib/isc/task.c:1157:7
    #2 run lib/isc/task.c:1331:2

  Previous write of size 8 at 0x000000000001 by thread T2 (mutexes: write M1, write M2):
    #0 client_shutdown bin/named/client.c:850:3
    #1 dispatch lib/isc/task.c:1157:7
    #2 run lib/isc/task.c:1331:2
2020-09-08 09:25:43 +10:00
Mark Andrews be4ed41611 Obtain a lock on the quota structure.
WARNING: ThreadSanitizer: data race (pid=15228)
  Read of size 4 at 0x7b5c00000444 by thread T8:
    #0 ns_server_status /builds/isc-projects/bind9/bin/named/./server.c:10935:31 (named+0x514671)
    #1 ns_control_docommand /builds/isc-projects/bind9/bin/named/control.c:263:12 (named+0x4e4726)
    #2 control_recvmessage /builds/isc-projects/bind9/bin/named/controlconf.c:465:13 (named+0x4e9046)
    #3 dispatch /builds/isc-projects/bind9/lib/isc/task.c:1157:7 (libisc.so.1107+0x50845)
    #4 run /builds/isc-projects/bind9/lib/isc/task.c:1331:2 (libisc.so.1107+0x4d799)

  Previous write of size 4 at 0x7b5c00000444 by thread T9 (mutexes: write M1082):
    #0 isc_quota_release /builds/isc-projects/bind9/lib/isc/quota.c:73:13 (libisc.so.1107+0x3bf10)
    #1 isc_quota_detach /builds/isc-projects/bind9/lib/isc/quota.c:111:2 (libisc.so.1107+0x3c12b)
    #2 ns_client_endrequest /builds/isc-projects/bind9/bin/named/client.c:896:3 (named+0x4dcad1)
    #3 exit_check /builds/isc-projects/bind9/bin/named/client.c:512:3 (named+0x4d570d)
    #4 ns_client_detach /builds/isc-projects/bind9/bin/named/client.c:3687:8 (named+0x4d7732)
    #5 query_find /builds/isc-projects/bind9/bin/named/query.c (named+0x4f8ff1)
    #6 query_resume /builds/isc-projects/bind9/bin/named/query.c:4164:12 (named+0x509b38)
    #7 dispatch /builds/isc-projects/bind9/lib/isc/task.c:1157:7 (libisc.so.1107+0x50845)
    #8 run /builds/isc-projects/bind9/lib/isc/task.c:1331:2 (libisc.so.1107+0x4d799)
2020-09-08 09:25:43 +10:00
Mark Andrews 818520216d Missing locks in ns_lwresd_shutdown.
WARNING: ThreadSanitizer: data race
  Read of size 8 at 0x000000000001 by main thread:
    #0 ns_lwresd_shutdown bin/named/lwresd.c:885:3
    #1 destroy_managers bin/named/./main.c:938:2
    #2 cleanup bin/named/./main.c:1346:2
    #3 main bin/named/./main.c:1594:2

  Previous write of size 8 at 0x000000000001 by thread T1 (mutexes: write M1):
    #0 configure_listener bin/named/lwresd.c:768:2
    #1 ns_lwresd_configure bin/named/lwresd.c:836:5
    #2 load_configuration bin/named/./server.c:8230:2
    #3 run_server bin/named/./server.c
    #4 dispatch lib/isc/task.c:1157:7
    #5 run lib/isc/task.c:1331:2
2020-09-08 09:25:43 +10:00
Mark Andrews 00d315046b Defer read of zl->server and zl->reconfig until
the reference counter has gone to zero and there is
nolonger a possibility of changes in other threads.
2020-09-08 09:25:43 +10:00
Mark Andrews a26d0c5ef3 Access ns_client_requests atomically 2020-09-08 09:25:43 +10:00
Mark AndrewsandMichał Kępień e4cccf9668 Update-policy 'subdomain' was incorrectly treated as 'zonesub'
resulting in names outside the specified subdomain having the wrong
restrictions for the given key.
2020-08-05 15:55:14 +02:00
Mark Andrews 5770740587 Map DNS_R_BADTSIG to FORMERR
Now that the log message has been printed set the result code to
DNS_R_FORMERR.  We don't do this via dns_result_torcode() as we
don't want upstream errors to produce FORMERR if that processing
end with DNS_R_BADTSIG.

(cherry picked from commit 20488d6ad3)
2020-08-04 23:07:02 +10:00
Petr MenšíkandMark Andrews 35fbfaa498 Prevent crash on dst initialization failure
server might be created, but not yet fully initialized, when fatal
function is called. Check both server and task before attaching
exclusive task.

(cherry picked from commit c5e7152cf0)
2020-07-23 11:29:57 +10:00
Evan Hunt 0f7f6201e3 make sure new_zone_lock is locked before unlocking it
it was possible for the count_newzones() function to try to
unlock view->new_zone_lock on return before locking it, which
caused a crash on shutdown.

(cherry picked from commit ed37c63e2b)
2020-07-13 12:32:51 -07:00
Mark Andrews 55f567ab35 Fallback to built in trust-anchors, managed-keys, or trusted-keys
if the bind.keys file cannot be parsed.

(cherry picked from commit d02a14c795)
2020-07-13 15:55:59 +10:00
Michał Kępień 25818ac81f Fix locking for LMDB 0.9.26
When "rndc reconfig" is run, named first configures a fresh set of views
and then tears down the old views.  Consider what happens for a single
view with LMDB enabled; "envA" is the pointer to the LMDB environment
used by the original/old version of the view, "envB" is the pointer to
the same LMDB environment used by the new version of that view:

 1. mdb_env_open(envA) is called when the view is first created.
 2. "rndc reconfig" is called.
 3. mdb_env_open(envB) is called for the new instance of the view.
 4. mdb_env_close(envA) is called for the old instance of the view.

This seems to have worked so far.  However, an upstream change [1] in
LMDB which will be part of its 0.9.26 release prevents the above
sequence of calls from working as intended because the locktable mutexes
will now get destroyed by the mdb_env_close() call in step 4 above,
causing any subsequent mdb_txn_begin() calls to fail (because all of the
above steps are happening within a single named process).

Preventing the above scenario from happening would require either
redesigning the way we use LMDB in BIND, which is not something we can
easily backport, or redesigning the way BIND carries out its
reconfiguration process, which would be an even more severe change.

To work around the problem, set MDB_NOLOCK when calling mdb_env_open()
to stop LMDB from controlling concurrent access to the database and do
the necessary locking in named instead.  Reuse the view->new_zone_lock
mutex for this purpose to prevent the need for modifying struct dns_view
(which would necessitate library API version bumps).  Drop use of
MDB_NOTLS as it is made redundant by MDB_NOLOCK: MDB_NOTLS only affects
where LMDB reader locktable slots are stored while MDB_NOLOCK prevents
the reader locktable from being used altogether.

[1] https://git.openldap.org/openldap/openldap/-/commit/2fd44e325195ae81664eb5dc36e7d265927c5ebc

(cherry picked from commit 53120279b5)
2020-07-10 11:31:43 +02:00
Mark Andrews 003d9d55ca Resize unamebuf[] to avoid warnings about snprintf() not having
enough buffer space.  Also change named_os_uname() prototype so
that it is now returning (const char *) rather than (char *).  If
uname() is not supported on a UNIX build prepopulate unamebuf[]
with "unknown architecture".

(cherry picked from commit 4bc3de070f)
2020-06-25 09:32:02 +10:00
Mark Andrews 2f75aa8120 Add DBC checks to named_checknames_get
(cherry picked from commit 20c07da49b)
2020-05-25 11:56:15 +10:00
Mark Andrews 35964017d0 Set obj to NULL so INSIST makes sense
(cherry picked from commit ac9e266749)
2020-05-25 11:56:14 +10:00
Mark Andrews 7c1043ae73 move provide-ixfr testing after the serial has been checked
(cherry picked from commit c7cdc47cc5)
(cherry picked from commit 89c1bc58e4)
2020-05-18 09:40:13 +10:00
Mark AndrewsandOndřej Surý 94ec98ad7a Propagate first_time to named_os_openfile in generate_session_key.
named_os_openfile was being called with switch_user set to true
unconditionally leading to log messages about being unable to
switch user identity from named when regenerating the key.

(cherry picked from commit 071bc29962)
2020-04-28 17:31:26 +02:00
Ondřej SurýandOndřej Surý 89ad4912e9 Don't change effective uid when we already dropped privileges
When running on Linux and system capabilities are available, named will
drop the extra capabilities before loading the configuration.  This led
to spurious warnings from `seteuid()` because named already dropped
CAP_SETUID and CAP_GETUID capabilities.

The fix removes setting the effective uid/gid when capabilities are
available, and adds a check that we are running under the user we were
requested to run.

(cherry picked from commit 6c82e2af92)
2020-04-28 17:31:26 +02:00
Tinderbox UserandMichał Kępień aaa281ec6b regen v9_11 2020-04-16 23:10:26 +02:00
Ondřej Surý 5fd1df8cfa Disable MSB8028 warning
All our MSVS Project files share the same intermediate directory.  We
know that this doesn't cause any problems, so we can just disable the
detection in the project files.

Example of the warning:

  warning MSB8028: The intermediate directory (.\Release\) contains files shared from another project (dnssectool.vcxproj).  This can lead to incorrect clean and rebuild behavior.

(cherry picked from commit b6c2012d93)
2020-04-16 08:12:39 +02:00
Ondřej Surý 1eb5a77baa Set WarningLevel to Level1 for Release, treat warnings as errors
Our vcxproj files set the WarningLevel to Level3, which is too verbose
for a code that needs to be portable.  That basically leads to ignoring
all the errors that MSVC produces.  This commits downgrades the
WarningLevel to Level1 and enables treating warnings as errors for
Release builds.  For the Debug builds the WarningLevel got upgraded to
Level4, and treating warnings as errors is explicitly disabled.

We should eventually make the code clean of all MSVC warnings, but it's
a long way to go for Level4, so it's more reasonable to start at Level1.

For reference[1], these are the warning levels as described by MSVC
documentation:

  * /W0 suppresses all warnings. It's equivalent to /w.
  * /W1 displays level 1 (severe) warnings. /W1 is the default setting
    in the command-line compiler.
  * /W2 displays level 1 and level 2 (significant) warnings.
  * /W3 displays level 1, level 2, and level 3 (production quality)
    warnings. /W3 is the default setting in the IDE.
  * /W4 displays level 1, level 2, and level 3 warnings, and all level 4
    (informational) warnings that aren't off by default. We recommend
    that you use this option to provide lint-like warnings. For a new
    project, it may be best to use /W4 in all compilations. This option
    helps ensure the fewest possible hard-to-find code defects.
  * /Wall displays all warnings displayed by /W4 and all other warnings
    that /W4 doesn't include — for example, warnings that are off by
    default.
  * /WX treats all compiler warnings as errors. For a new project, it
    may be best to use /WX in all compilations; resolving all warnings
    ensures the fewest possible hard-to-find code defects.

1. https://docs.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level?view=vs-2019

(cherry picked from commit 789d253e3d)
2020-04-16 08:12:39 +02:00
Evan Hunt beaca3e8db remove or comment empty conditional branches
some empty conditional branches which contained a semicolon were
"fixed" by clang-format to contain nothing. add comments to prevent this.

(cherry picked from commit 735be3b816)
2020-03-17 15:39:03 -07:00
Mark Andrews 4166d06042 Quote zone name so that specials are handled
(cherry picked from commit 59498ce17f)
2020-03-13 15:49:44 +11:00
Mark Andrews be900e9ba7 Pass NUL terminated buffer name to cfg_parse_buffer
(cherry picked from commit 91efc587b2)
2020-03-13 15:49:44 +11:00
Michał Kępień ab4c911629 Fix cppcheck 1.90 warning
cppcheck 1.90 reports the following issue for bin/named/query.c:

    bin/named/query.c:6838:2: warning: %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
     snprintf(mbuf, sizeof(mbuf) - 1,
     ^

Tweak the format specifier for client->query.restarts to address the
problem.
2020-03-04 12:41:01 +01:00
Evan Hunt 191b616579 fix additional spelling errors 2020-02-24 22:04:43 -08:00
Evan Hunt e63223332c fix spelling errors reported by Fossies. 2020-02-21 14:09:59 -08:00
Tinderbox UserandEvan Hunt 6b5916325f regen v9_11 2020-01-27 11:10:16 -08:00
Tony FinchandMark Andrews e0a7baa6af Fix line spacing in rndc secroots
Before this change, there was a missing blank line between the
negative trust anchors for one view, and the heading line for the next
view. This is because dns_ntatable_totext() omits the last newline.
There is an example of the incorrect output below; the fixed output
has a blank line before "Start view auth".

secure roots as of 21-Oct-2019 12:03:23.500:

 Start view rec
   Secure roots:

./RSASHA256/20326 ; managed

   Negative trust anchors:

example.com: expiry 21-Oct-2019 13:03:15.000
 Start view auth
   Secure roots:

./RSASHA256/20326 ; managed

   Negative trust anchors:

example.com: expiry 21-Oct-2019 13:03:07.000

(cherry picked from commit 5b600c2cd8)
2020-01-14 08:57:17 +11:00
Evan Hunt 50ce299518 update copyright year to 2020 2020-01-02 21:50:03 -08:00
Petr MenšíkandMark Andrews a9a39eb5b5 Include protobuf-c version
Include used version of protobuf-c in version info, both link time and
runtime version is available.

(cherry picked from commit 85f3476894)
2019-12-18 11:39:20 +11:00
Petr MenšíkandMark Andrews a6f25f9d25 Provide GeoIP2 library version in version
Libmaxmind does not provide any version macro for link time version.
Print at least runtime version library used, if linked.

(cherry picked from commit e6d7384c0d)
2019-12-18 11:38:41 +11:00
Michał Kępień e396979233 Move xmlInitThreads()/xmlCleanupThreads() calls
xmlInitThreads() and xmlCleanupThreads() are called from within
ns_statschannels_configure() and ns_statschannels_shutdown(),
respectively.  Both of these functions are executed by worker threads,
not the main named thread.  This causes ASAN to report memory leaks like
the following one upon shutdown (as long as named is asked to produce
any XML output over its configured statistics channels during its
lifetime):

    Direct leak of 968 byte(s) in 1 object(s) allocated from:
        #0 0x7f677c249cd8 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:153
        #1 0x7f677bc1838f in xmlGetGlobalState (/usr/lib/libxml2.so.2+0xa838f)

The data mentioned in the above report is a libxml2 state structure
stored as thread-specific data.  Such chunks of memory are automatically
released (by a destructor passed to pthread_key_create() by libxml2)
whenever a thread that allocated a given chunk exits.  However, if
xmlCleanupThreads() is called by a given thread before it exits, the
destructor will not be invoked (due to xmlCleanupThreads() calling
pthread_key_delete()) and ASAN will report a memory leak.  Thus,
xmlInitThreads() and xmlCleanupThreads() must not be called from worker
threads.  Since xmlInitThreads() must be called on Windows in order for
libxml2 to work at all, move xmlInitThreads() and xmlCleanupThreads()
calls to the main named thread (which does not produce any XML output
itself) in order to prevent the memory leak from being reported by ASAN.

(cherry picked from commit b425b5d56e)
2019-12-02 17:10:03 +01:00
Michał Kępień 95a5589fa2 Fix GeoIP2 memory leak upon reconfiguration
Loaded GeoIP2 databases are only released when named is shut down, but
not during server reconfiguration.  This causes memory to be leaked
every time "rndc reconfig" or "rndc reload" is used, as long as any
GeoIP2 database is in use.  Fix by releasing any loaded GeoIP2 databases
before reloading them.  Do not call dns_geoip_shutdown() until server
shutdown as that function releases the memory context used for caching
GeoIP2 lookup results.

(cherry picked from commit 670afbe84a)
2019-12-02 15:27:00 +01:00
Evan Hunt 47da3129fd add a stats counter for clients dropped due to recursive-clients limit
(cherry picked from commit 715afa9c57)
2019-11-26 11:34:23 -08:00
Ondřej Surý 1e96fb8dc0 Request exclusive access when crashing via fatal()
When loading the configuration fails, there might be already other tasks
running and calling OpenSSL library functions.  The OpenSSL on_exit
handler is called when exiting the main process and there's a timing
race between the on_exit function that destroys OpenSSL allocated
resources (threads, locks, ...) and other tasks accessing the very same
resources leading to a crash in the system threading library. Therefore,
the fatal() function needs to request exlusive access to the task
manager to finish the already running tasks and exit only when no other
tasks are running.

(cherry picked from commit 952d7fde63)
2019-11-26 13:56:02 +01:00
Evan Hunt 99203103b7 Merge tag 'v9_11_13' into merge-v9_11_13 2019-11-20 13:43:34 -08:00
Ondřej Surý d2c86b2159 Internally, use {PATH,NAME}_MAX instead of ISC_DIR_{PATH,NAME}MAX 2019-11-12 09:31:40 +01:00
Samuel ThibaultandOndřej Surý 88061c2acb hurd: Fix build
Move PATH_MAX, NAME_MAX, IOV_MAX default definitions to the common
<isc/platform.h>.

(cherry picked from commit d10fbdec84)
2019-11-12 09:22:15 +01:00
Witold KręcickiandMichał Kępień 7562485282 libns: Rename ns_tcpconn refs member to clients 2019-11-06 21:32:54 +01:00
Witold KręcickiandMichał Kępień 7d14ea3795 Limit query pipelining within each TCP connection
Previously, there was no limit to the number of concurrently served
queries over one pipelined TCP connection; an unlimited number of
queries sent over a single TCP connection could have potentially
exhausted the server's resources.
2019-11-06 21:32:54 +01:00
Diego FronzaandOndřej Surý 4ad2ab54cd Added TCP high-water statistics variable
This variable will report the maximum number of simultaneous tcp clients
that BIND has served while running.

It can be verified by running rndc status, then inspect "tcp high-water:
count", or by generating statistics file, rndc stats, then inspect the
line with "TCP connection high-water" text.

The tcp-highwater variable is atomically updated based on an existing
tcp-quota system handled in ns/client.c.

(cherry picked from commit 66fe8627de)
2019-11-06 12:54:40 +01:00
Mark Andrews 0e6259fd7b Have 'named -V' report geoip-directory
(cherry picked from commit 2eaa75c380)
2019-11-06 12:12:43 +11:00
Mark Andrews 7e79ebeeba The default geoip-directory should be <MAXMINDDB_PREFIX>/share/GeoIP
(cherry picked from commit fcd765a59d)
2019-11-06 11:50:09 +11:00
Ondřej Surý d374009464 named: remove named_g_defaultdnstap global variable
The named_g_defaultdnstap was never used as the dnstap requires
explicit configuration of the output file.

Related scan-build report:

./server.c:3476:14: warning: Value stored to 'dpath' during its initialization is never read
        const char *dpath = named_g_defaultdnstap;
                    ^~~~~   ~~~~~~~~~~~~~~~~~~~~~
1 warning generated.

(cherry picked from commit 6decd14592)
2019-11-05 09:52:52 +01:00
Ondřej Surý 1968639776 named: Add INSIST() after bindkeysfile configuration load to silence scan-build FP
(cherry picked from commit 6bf364aec8)
2019-11-05 09:49:58 +01:00
Michał Kępień a5f25f837f Make "geoip-use-ecs yes;" work for GeoIP2
Add a missing preprocessor condition to make "geoip-use-ecs yes;" work
for GeoIP2.
2019-10-30 16:05:28 +01:00
Ondřej Surý 7f0152632f lib/ns/query.c: Fix invalid order of DbC checks that could cause dereference before NULL check
(cherry picked from commit d1f035bbba)
2019-10-03 14:21:47 +02:00