The rpz servfail-until-ready test assumed a particular policy zone always
finished loading last, which only holds when zone updates run in parallel;
on a single CPU (or with serialized offload) it could fail spuriously. It now
polls until RPZ reports ready instead.
Merge branch 'ondrej/fix-rpz-system-test-on-single-cpu' into 'main'
See merge request isc-projects/bind9!12251
RPZ is ready only once every policy zone has completed its first update,
and the zones do not finish in a fixed order, so whenever the updates
run serially (per-loop offload, or any single-CPU run): 'slow-rpz' zone
can finish before the others and the query still gets SERVFAIL. Poll
the query until it returns NOERROR instead.
Assisted-by: Claude:claude-opus-4-8
Move the fixed name storage out of the NCACHE branch so the name passed to dns_rrl() remains valid for cached NXDOMAIN responses.
Closes#6029
Merge branch '6029-fixedname-fix' into 'main'
See merge request isc-projects/bind9!12096
Tests interacting with mock ans servers had to hardcode their IP
addresses and open ans.run directly, while named instances already
had the NamedInstance abstraction with `.ip`, `.log` and the
watch_log_*() helpers. Factor the parts of NamedInstance that are
not named-specific into a ServerInstance base class and add an
AnsInstance subclass for ans servers, exposed through the `servers`
fixture and new ans1-ans11 convenience fixtures.
Assisted-by: Claude:claude-fable-5
Merge branch 'nicki/pytest-ans-instance' into 'main'
See merge request isc-projects/bind9!12241
Tests interacting with mock ans servers had to hardcode their IP
addresses and open ans.run directly, while named instances already
had the NamedInstance abstraction with `.ip`, `.log` and the
watch_log_*() helpers. Factor the parts of NamedInstance that are
not named-specific into a ServerInstance base class and add an
AnsInstance subclass for ans servers, exposed through the `servers`
fixture and new ans1-ans11 convenience fixtures.
Assisted-by: Claude:claude-fable-5
isc__nm_http_request()'s error path reloaded sock->h2->connect.cstream after client_send() had already detached and freed it on a submit failure, dereferencing NULL. The helper is only used by the DoH unit tests. Guard the cleanup path against the detached stream.
Closes#6160
Merge branch '6160-fix-latent-NULL-dereference-in-http2' into 'main'
See merge request isc-projects/bind9!12247
On a submit failure, client_send() nullifies sock->h2->connect.cstream
and frees the stream before returning the error. The error: label in
isc__nm_http_request() reloaded that pointer and dereferenced it
unconditionally, reading through a NULL stream. The function is only
used by the DoH unit tests -- production DoH client send goes through
isc__nm_http_send()/client_httpsend(), whose submit failure is reported
via the NULL-safe send callback -- so this is a latent defect in the
test helper rather than a reachable named crash.
Skip the read callback when the stream has already been detached and
let the caller report the failure from the error result it receives.
Assisted-by: Claude:claude-opus-4-8
It was possible to use a maliciously crafted authoritative
zone to make :iscman:`named` resolver synthesize a ``DNAME``
"alias" that should have been rejected by the configured
:any:`deny-answer-aliases` option. This has been fixed.
Closes#5930
Merge branch '5930-deny-answer-aliases-and-cached-dname-buf-fix' into 'main'
See merge request isc-projects/bind9!12044
When using a cached DNAME to resolve a name, make sure to consult
the denied answers lists, otherwise it is possible to consutruct
a restricted alias by caching a DNAME that is a parent of the
denied alias. See the comments in the tests case from the previous
commit an example.
This new check exercises an attack against guarantees given by the
'deny-answer-aliases' configuration option by caching a DNAME
that is a parent of the restricted alias, and then "constructing"
the restricted alias from the cache.
The isc_mem_strndup() function had a single caller, the HTTP/2
request-path handling, which now uses isc_mem_allocate() and strlcpy()
directly. Remove the function from the libisc API.
Closes#6087
Merge branch '6087-remove-isc_mem_strndup' into 'main'
See merge request isc-projects/bind9!12240
The function had a single caller, the HTTP/2 request-path handling in
the network manager, and its semantics (strlen() of the source clamped
to the requested size) amounted to an obscured bounded string copy.
Replace the only use with a plain allocation and strlcpy(), and drop the
function.
The :iscman:`named` process, running as secondary DNS server,
configured to transfer a zone from a primary server using an
encrypted XoT transport in opportunistic TLS mode (i.e. without
peer certificate/hostname validation) could terminate unexpectedly
when the TLS ALPN negotiation with primary server was unsuccessful.
This has been fixed.
Closes#5957
Merge branch '5957-xot-xfrin_connect_done-bug-fix' into 'main'
See merge request isc-projects/bind9!12081
When the connect callback's result is ISC_R_SUCCESS and the callback
changes the result because of some condition, the 'xfr' should not
be detached, because it now belongs to the receive callback.
Detach the reference only if the callback's result is non-success.
Configure a zone transfer using XoT (with opportunistic TLS) from
a non-DoT port, which does not provide ALPN "dot" (in this case
it will try to connect to a DoH port). This is expected to fail,
but the client should handle the error gracefully and not to crash.
A buffer used to dump a DNS name in the delegdb dump flow was using the
wrong size: it was using `DNS_NAME_MAXWIRE` which is the actual max
length of a DNS name on the wire instead of using `DNS_NAME_FORMATSIZE`
which is the maximum length of a textual representation of a DNS name
(which can be way longer than `DNS_NAME_MAXWIRE` if using the master
file escape sequence format) plus 1 (end of string byte). This could
lead to a buffer overflow. This is now fixed.
Closes#6132
Merge branch '6132-delegdb-dump-overflow' into 'main'
See merge request isc-projects/bind9!12195
A buffer used to dump a DNS name in the delegdb dump flow was using the
wrong size: it was using `DNS_NAME_MAXWIRE` which is the actual max
length of a DNS name on the wire instead of using `DNS_NAME_FORMATSIZE`
which is the maximum length of a textual representation of a DNS name
(which can be way longer than `DNS_NAME_MAXWIRE` if using the master
file escape sequence format) plus 1 (end of string byte). This could
lead to a buffer overflow. This is now fixed.
Add a delegdb test which dump a database which contains a very long name
(using DNS master file format with escape sequence as defined per RFC
1035). This ensure that the delegdb uses large enough internal buffers
to load the names in DB and generate the dump. If this is not the case,
the test crashes on a build with address sanatizer enabled.
The README documents what the framework is; the cookbook documents how
to get common tasks done with it: iterating on a single test, adding a
new test directory, writing a regression reproducer, mocking a
misbehaving server with isctest.asyncserver, signing zones in
bootstrap(), and driving named via the NamedInstance fixtures. All
recipes are distilled from existing tests (cyclic_glue, dnssec_py,
dispatch) so they reflect the current canonical patterns.
Assisted-by: Claude:claude-fable-5
Merge branch 'nicki/systest-cookbook' into 'main'
See merge request isc-projects/bind9!12234
The README documents what the framework is; the cookbook documents how
to get common tasks done with it: iterating on a single test, adding a
new test directory, writing a regression reproducer, mocking a
misbehaving server with isctest.asyncserver, signing zones in
bootstrap(), and driving named via the NamedInstance fixtures. All
recipes are distilled from existing tests (cyclic_glue, dnssec_py,
dispatch) so they reflect the current canonical patterns.
Assisted-by: Claude:claude-fable-5
`CONTRIBUTING.md` documents several rules around how AI coding assistants should (and should not) be attributed in commit messages. Teach `dangerfile.py` to enforce them so that violations are caught at MR time.
Merge branch 'mnowak/danger-ai-trailer-checks' into 'main'
See merge request isc-projects/bind9!11969
CONTRIBUTING.md documents the Assisted-by trailer format as
Assisted-by: AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]
and excludes basic development tools (git, compilers, meson,
ninja, editors, clang-format, black, ruff) from the optional
tool list.
Walk every `Assisted-by:` line in each commit message and emit a
`warn()` when:
- the line does not match the documented `AGENT:VERSION` shape;
- the optional tool list contains basic-tool names.
The basic-tool list extends the CONTRIBUTING.md examples with
other formatters, generic linters, and build/test runners
commonly invoked from `.gitlab-ci.yml`. Specialized analysis
tools (coccinelle, clang-tidy, AFL, Coverity, cppcheck,
valgrind, sanitizers) are intentionally absent so they remain
allowed in the trailer.
Use `warn()` rather than `fail()` because the format is
human-written and overly strict matching would produce false
positives on edge cases.
Assisted-by: Claude:claude-opus-4-7
CONTRIBUTING.md states that AI agents must not add Signed-off-by
tags, since only humans can legally certify the Developer
Certificate of Origin. Mirror the existing LLM Co-Authored-By
check against the Signed-off-by trailer line so danger fails on
commits that violate the rule.
The shared alternation of known LLM agent names is factored out
into LLM_AGENT_NAMES_RE so adding a new tool only requires one
edit.
Assisted-by: Claude:claude-opus-4-7
CONTRIBUTING.md states that AI agents must not be listed as
co-authors and that contributors should use the `Assisted-by:`
trailer instead. Teach `dangerfile.py` to fail merge requests
whose commit messages include a `Co-Authored-By:` trailer naming
a known LLM (Claude, Codex, Mistral, Copilot, Gemini, Cursor,
Devin, Aider, Sourcegraph, CodeWhisperer).
For any other `Co-Authored-By:` trailer, emit an info-level
`message()` that includes the full trailer line so reviewers can
confirm the named co-author is a human contributor and not an
unrecognised AI tool.
Assisted-by: Claude:claude-opus-4-7
When using GeoIP2 ACLs (see :any:`acl`), :iscman:`named` could
incorrectly match a name using a sub-string instead of the full
name match. This has been fixed.
Closes#6019
Merge branch '6019-geoip2-string-match-buf-fix' into 'main'
See merge request isc-projects/bind9!12092
The geoip2.c:match_string() function can incorrectly return 'true'
when matching strings of different lengths (i.e. it matches a
substring). Return 'false' when the lengths of the matched strings
are different.
The README predated the meson migration and most of the pytest runner
features. Document building the test dependencies with meson and drop
the make-based instructions, the Makefile.am registration step, and the
stale -T flag list. Describe the jinja2 templating, bootstrap(), the
conftest fixtures, the pytest marks, and recommend node IDs and
parametrization over -k matching. Fix the directory naming rule, which
switched from hyphens to underscores.
Also declare pytest and pytest-xdist as required dependencies: the
runner's pytest.ini uses --dist=loadscope unconditionally, so pytest
without pytest-xdist cannot even start.
Related #3810
Assisted-by: Claude:claude-fable-5
Merge branch 'nicki/systest-readme-refresh' into 'main'
See merge request isc-projects/bind9!12232
The README predated the meson migration and most of the pytest runner
features. Document building the test dependencies with meson and drop
the make-based instructions, the Makefile.am registration step, and the
stale -T flag list. Describe the jinja2 templating, bootstrap(), the
conftest fixtures, the pytest marks, and recommend node IDs and
parametrization over -k matching. Fix the directory naming rule, which
switched from hyphens to underscores.
Also declare pytest and pytest-xdist as required dependencies: the
runner's pytest.ini uses --dist=loadscope unconditionally, so pytest
without pytest-xdist cannot even start.
Related #3810
Assisted-by: Claude:claude-fable-5
The :any:`http-listener-clients` and :any:`http-streams-per-connection`
configuration options could be truncated to smaller values (or to ``0``,
which means unlimited) when very big configuration values were used, which
exceeded ``65535``. As a note - it is very unlikely that such big values
are used in production, and the default values for the affected options
are ``300`` and ``100``, correspondingly. This has been fixed.
Closes#6021
Merge branch '6021-doh-quota-type-truncation-fix' into 'main'
See merge request isc-projects/bind9!12085
The 'named_g_http_listener_clients' and 'named_g_http_streams_per_conn'
global variables are defined as 'in_port_t', which is usually 16 bits,
but both the readers and the writers of those variables use 'uint32_t'
as the target/source, which can result in truncation.
Use correct types.
When a Dynamic Update is received that removes the ``DNSKEY`` (or ``CDNSKEY``,
or ``CDS``) RRset, remove all records except the ones that are in use
for signing for the zone.
Closes#6045
Merge branch '6045-dns-update-delete-in-use-dnskey-any' into 'main'
See merge request isc-projects/bind9!12166
When a Dynamic Update is received that removes the DNSKEY (or CDNSKEY,
or CDS) RRset, remove all records except the ones that are in use
for signing for the zone (with dnssec-policy).
Suggested changes to the CVE checklist, submitted for your approval (or rejection). Commits in order of decreasing priority:
* 81bd4025c0e4ac407d5a8c7e0c414e2ed20f543f: Update Zulip link for new bind-incidents channel
* 65caa4912b25058cdeaef35094985424f2b2478f: Explicit sub-steps for assessing -S and EOL
* 1dc0d656edc1f6b5586da3ec1d416e5223c5504a: Clearer separation between CVE and CVSS+CWE steps
* 5ebb9ac33fda59adec6b9019bd211e96edebca49: EVN texts: Clarify what is done, when, and using which tool
* 29a79c21cbfe3477cd706861eeb12b53c98e2091: Use reference links for Earliest Notification and printing-press, to make the table less cumbersome to edit
* fd1b6d98c3cef2f6d212a5988b3726d933c84d27: Add TOC and sections for better (?) navigation
* 803b1d2aff04cf88ab860b42e3463c57fbf17f6d: Remove a few words to make the table narrower
* e68a1c84bd41a769ea8d96b7542cd7c062dd4af1: Clearer links to instructions, at top (bikeshed-y)
* e75b65072fe6eb271667659eac79817da1dd6515: Typo fix to the above
Merge branch 'bscott-cve-quicklist-tweaks' into 'main'
See merge request isc-projects/bind9!12221
"Assigning CVE" and "Assigning CVSS+CWE" are really two different
steps. CVE is bookeeping; we just request the ID and type it in.
CVSS and CWE require a judgement determination, and often involve
discussion. At the same time, sometimes we forget to put the CVE ID
in right away. Since we already have a separate step for CVE
assignment, let's put "update the issue with the CVE ID" in that step,
too. Then the second step can be entirely about CVSS+CWE. Same
number of steps, just clearer separation of what the steps are about.
Doubling system-test parallelism and removing the `@pytest.mark.flaky` markers exposed a set of timing-sensitive failures across CI. This branch keeps only the fixes for failures that are test-robustness bugs. Other - frequent - failures often guarded by `@pytest.mark.flaky` have their separate MRs already.
Merge branch 'mnowak/system-test-stability-fixes' into 'main'
See merge request isc-projects/bind9!12171
While a response-policy zone is being (re)loaded it can briefly answer
with no SOA (SERVFAIL/REFUSED), which `dig +short` renders as empty
output. get_sn() aborted the whole tests.sh on the first such miss,
defeating the retry_quiet() loop in ck_soa() that is meant to wait for
the reload to settle. Return failure instead so the check is retried.
Assisted-by: Claude:claude-opus-4-8
