The use of isc_appctx_t in dns_client was used to wait for
dns_client_startresolve() to finish the processing (the resolve_done()
task callback).
This has been replaced with standard bool+cond+lock combination removing
the need of isc_appctx_t altogether.
Previously, the isc_ht API would always take the key as a literal input
to the hashing function. Change the isc_ht_init() function to take an
'options' argument, in which ISC_HT_CASE_SENSITIVE or _INSENSITIVE can
be specified, to determine whether to use case-sensitive hashing in
isc_hash32() when hashing the key.
In couple places, we have missed INSIST(0) or ISC_UNREACHABLE()
replacement on some branches with UNREACHABLE(). Replace all
ISC_UNREACHABLE() or INSIST(0) calls with UNREACHABLE().
This commit extends the 'doth' system test with a set of Strict/Mutual
TLS related checks.
This commit also makes each doth NS instance use its own TLS
certificate that includes FQDN, IPv4, and IPv6 addresses, issued using
a common Certificate Authority, instead of ad-hoc certs.
Extend servers initialisation timeout to 60 seconds to improve the
tests stability in the CI as certain configurations could fail to
initialise on time under load.
This commit adds support for Strict/Mutual TLS into BIND. It does so
by implementing the backing code for 'hostname' and 'ca-file' options
of the 'tls' statement. The commit also updates the documentation
accordingly.
This commit adds support for Strict/Mutual TLS to dig.
The new command-line options and their behaviour are modelled after
kdig (+tls-ca, +tls-hostname, +tls-certfile, +tls-keyfile) for
compatibility reasons. That is, using +tls-* is sufficient to enable
DoT in dig, implying +tls-ca
If there is no other DNS transport specified via command-line,
specifying any of +tls-* options makes dig use DoT. In this case, its
behaviour is the same as if +tls-ca is specified: that is, the remote
peer's certificate is verified using the platform-specific
intermediate CA certificates store. This behaviour is introduced for
compatibility with kdig.
Because of the "goto" in the "if" body the "else" part is unnecessary
and adds another level of indentation.
Cleanup the code to not have the "else" part.
Catz logs a warning message when it is told to modify a zone which was
not added by the current catalog zone.
When logging a warning, distinguish the two cases when the zone
was not added by a catalog zone at all, and when the zone was
added by a different catalog zone.
The dnssec-settime -p and -up options print times in asctime() and
UNIX time_t formats, respectively. The asctime() format can also be
found inside K*.key public key files. Key files also contain times in
the YYYYMMDDHHMMSS format that can be used in timing parameter
options.
The dnssec-settime -p and -up time formats are now acceptable in
timing parameter options to dnssec-settime and dnssec-keygen, so it is
no longer necessary to parse key files to retrieve times that are
acceptable in timing parameter options.
Previously, it was possible to assign a bit of memory space in the
nmhandle to store the client data. This was complicated and prevents
further refactoring of isc_nmhandle_t caching (future work).
Instead of caching the data in the nmhandle, allocate the hot-path
ns_client_t objects from per-thread clientmgr memory context and just
assign it to the isc_nmhandle_t via isc_nmhandle_set().
Historically, the inline keyword was a strong suggestion to the compiler
that it should inline the function marked inline. As compilers became
better at optimising, this functionality has receded, and using inline
as a suggestion to inline a function is obsolete. The compiler will
happily ignore it and inline something else entirely if it finds that's
a better optimisation.
Therefore, remove all the occurences of the inline keyword with static
functions inside single compilation unit and leave the decision whether
to inline a function or not entirely on the compiler
NOTE: We keep the usage the inline keyword when the purpose is to change
the linkage behaviour.
C11 has builtin support for _Noreturn function specifier with
convenience noreturn macro defined in <stdnoreturn.h> header.
Replace ISC_NORETURN macro by C11 noreturn with fallback to
__attribute__((noreturn)) if the C11 support is not complete.
Previously, the unreachable code paths would have to be tagged with:
INSIST(0);
ISC_UNREACHABLE();
There was also older parts of the code that used comment annotation:
/* NOTREACHED */
Unify the handling of unreachable code paths to just use:
UNREACHABLE();
The UNREACHABLE() macro now asserts when reached and also uses
__builtin_unreachable(); when such builtin is available in the compiler.
Gcc 7+ and Clang 10+ have implemented __attribute__((fallthrough)) which
is explicit version of the /* FALLTHROUGH */ comment we are currently
using.
Add and apply FALLTHROUGH macro that uses the attribute if available,
but does nothing on older compilers.
In one case (lib/dns/zone.c), using the macro revealed that we were
using the /* FALLTHROUGH */ comment in wrong place, remove that comment.
Instead of passing the "workers" variable back and forth along with
passing the single isc_nm_t instance, add isc_nm_getnworkers() function
that returns the number of netmgr threads are running.
Change the ns_interfacemgr and ns_taskmgr to utilize the newly acquired
knowledge.
From an attacker's point of view, a VLA declaration is essentially a
primitive for performing arbitrary arithmetic on the stack pointer. If
the attacker can control the size of a VLA they have a very powerful
tool for causing memory corruption.
To mitigate this kind of attack, and the more general class of stack
clash vulnerabilities, C compilers insert extra code when allocating a
VLA to probe the growing stack one page at a time. If these probes hit
the stack guard page, the program will crash.
From the point of view of a C programmer, there are a few things to
consider about VLAs:
* If it is important to handle allocation failures in a controlled
manner, don't use VLAs. You can use VLAs if it is OK for
unreasonable inputs to cause an uncontrolled crash.
* If the VLA is known to be smaller than some known fixed size,
use a fixed size array and a run-time check to ensure it is large
enough. This will be more efficient than the compiler's stack
probes that need to cope with arbitrary-size VLAs.
* If the VLA might be large, allocate it on the heap. The heap
allocator can allocate multiple pages in one shot, whereas the
stack clash probes work one page at a time.
Most of the existing uses of VLAs in BIND are in test code where they
are benign, but there was one instance in `named`, in the GSS-TSIG
verification code, which has now been removed.
This commit adjusts the style guide and the C compiler flags to allow
VLAs in test code but not elsewhere.
Rework the "ans8" server in the "digdelv" system test to support various
modes of operations using a control channel.
The supported modes are:
1. `silent` (do not respond)
2. `close` (UDP: same as `silent`; TCP: also close the connection)
3. `servfail` (always respond with `SERVFAIL`)
4. `unstable` (constantly switch between `silent` and `servfail`)
Add multiple tests to check the handling of both TCP and UDP socket
error scenarios in dig/host.
When encountering a TCP connection error while trying to initiate a
connection to a server, dig erroneously cancels the lookup even when
there are other server(s) to try, which results in an assertion failure.
Cancel the lookup only when there are no more queries left in the
lookup's queries list (i.e. `next` is NULL).
Add a test to check whether dig tries the next query/server after
a connection error.
Add a test to check whether dig tries the next query/server after
a one or more (default is 3) connection/request timeouts.
When timing-out or having other types of socket errors during a query,
dig isn't trying to perform the lookup using other servers which exist
in the lookup's queries list.
After configured amount of timeout retries, or after a socket error,
check if there are other queries/servers in the lookup's queries list,
and start the next one if it exists, instead of unconditionally failing.
This test ensures that `dig` retries with another attempt after a
timed-out request, and that it does not crash when the retried
request returns a SERVFAIL result. See [GL #3020] for the latter
issue.
When a query times out, and `dig` (or `host`) creates a new query
to resend the request, it is being prepended to the lookup's queries
list, which can cause a confusion later, making `dig` (or `host`)
believe that there is another new query in the list, but that is
actually the old one, which was timed out. That mistake will result
in an assertion failure.
That can happen, in particular, when after a timed out request,
the retried request returns a SERVFAIL result, and the recursion
is enabled, and `+nofail` option was used with `dig` (that is the
default behavior in `host`, unless the `-s` option is provided).
Fix the problem by inserting the query just after the current,
timed-out query, instead of prepending to the list.
Before calling start_udp() detach `l->current_query`, like it is
done in another place in the function.
Slightly update a couple of debug messages to make them more
consistent.
After a query results in a SERVFAIL result, and there is another
registered query in the lookup's queries list, `dig` starts the next
query to try another server, but for some reason, reports about that
also when the current query is in the head of the list, even if there
is no other query in the list to try.
Use the same condition for both decisions, and after starting the next
query, jump to the "detach_query" label instead of "next_lookup",
because there is no need to start the next lookup after we just started
a query in the current lookup.
The clang-format-15 has new option InsertBraces that could add missing
branches around single line statements. Use that to our advantage
without switching to not-yet-released LLVM version to add missing braces
in couple of places.
Commit 4ca74eee49 update the zone grammar
such that the zone statement is printed with the valid options per
zone type.
This commit is a follow-up, putting back the ZONE heading and adding
a note that these zone statements may also be put inside the view
statement.
It is tricky to actually print the zone statements inside
the view statement, and so we decided that we would add a note to say
that this is possible.
The isc_timer_create() function was a bit conflated. It could have been
used to create a timer and start it at the same time. As there was a
single place where this was done before (see the previous commit for
nta.c), this was cleaned up and the isc_timer_create() function was
changed to only create new timer.
Replace :manpage: with :iscman: to generate internal hyperlinks. That
way reader can use links even when offline, and jumps to man pages
for the same version.
Formerly HTML version of man pages did not have links in See Also
section because :manpage: role in Sphinx can generate only external
hyperlinks - and we do not have that enabled.
Enabling the Sphinx :manpage: linking could reliably create hyperlinks
only to external URLs, but that would take users to another version
of docs.
Generated by:
find bin -name '*.rst' | xargs sed -i -e 's/:manpage:`\([^(]\+\)(\([0-9]\))`/:iscman:`\1(\2) <\1>`/g'
+ hand-edit to revert change for mmencode reference which is
not provided in our source tree.
Use the new role :iscman: to replace all occurences or ``binary``
with :iscman:`binary`, creating a hyperlink to the manual page.
Generated using:
find bin -name *.rst | xargs fgrep --files-with-matches '.. iscman' | xargs -I{} -n1 basename {} .rst > /tmp/progs
for PROG in $(cat /tmp/progs); do find -name '*.rst' | xargs sed -i -e "s/\`\`$PROG\`\`/:iscman:\`$PROG\`/g"; done
Additional hand-edits were done mainly around filter-aaaa and
filter-a which are program names and and option names at the
same time. Couple more edits was neede to fix .rst syntax broken by
automatic replacement.
Sphinx has it's own :program: syntax for refering to program names.
Use it for self-references in manual pages. These self-references are
not clickable and not as eye-cathing as links, which is a good thing.
There is no point in attracting attention to ``dig`` several times on a
single page dedicated to dig itself.
Substituted automatically using:
find bin -name *.rst | xargs fgrep --files-with-matches '.. program' | xargs -n1 bash /tmp/repl.sh
With /tmp/repl.sh being:
BASE=$(basename "$1" .rst)
sed -i -e "s/\`\`$BASE\`\`/:program:\`$BASE\`/g" "$1"
The new directive and role "iscman" allow to tag & reference man pages in
our source tree. Essentially it is just namespacing for ISC man pages,
but it comes with couple benefits.
Differences from .. _man_program label we formerly used:
- Does not expand :ref:`man_program` into full text of the page header.
- Generates index entry with category "manual page".
- Rendering style is closer to ubiquitous to the one produced
by ``named`` syntax.
Differences from Sphinx built-in :manpage: role:
- Supports all builders with support for cross-references.
- Generates internal links (unlike :manpage: which generates external
URLs).
- Checks that target exists withing our source tree.
Side-effect of hyperlinking is that typos in program and option names
are now detected by Sphinx.
Candidate -options were detected using:
find -name *.rst | xargs grep '``-[^`]'
and then modified from ``-o`` to :option:`-o` using regex
s/``\(-[^`]\+\)``/:option:`\1`/
+ manual modifications where necessary.
Non-hyphenated options were detected by looking at context around
program names:
find bin -name *.rst | xargs -I{} -n1 basename {} .rst | sort -u
and grepping for program name with trailing whitespace.
Stand-alone program names like ``named`` are not hyperlinked in this
commit.
The markup allows referencing individual options, and also makes them
more legible (no more thin red text on gray background).
Most of the work was done using regexes:
s/^``-\(.*\)``$/.. option:: -\1\r/
s/^``+\(.*\)``$/.. option:: +\1\r/
on bin/**/*.rst files along with visual inspection and hand-edits,
mostly for positional arguments.
Regex for rndc.rst:
s/^``\(.*\)``/.. option:: \1\r/
+ hand edits to remove extra asterisk and whitespace here and there.
Since pytest itself skips tests using dnspython if the latter is not
available, also using Automake conditionals for silently skipping
pytest-based tests requiring dnspython is redundant and hides
information. Allow all pytest-based tests requiring dnspython to be run
whenever pytest itself is available, in order to ensure test skipping is
done in a uniform manner.
Note that the above reasoning only applies to pytest-based tests, so
similar adjustments were not made for shell-based tests using Python
scripts that require dnspython ("chain", "cookie", "dnssec", "qmin").
The ability to conveniently mark tests which should only be run when the
CI_ENABLE_ALL_TESTS environment variable is set seems to be useful on a
general level and therefore it should not be limited to the "timeouts"
system test, where it is currently used.
pytest documentation [1] suggests to reuse commonly used test markers by
putting them all in a single Python module which then has to be imported
by test files that want to use the markers defined therein. Follow that
advice by creating a new bin/tests/system/pytest_custom_markers.py
Python module containing the relevant marker definitions.
Note that "import pytest_custom_markers" works from a test-specific
subdirectory because pytest modifies sys.path so that it contains the
paths to all parent directories containing a conftest.py file (and
bin/tests/system/ is one). PyLint does not like that, though, so add a
relevant PyLint suppression.
The above changes make bin/tests/system/timeouts/conftest.py redundant,
so remove it.
[1] https://docs.pytest.org/en/7.0.x/how-to/skipping.html#id1
Ensure all "import dns.*" statements are always placed after
pytest.importorskip('dns') calls, in order to allow the latter to
fulfill their purpose. Explicitly import all dnspython modules used by
each dnspython-based test to avoid relying on nested imports. Replace
function-scoped imports with global imports to reduce code duplication.
The intended purpose of the @pytest.mark.dnspython{,2} decorators was to
cause dnspython-based tests to be skipped if dnspython is not available
(or not recent enough). However, a number of system tests employing
those decorators contain global "import dns.resolver" statements which
trigger ImportError exceptions during test initialization if dnspython
is not available. In other words, the @pytest.mark.dnspython{,2}
decorators serve no useful purpose.
Currently, whenever a Python-based test requires dnspython, that
requirement applies to all tests in a given *.py file. Given that,
employ global pytest.importorskip() calls to ensure dnspython-based
parts of various system tests are skipped when dnspython is not
available. Remove all occurrences of the @pytest.mark.dnspython{,2}
decorators (and all associated code) to prevent confusion.
The intended purpose of the @pytest.mark.requests decorator was to cause
Python-based parts of the "statschannel" system test to be skipped if
the requests Python module is not available. However, both
tests-json.py and tests-xml.py contain a global "import requests"
statement which triggers ImportError exceptions during test
initialization if the requests module is not available. In other words,
the @pytest.mark.requests decorator serves no useful purpose.
Since all tests in both tests-json.py and tests-xml.py depend on the
requests Python module, employ pytest.importorskip() to ensure the
Python-based parts of the "statschannel" system test are skipped when
the requests module is not available. Remove all occurrences of the
@pytest.mark.requests decorator (and all associated code) to prevent
confusion.
All tests in bin/tests/system/statschannel/tests-xml.py require libxml2
support to be enabled in BIND 9 at build-time. Instead of applying the
same pytest.mark.skipif() decorator to every test in that file, set the
'pytestmark' global accordingly in order to immediately skip all tests
in tests-xml.py if libxml2 support is not compiled in.
Remove all occurrences of the @pytest.mark.xml decorator (and all
associated code) from the "statschannel" system test as the
xml.etree.ElementTree module is a part of the Python standard library
since Python 2.5 (so checking whether it is available is redundant) and
checking for libxml2 support in the tested BIND 9 build is already
handled by setting the 'pytestmark' global accordingly.
