diff --git a/doc/arm/changelog.rst b/doc/arm/changelog.rst index b08a03433c..2188280886 100644 --- a/doc/arm/changelog.rst +++ b/doc/arm/changelog.rst @@ -18,6 +18,8 @@ Changelog development. Regular users should refer to :ref:`Release Notes ` for changes relevant to them. +.. include:: ../changelog/changelog-9.20.15.rst +.. include:: ../changelog/changelog-9.20.14.rst .. include:: ../changelog/changelog-9.20.13.rst .. include:: ../changelog/changelog-9.20.12.rst .. include:: ../changelog/changelog-9.20.11.rst diff --git a/doc/arm/notes.rst b/doc/arm/notes.rst index 65ea30de4f..77ce5df00c 100644 --- a/doc/arm/notes.rst +++ b/doc/arm/notes.rst @@ -45,6 +45,8 @@ The list of known issues affecting the latest version in the 9.20 branch can be found at https://gitlab.isc.org/isc-projects/bind9/-/wikis/Known-Issues-in-BIND-9.20 +.. include:: ../notes/notes-9.20.15.rst +.. include:: ../notes/notes-9.20.14.rst .. include:: ../notes/notes-9.20.13.rst .. include:: ../notes/notes-9.20.12.rst .. include:: ../notes/notes-9.20.11.rst diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst index 3fc92abc40..130fee53bc 100644 --- a/doc/arm/reference.rst +++ b/doc/arm/reference.rst @@ -2229,6 +2229,8 @@ Boolean Options autodetection of DNS COOKIE support to determine when to retry a request over TCP. + For DNAME lookups the default is ``yes`` and it is enforced. Servers + serving DNAME must correctly support DNS over TCP. .. note:: If a UDP response is signed using TSIG, :iscman:`named` accepts it even if diff --git a/doc/changelog/changelog-9.20.14.rst b/doc/changelog/changelog-9.20.14.rst new file mode 100644 index 0000000000..ac07d13519 --- /dev/null +++ b/doc/changelog/changelog-9.20.14.rst @@ -0,0 +1,18 @@ +.. 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. + +BIND 9.20.14 +------------ + +.. note:: + + The BIND 9.20.14 release was withdrawn after the discovery of a + regression in a security fix in it during pre-release testing. diff --git a/doc/changelog/changelog-9.20.15.rst b/doc/changelog/changelog-9.20.15.rst new file mode 100644 index 0000000000..f67e400f5c --- /dev/null +++ b/doc/changelog/changelog-9.20.15.rst @@ -0,0 +1,133 @@ +.. 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. + +BIND 9.20.15 +------------ + +Security Fixes +~~~~~~~~~~~~~~ + +- [CVE-2025-8677] DNSSEC validation fails if matching but invalid DNSKEY + is found. ``0d676bf9f23`` + + Previously, if a matching but cryptographically invalid key was + encountered during DNSSEC validation, the key was skipped and not + counted towards validation failures. :iscman:`named` now treats such + DNSSEC keys as hard failures and the DNSSEC validation fails + immediately, instead of continuing with the next DNSKEYs in the RRset. + + ISC would like to thank Zuyao Xu and Xiang Li from the All-in-One + Security and Privacy Laboratory at Nankai University for bringing this + vulnerability to our attention. :gl:`#5343` + +- [CVE-2025-40778] Address various spoofing attacks. ``23de94fd236`` + + Previously, several issues could be exploited to poison a DNS cache + with spoofed records for zones which were not DNSSEC-signed or if the + resolver was configured to not do DNSSEC validation. These issues were + assigned CVE-2025-40778 and have now been fixed. + + As an additional layer of protection, :iscman:`named` no longer + accepts DNAME records or extraneous NS records in the AUTHORITY + section unless these are received via spoofing-resistant transport + (TCP, UDP with DNS cookies, TSIG, or SIG(0)). + + ISC would like to thank Yuxiao Wu, Yunyi Zhang, Baojun Liu, and Haixin + Duan from Tsinghua University for bringing this vulnerability to our + attention. :gl:`#5414` + +- [CVE-2025-40780] Cache-poisoning due to weak pseudo-random number + generator. ``34af35c2df8`` + + It was discovered during research for an upcoming academic paper that + a xoshiro128\*\* internal state can be recovered by an external 3rd + party, allowing the prediction of UDP ports and DNS IDs in outgoing + queries. This could lead to an attacker spoofing the DNS answers with + great efficiency and poisoning the DNS cache. + + The internal random generator has been changed to a cryptographically + secure pseudo-random generator. + + ISC would like to thank Prof. Amit Klein and Omer Ben Simhon from + Hebrew University of Jerusalem for bringing this vulnerability to our + attention. :gl:`#5484` + +New Features +~~~~~~~~~~~~ + +- Add dnssec-policy keys configuration check to named-checkconf. + ``1f5a0405f72`` + + A new option `-k` is added to `named-checkconf` that allows checking + the `dnssec-policy` `keys` configuration against the configured key + stores. If the found key files are not in sync with the given + `dnssec-policy`, the check will fail. + + This is useful to run before migrating to `dnssec-policy`. :gl:`#5486` + :gl:`!11011` + +Feature Changes +~~~~~~~~~~~~~~~ + +- Minor refactor of dst code. ``c6acbaa020b`` + + Convert the defines to enums. Initialize the tags more explicitly and + less ugly. :gl:`!11038` + +Bug Fixes +~~~~~~~~~ + +- Use signer name when disabling DNSSEC algorithms. ``986816baa74`` + + ``disable-algorithms`` could cause DNSSEC validation failures when the + parent zone was signed with the algorithms that were being disabled + for the child zone. This has been fixed; `disable-algorithms` now + works on a whole-of-zone basis. + + If the zone's name is at or below the ``disable-algorithms`` name the + algorithm is disabled for that zone, using deepest match when there + are multiple ``disable-algorithms`` clauses. :gl:`#5165` :gl:`!11014` + +- Rndc sign during ZSK rollover will now replace signatures. + ``d2f551140cd`` + + When performing a ZSK rollover, if the new DNSKEY is omnipresent, the + :option:`rndc sign` command now signs the zone completely with the + successor key, replacing all zone signatures from the predecessor key + with new ones. :gl:`#5483` :gl:`!11017` + +- Missing DNSSEC information when CD bit is set in query. + ``968a6be41fb`` + + The RRSIGs for glue records were not being cached correctly for CD=1 + queries. This has been fixed. :gl:`#5502` :gl:`!10956` + +- Preserve cache when reload fails and reload the server again. + ``975aeda10b4`` + + Fixes an issue where failing to reconfigure/reload the server would + prevent to preserved the views caches on the subsequent server + reconfiguration/reload. :gl:`#5523` :gl:`!10988` + +- Check plugin config before registering. ``e2260b80702`` + + In `named_config_parsefile()`, when checking the validity of + `named.conf`, the checking of plugin correctness was deliberately + postponed until the plugin is loaded and registered. However, the + checking was never actually done: the `plugin_register()` + implementation was called, but `plugin_check()` was not. + + `ns_plugin_register()` (used by `named`) now calls the check function + before the register function, and aborts if either one fails. + `ns_plugin_check()` (used by `named-checkconf`) calls only the check + function. :gl:`!11032` + + diff --git a/doc/notes/notes-9.20.14.rst b/doc/notes/notes-9.20.14.rst new file mode 100644 index 0000000000..af7366caff --- /dev/null +++ b/doc/notes/notes-9.20.14.rst @@ -0,0 +1,18 @@ +.. 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. + +Notes for BIND 9.20.14 +---------------------- + +.. note:: + + The BIND 9.20.14 release was withdrawn after the discovery of a + regression in a security fix in it during pre-release testing. diff --git a/doc/notes/notes-9.20.15.rst b/doc/notes/notes-9.20.15.rst new file mode 100644 index 0000000000..7c27dce3d2 --- /dev/null +++ b/doc/notes/notes-9.20.15.rst @@ -0,0 +1,108 @@ +.. 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. + +Notes for BIND 9.20.15 +---------------------- + +Security Fixes +~~~~~~~~~~~~~~ + +- DNSSEC validation fails if matching but invalid DNSKEY is found. + :cve:`2025-8677` + + Previously, if a matching but cryptographically invalid key was + encountered during DNSSEC validation, the key was skipped and not + counted towards validation failures. :iscman:`named` now treats such + DNSSEC keys as hard failures and the DNSSEC validation fails + immediately, instead of continuing with the next DNSKEYs in the RRset. + + ISC would like to thank Zuyao Xu and Xiang Li from the All-in-One + Security and Privacy Laboratory at Nankai University for bringing this + vulnerability to our attention. :gl:`#5343` + +- Address various spoofing attacks. :cve:`2025-40778` + + Previously, several issues could be exploited to poison a DNS cache + with spoofed records for zones which were not DNSSEC-signed or if the + resolver was configured to not do DNSSEC validation. These issues were + assigned CVE-2025-40778 and have now been fixed. + + As an additional layer of protection, :iscman:`named` no longer + accepts DNAME records or extraneous NS records in the AUTHORITY + section unless these are received via spoofing-resistant transport + (TCP, UDP with DNS cookies, TSIG, or SIG(0)). + + ISC would like to thank Yuxiao Wu, Yunyi Zhang, Baojun Liu, and Haixin + Duan from Tsinghua University for bringing this vulnerability to our + attention. :gl:`#5414` + +- Cache-poisoning due to weak pseudo-random number generator. + :cve:`2025-40780` + + It was discovered during research for an upcoming academic paper that + a xoshiro128\*\* internal state can be recovered by an external 3rd + party, allowing the prediction of UDP ports and DNS IDs in outgoing + queries. This could lead to an attacker spoofing the DNS answers with + great efficiency and poisoning the DNS cache. + + The internal random generator has been changed to a cryptographically + secure pseudo-random generator. + + ISC would like to thank Prof. Amit Klein and Omer Ben Simhon from + Hebrew University of Jerusalem for bringing this vulnerability to our + attention. :gl:`#5484` + +New Features +~~~~~~~~~~~~ + +- Add :any:`dnssec-policy` keys configuration check to + :iscman:`named-checkconf`. + + A new option :option:`-k ` was added to + :iscman:`named-checkconf` that allows checking the + :any:`dnssec-policy` :any:`keys` configuration against the configured + key stores. If the found key files are not in sync with the given + :any:`dnssec-policy`, the check will fail. + + This is useful to run before migrating to :any:`dnssec-policy`. + :gl:`#5486` + +Bug Fixes +~~~~~~~~~ + +- Missing DNSSEC information when CD bit is set in query. + + The RRSIGs for glue records were not being cached correctly for CD=1 + queries. This has been fixed. :gl:`#5502` + +- :option:`rndc sign` during ZSK rollover will now replace signatures. + + When performing a ZSK rollover, if the new DNSKEY is omnipresent, the + :option:`rndc sign` command now signs the zone completely with the + successor key, replacing all zone signatures from the predecessor key + with new ones. :gl:`#5483` + +- Use signer name when disabling DNSSEC algorithms. + + :any:`disable-algorithms` could cause DNSSEC validation failures when + the parent zone was signed with the algorithms that were being + disabled for the child zone. This has been fixed; + :any:`disable-algorithms` now works on a whole-of-zone basis. + + If the zone's name is at or below the :any:`disable-algorithms` name + the algorithm is disabled for that zone, using deepest match when + there are multiple :any:`disable-algorithms` clauses. :gl:`#5165` + +- Preserve cache when reload fails and reload the server again. + + This fixes an issue where failing to reconfigure/reload the server + would fail to preserve the views' caches for subsequent server + reconfigurations/reloads. :gl:`#5523` diff --git a/lib/dns/include/dns/message.h b/lib/dns/include/dns/message.h index c105d0a8c0..a3ad4ba2ad 100644 --- a/lib/dns/include/dns/message.h +++ b/lib/dns/include/dns/message.h @@ -262,6 +262,7 @@ struct dns_message { unsigned int rdclass_set : 1; /* 14 */ unsigned int fuzzing : 1; /* 15 */ unsigned int free_pools : 1; /* 16 */ + unsigned int has_dname : 1; /* 17 */ unsigned int : 0; unsigned int opt_reserved; @@ -1482,4 +1483,11 @@ dns_message_createpools(isc_mem_t *mctx, isc_mempool_t **namepoolp, void dns_message_destroypools(isc_mempool_t **namepoolp, isc_mempool_t **rdspoolp); +bool +dns_message_hasdname(dns_message_t *msg); +/*%< + * Return whether a DNAME was detected in the ANSWER section of a QUERY + * message when it was parsed. + */ + ISC_LANG_ENDDECLS diff --git a/lib/dns/message.c b/lib/dns/message.c index 8b9fd19bd7..19bb6a8530 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -463,6 +463,7 @@ msginit(dns_message_t *m) { m->cc_bad = 0; m->tkey = 0; m->rdclass_set = 0; + m->has_dname = 0; m->querytsig = NULL; m->indent.string = "\t"; m->indent.count = 0; @@ -1644,6 +1645,11 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx, */ msg->tsigname->attributes.nocompress = true; free_name = false; + } else if (rdtype == dns_rdatatype_dname && + sectionid == DNS_SECTION_ANSWER && + msg->opcode == dns_opcode_query) + { + msg->has_dname = 1; } rdataset = NULL; @@ -5145,3 +5151,9 @@ dns_message_destroypools(isc_mempool_t **namepoolp, isc_mempool_t **rdspoolp) { isc_mempool_destroy(rdspoolp); isc_mempool_destroy(namepoolp); } + +bool +dns_message_hasdname(dns_message_t *msg) { + REQUIRE(DNS_MESSAGE_VALID(msg)); + return msg->has_dname; +} diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index d64f71486a..037bb2e0c8 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -815,6 +815,7 @@ typedef struct respctx { bool get_nameservers; /* get a new NS rrset at * zone cut? */ bool resend; /* resend this query? */ + bool secured; /* message was signed or had a valid cookie */ bool nextitem; /* invalid response; keep * listening for the correct one */ bool truncated; /* response was truncated */ @@ -6680,7 +6681,8 @@ mark_related(dns_name_t *name, dns_rdataset_t *rdataset, bool external, * locally served zone. */ static inline bool -name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { +name_external(const dns_name_t *name, dns_rdatatype_t type, respctx_t *rctx) { + fetchctx_t *fctx = rctx->fctx; isc_result_t result; dns_forwarders_t *forwarders = NULL; dns_name_t *apex = NULL; @@ -6690,7 +6692,7 @@ name_external(const dns_name_t *name, dns_rdatatype_t type, fetchctx_t *fctx) { dns_namereln_t rel; apex = (ISDUALSTACK(fctx->addrinfo) || !ISFORWARDER(fctx->addrinfo)) - ? fctx->domain + ? rctx->ns_name != NULL ? rctx->ns_name : fctx->domain : fctx->fwdname; /* @@ -6794,7 +6796,7 @@ check_section(void *arg, const dns_name_t *addname, dns_rdatatype_t type, result = dns_message_findname(rctx->query->rmessage, section, addname, dns_rdatatype_any, 0, &name, NULL); if (result == ISC_R_SUCCESS) { - external = name_external(name, type, fctx); + external = name_external(name, type, rctx); if (type == dns_rdatatype_a) { for (rdataset = ISC_LIST_HEAD(name->list); rdataset != NULL; @@ -7550,6 +7552,119 @@ cleanup: isc_mem_putanddetach(&rctx->mctx, rctx, sizeof(*rctx)); } +static isc_result_t +rctx_cookiecheck(respctx_t *rctx) { + fetchctx_t *fctx = rctx->fctx; + resquery_t *query = rctx->query; + + /* + * If the message was secured or TCP is already in the + * retry flags, no need to continue. + */ + if (rctx->secured || (rctx->retryopts & DNS_FETCHOPT_TCP) != 0) { + return ISC_R_SUCCESS; + } + + /* + * If we've had a cookie from the same server previously, + * retry with TCP. This may be a misconfigured anycast server + * or an attempt to send a spoofed response. + */ + if (dns_adb_getcookie(query->addrinfo, NULL, 0) > CLIENT_COOKIE_SIZE) { + if (isc_log_wouldlog(dns_lctx, ISC_LOG_INFO)) { + char addrbuf[ISC_SOCKADDR_FORMATSIZE]; + isc_sockaddr_format(&query->addrinfo->sockaddr, addrbuf, + sizeof(addrbuf)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, + "missing expected cookie from %s", + addrbuf); + } + rctx->retryopts |= DNS_FETCHOPT_TCP; + rctx->resend = true; + rctx_done(rctx, ISC_R_SUCCESS); + return ISC_R_COMPLETE; + } + + /* + * Retry over TCP if require-cookie is true. + */ + if (fctx->res->view->peers != NULL) { + isc_result_t result; + dns_peer_t *peer = NULL; + bool required = false; + isc_netaddr_t netaddr; + + isc_netaddr_fromsockaddr(&netaddr, &query->addrinfo->sockaddr); + result = dns_peerlist_peerbyaddr(fctx->res->view->peers, + &netaddr, &peer); + if (result == ISC_R_SUCCESS) { + dns_peer_getrequirecookie(peer, &required); + } + if (!required) { + return ISC_R_SUCCESS; + } + + if (isc_log_wouldlog(dns_lctx, ISC_LOG_INFO)) { + char addrbuf[ISC_SOCKADDR_FORMATSIZE]; + isc_sockaddr_format(&query->addrinfo->sockaddr, addrbuf, + sizeof(addrbuf)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, + DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, + "missing required cookie from %s", + addrbuf); + } + + rctx->retryopts |= DNS_FETCHOPT_TCP; + rctx->resend = true; + rctx_done(rctx, ISC_R_SUCCESS); + return ISC_R_COMPLETE; + } + + return ISC_R_SUCCESS; +} + +static bool +rctx_need_tcpretry(respctx_t *rctx) { + resquery_t *query = rctx->query; + if ((rctx->retryopts & DNS_FETCHOPT_TCP) != 0) { + /* TCP is already in the retry flags */ + return false; + } + + /* + * If the message was secured, no need to continue. + */ + if (rctx->secured) { + return false; + } + + /* + * Currently the only extra reason why we might need to + * retry a UDP response over TCP is a DNAME in the message. + */ + if (dns_message_hasdname(query->rmessage)) { + return true; + } + + return false; +} + +static isc_result_t +rctx_tcpretry(respctx_t *rctx) { + /* + * Do we need to retry a UDP response over TCP? + */ + if (rctx_need_tcpretry(rctx)) { + rctx->retryopts |= DNS_FETCHOPT_TCP; + rctx->resend = true; + rctx_done(rctx, ISC_R_SUCCESS); + return ISC_R_COMPLETE; + } + + return ISC_R_SUCCESS; +} + static void resquery_response_continue(void *arg, isc_result_t result) { respctx_t *rctx = arg; @@ -7567,6 +7682,17 @@ resquery_response_continue(void *arg, isc_result_t result) { goto cleanup; } + /* + * Remember whether this message was signed or had a + * valid client cookie; if not, we may need to retry over + * TCP later. + */ + if (query->rmessage->cc_ok || query->rmessage->tsig != NULL || + query->rmessage->sig0 != NULL) + { + rctx->secured = true; + } + /* * The dispatcher should ensure we only get responses with QR * set. @@ -7574,75 +7700,24 @@ resquery_response_continue(void *arg, isc_result_t result) { INSIST((query->rmessage->flags & DNS_MESSAGEFLAG_QR) != 0); /* - * If we have had a server cookie and don't get one retry over - * TCP. This may be a misconfigured anycast server or an attempt - * to send a spoofed response. Additionally retry over TCP if - * require-cookie is true and we don't have a got client cookie. - * Skip if we have a valid TSIG. + * Check for cookie issues; if found, maybe retry over TCP. */ - if (dns_message_gettsig(query->rmessage, NULL) == NULL && - !query->rmessage->cc_ok && !query->rmessage->cc_bad && - (rctx->retryopts & DNS_FETCHOPT_TCP) == 0) - { - if (dns_adb_getcookie(query->addrinfo, NULL, 0) > - CLIENT_COOKIE_SIZE) - { - if (isc_log_wouldlog(dns_lctx, ISC_LOG_INFO)) { - char addrbuf[ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_format(&query->addrinfo->sockaddr, - addrbuf, sizeof(addrbuf)); - isc_log_write( - dns_lctx, DNS_LOGCATEGORY_RESOLVER, - DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, - "missing expected cookie " - "from %s", - addrbuf); - } - rctx->retryopts |= DNS_FETCHOPT_TCP; - rctx->resend = true; - rctx_done(rctx, result); - goto cleanup; - } else if (fctx->res->view->peers != NULL) { - dns_peer_t *peer = NULL; - isc_netaddr_t netaddr; - isc_netaddr_fromsockaddr(&netaddr, - &query->addrinfo->sockaddr); - result = dns_peerlist_peerbyaddr(fctx->res->view->peers, - &netaddr, &peer); - if (result == ISC_R_SUCCESS) { - bool required = false; - result = dns_peer_getrequirecookie(peer, - &required); - if (result == ISC_R_SUCCESS && required) { - if (isc_log_wouldlog(dns_lctx, - ISC_LOG_INFO)) - { - char addrbuf - [ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_format( - &query->addrinfo - ->sockaddr, - addrbuf, - sizeof(addrbuf)); - isc_log_write( - dns_lctx, - DNS_LOGCATEGORY_RESOLVER, - DNS_LOGMODULE_RESOLVER, - ISC_LOG_INFO, - "missing required " - "cookie " - "from %s", - addrbuf); - } - rctx->retryopts |= DNS_FETCHOPT_TCP; - rctx->resend = true; - rctx_done(rctx, result); - goto cleanup; - } - } - } + result = rctx_cookiecheck(rctx); + if (result == ISC_R_COMPLETE) { + goto cleanup; } + /* + * Check whether we need to retry over TCP for some other reason. + */ + result = rctx_tcpretry(rctx); + if (result == ISC_R_COMPLETE) { + goto cleanup; + } + + /* + * Check for EDNS issues. + */ rctx_edns(rctx); /* @@ -8374,8 +8449,8 @@ rctx_answer_positive(respctx_t *rctx) { } /* - * Cache records in the authority section, if - * there are any suitable for caching. + * Cache records in the authority section, if there are + * any suitable for caching. */ rctx_authority_positive(rctx); @@ -8447,7 +8522,7 @@ rctx_answer_scan(respctx_t *rctx) { /* * Don't accept DNAME from parent namespace. */ - if (name_external(name, dns_rdatatype_dname, fctx)) { + if (name_external(name, dns_rdatatype_dname, rctx)) { continue; } @@ -8748,14 +8823,14 @@ rctx_answer_dname(respctx_t *rctx) { /* * rctx_authority_positive(): - * Examine the records in the authority section (if there are any) for a - * positive answer. We expect the names for all rdatasets in this - * section to be subdomains of the domain being queried; any that are - * not are skipped. We expect to find only *one* owner name; any names - * after the first one processed are ignored. We expect to find only - * rdatasets of type NS, RRSIG, or SIG; all others are ignored. Whatever - * remains can be cached at trust level authauthority or additional - * (depending on whether the AA bit was set on the answer). + * If a positive answer was received over TCP or secured with a cookie + * or TSIG, examine the authority section. We expect names for all + * rdatasets in this section to be subdomains of the domain being queried; + * any that are not are skipped. We expect to find only *one* owner name; + * any names after the first one processed are ignored. We expect to find + * only rdatasets of type NS; all others are ignored. Whatever remains can + * be cached at trust level authauthority or additional (depending on + * whether the AA bit was set on the answer). */ static void rctx_authority_positive(respctx_t *rctx) { @@ -8763,6 +8838,11 @@ rctx_authority_positive(respctx_t *rctx) { bool done = false; isc_result_t result; + /* If it's spoofable, don't cache it. */ + if (!rctx->secured && (rctx->query->options & DNS_FETCHOPT_TCP) == 0) { + return; + } + result = dns_message_firstname(rctx->query->rmessage, DNS_SECTION_AUTHORITY); while (!done && result == ISC_R_SUCCESS) { @@ -8771,7 +8851,9 @@ rctx_authority_positive(respctx_t *rctx) { dns_message_currentname(rctx->query->rmessage, DNS_SECTION_AUTHORITY, &name); - if (!name_external(name, dns_rdatatype_ns, fctx)) { + if (!name_external(name, dns_rdatatype_ns, rctx) && + dns_name_issubdomain(fctx->name, name)) + { dns_rdataset_t *rdataset = NULL; /* diff --git a/lib/dns/validator.c b/lib/dns/validator.c index bcbe026f91..8ecc59f665 100644 --- a/lib/dns/validator.c +++ b/lib/dns/validator.c @@ -378,6 +378,12 @@ trynsec3: static void resume_answer_with_key_done(void *arg); +static bool +over_max_fails(dns_validator_t *val); + +static void +consume_validation_fail(dns_validator_t *val); + static void resume_answer_with_key(void *arg) { dns_validator_t *val = arg; @@ -386,6 +392,13 @@ resume_answer_with_key(void *arg) { isc_result_t result = select_signing_key(val, rdataset); if (result == ISC_R_SUCCESS) { val->keyset = &val->frdataset; + } else if (result != ISC_R_NOTFOUND) { + val->result = result; + if (over_max_fails(val)) { + INSIST(val->key == NULL); + val->result = ISC_R_QUOTA; + } + consume_validation_fail(val); } (void)validate_async_run(val, resume_answer_with_key_done); @@ -395,6 +408,16 @@ static void resume_answer_with_key_done(void *arg) { dns_validator_t *val = arg; + switch (val->result) { + case ISC_R_CANCELED: /* Validation was canceled */ + case ISC_R_SHUTTINGDOWN: /* Server shutting down */ + case ISC_R_QUOTA: /* Validation fails quota reached */ + dns_validator_cancel(val); + break; + default: + break; + } + resume_answer(val); } @@ -1070,6 +1093,8 @@ select_signing_key(dns_validator_t *val, dns_rdataset_t *rdataset) { goto done; } dst_key_free(&val->key); + } else { + break; } dns_rdata_reset(&rdata); result = dns_rdataset_next(rdataset); @@ -1309,6 +1334,7 @@ selfsigned_dnskey(dns_validator_t *val) { dns_name_t *name = val->name; isc_result_t result; isc_mem_t *mctx = val->view->mctx; + bool match = false; if (rdataset->type != dns_rdatatype_dnskey) { return DNS_R_NOKEYMATCH; @@ -1349,17 +1375,16 @@ selfsigned_dnskey(dns_validator_t *val) { /* * If the REVOKE bit is not set we have a - * theoretically self signed DNSKEY RRset. - * This will be verified later. + * theoretically self-signed DNSKEY RRset; + * this will be verified later. + * + * We don't return the answer yet, though, + * because we need to check the remaining keys + * and possbly remove them if they're revoked. */ if ((key.flags & DNS_KEYFLAG_REVOKE) == 0) { - return ISC_R_SUCCESS; - } - - result = dns_dnssec_keyfromrdata(name, &keyrdata, mctx, - &dstkey); - if (result != ISC_R_SUCCESS) { - continue; + match = true; + break; } /* @@ -1369,6 +1394,20 @@ selfsigned_dnskey(dns_validator_t *val) { if (DNS_TRUST_PENDING(rdataset->trust) && dns_view_istrusted(val->view, name, &key)) { + result = dns_dnssec_keyfromrdata( + name, &keyrdata, mctx, &dstkey); + if (result == DST_R_UNSUPPORTEDALG) { + /* don't count towards max fails */ + break; /* continue with next key */ + } else if (result != ISC_R_SUCCESS) { + consume_validation(val); + if (over_max_fails(val)) { + return ISC_R_QUOTA; + } + consume_validation_fail(val); + break; /* continue with next key */ + } + if (over_max_validations(val)) { dst_key_free(&dstkey); return ISC_R_QUOTA; @@ -1402,6 +1441,8 @@ selfsigned_dnskey(dns_validator_t *val) { } consume_validation_fail(val); } + + dst_key_free(&dstkey); } else if (rdataset->trust >= dns_trust_secure) { /* * We trust this RRset so if the key is @@ -1409,12 +1450,14 @@ selfsigned_dnskey(dns_validator_t *val) { */ dns_view_untrust(val->view, name, &key); } - - dst_key_free(&dstkey); } } - return DNS_R_NOKEYMATCH; + if (!match) { + return DNS_R_NOKEYMATCH; + } + + return ISC_R_SUCCESS; } /*% @@ -1599,7 +1642,7 @@ validate_answer_signing_key_done(void *arg); static void validate_answer_signing_key(void *arg) { dns_validator_t *val = arg; - isc_result_t result = ISC_R_NOTFOUND; + isc_result_t result; if (CANCELED(val) || CANCELING(val)) { val->result = ISC_R_CANCELED; @@ -1622,15 +1665,21 @@ validate_answer_signing_key(void *arg) { default: /* Select next signing key */ result = select_signing_key(val, val->keyset); + if (result == ISC_R_SUCCESS) { + INSIST(val->key != NULL); + } else if (result == ISC_R_NOTFOUND) { + INSIST(val->key == NULL); + } else { + val->result = result; + if (over_max_fails(val)) { + INSIST(val->key == NULL); + val->result = ISC_R_QUOTA; + } + consume_validation_fail(val); + } break; } - if (result == ISC_R_SUCCESS) { - INSIST(val->key != NULL); - } else { - INSIST(val->key == NULL); - } - (void)validate_async_run(val, validate_answer_signing_key_done); } @@ -1894,10 +1943,7 @@ check_signer(dns_validator_t *val, dns_rdata_t *keyrdata, uint16_t keyid, result = dns_dnssec_keyfromrdata( val->name, keyrdata, val->view->mctx, &dstkey); if (result != ISC_R_SUCCESS) { - /* - * This really shouldn't happen, but... - */ - continue; + return result; } } result = verify(val, dstkey, &rdata, sig.keyid); diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index 1ea03acfb0..c0769ec527 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -21,7 +21,6 @@ libisc_la_HEADERS = \ include/isc/dir.h \ include/isc/dnsstream.h \ include/isc/endian.h \ - include/isc/entropy.h \ include/isc/errno.h \ include/isc/error.h \ include/isc/file.h \ @@ -127,7 +126,6 @@ libisc_la_SOURCES = \ counter.c \ crc64.c \ dir.c \ - entropy.c \ errno.c \ errno2result.c \ errno2result.h \ @@ -165,7 +163,6 @@ libisc_la_SOURCES = \ net.c \ netaddr.c \ netscope.c \ - nonce.c \ openssl_shim.c \ openssl_shim.h \ os.c \ diff --git a/lib/isc/entropy.c b/lib/isc/entropy.c deleted file mode 100644 index a037960bd1..0000000000 --- a/lib/isc/entropy.c +++ /dev/null @@ -1,24 +0,0 @@ -/* - * 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. - */ - -#include -#include -#include -#include - -void -isc_entropy_get(void *buf, size_t buflen) { - int r = uv_random(NULL, NULL, buf, buflen, 0, NULL); - - UV_RUNTIME_CHECK(uv_random, r); -} diff --git a/lib/isc/hash.c b/lib/isc/hash.c index 387e2373cd..49b942ef5b 100644 --- a/lib/isc/hash.c +++ b/lib/isc/hash.c @@ -16,7 +16,6 @@ #include #include -#include #include /* IWYU pragma: keep */ #include #include @@ -35,7 +34,7 @@ isc__hash_initialize(void) { */ uint8_t key[16] = { 1 }; #if !FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION - isc_entropy_get(key, sizeof(key)); + isc_random_buf(key, sizeof(key)); #endif /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ STATIC_ASSERT(sizeof(key) >= sizeof(isc_hash_key), "sizeof(key) < sizeof(isc_hash_key)"); diff --git a/lib/isc/hashmap.c b/lib/isc/hashmap.c index d8c2bd58ae..87b1a3f21a 100644 --- a/lib/isc/hashmap.c +++ b/lib/isc/hashmap.c @@ -32,7 +32,6 @@ #include #include -#include #include #include #include diff --git a/lib/isc/include/isc/entropy.h b/lib/isc/include/isc/entropy.h deleted file mode 100644 index 4e2dc5f884..0000000000 --- a/lib/isc/include/isc/entropy.h +++ /dev/null @@ -1,35 +0,0 @@ -/* - * 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. - */ - -#pragma once - -#include - -#include - -/*! \file isc/entropy.h - * \brief Implements wrapper around CSPRNG cryptographic library calls - * for getting cryptographically secure pseudo-random numbers. - * - * Uses synchronous version of uv_random(). - */ - -ISC_LANG_BEGINDECLS - -void -isc_entropy_get(void *buf, size_t buflen); -/*!< - * \brief Get cryptographically-secure pseudo-random data. - */ - -ISC_LANG_ENDDECLS diff --git a/lib/isc/include/isc/nonce.h b/lib/isc/include/isc/nonce.h index b593e41445..10fb8b10a0 100644 --- a/lib/isc/include/isc/nonce.h +++ b/lib/isc/include/isc/nonce.h @@ -16,6 +16,7 @@ #include #include +#include /*! \file isc/nonce.h * \brief Provides a function for generating an arbitrarily long nonce. @@ -23,8 +24,10 @@ ISC_LANG_BEGINDECLS -void -isc_nonce_buf(void *buf, size_t buflen); +static inline void +isc_nonce_buf(void *buf, size_t buflen) { + isc_random_buf(buf, buflen); +} /*!< * Fill 'buf', up to 'buflen' bytes, with random data from the * crypto provider's random function. diff --git a/lib/isc/include/isc/random.h b/lib/isc/include/isc/random.h index 79f1f9de2c..78035fbe9a 100644 --- a/lib/isc/include/isc/random.h +++ b/lib/isc/include/isc/random.h @@ -20,25 +20,18 @@ #include /*! \file isc/random.h - * \brief Implements wrapper around a non-cryptographically secure + * \brief Implements wrapper around a cryptographically secure * pseudo-random number generator. * */ ISC_LANG_BEGINDECLS -uint8_t -isc_random8(void); -/*!< - * \brief Returns a single 8-bit random value. - */ - -uint16_t -isc_random16(void); -/*!< - * \brief Returns a single 16-bit random value. - */ - +#if HAVE_ARC4RANDOM && !defined(__linux__) +#define isc_random32() arc4random() +#define isc_random_buf(buf, buflen) arc4random_buf(buf, buflen) +#define isc_random_uniform(upper_bound) arc4random_uniform(upper_bound) +#else /* HAVE_ARC4RANDOM && !defined(__linux__) */ uint32_t isc_random32(void); /*!< @@ -68,4 +61,21 @@ isc_random_uniform(uint32_t upper_bound); * when upper_bound is UINT32_MAX/2. */ +#endif /* HAVE_ARC4RANDOM && !defined(__linux__) */ + +static inline uint8_t +isc_random8(void) { + return (uint8_t)isc_random32(); +} +/*!< + * \brief Returns a single 8-bit random value. + */ + +static inline uint16_t +isc_random16(void) { + return (uint16_t)isc_random32(); +} +/*!< + * \brief Returns a single 16-bit random value. + */ ISC_LANG_ENDDECLS diff --git a/lib/isc/nonce.c b/lib/isc/nonce.c deleted file mode 100644 index 316498a613..0000000000 --- a/lib/isc/nonce.c +++ /dev/null @@ -1,20 +0,0 @@ -/* - * 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. - */ - -#include -#include - -void -isc_nonce_buf(void *buf, size_t buflen) { - isc_entropy_get(buf, buflen); -} diff --git a/lib/isc/random.c b/lib/isc/random.c index 88efd21c4e..1367fd5a12 100644 --- a/lib/isc/random.c +++ b/lib/isc/random.c @@ -30,126 +30,55 @@ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ +#if !HAVE_ARC4RANDOM || defined(__linux__) + #include -#include -#include -#include +#include -#include +#include #include -#include #include -#include #include +#include -/* - * Written in 2018 by David Blackman and Sebastiano Vigna (vigna@acm.org) - * - * To the extent possible under law, the author has dedicated all - * copyright and related and neighboring rights to this software to the - * public domain worldwide. This software is distributed without any - * warranty. - * - * See . - */ +#define ISC_RANDOM_BUFSIZE (ISC_OS_CACHELINE_SIZE / sizeof(uint32_t)) -/* - * This is xoshiro128** 1.0, our 32-bit all-purpose, rock-solid generator. - * It has excellent (sub-ns) speed, a state size (128 bits) that is large - * enough for mild parallelism, and it passes all tests we are aware of. - * - * The state must be seeded so that it is not everywhere zero. - */ - -static thread_local bool initialized = false; -static thread_local uint32_t seed[4] = { 0 }; - -static uint32_t -rotl(const uint32_t x, int k) { - return (x << k) | (x >> (32 - k)); -} - -static uint32_t -next(void) { - uint32_t result_starstar, t; - - result_starstar = rotl(seed[0] * 5, 7) * 9; - t = seed[1] << 9; - - seed[2] ^= seed[0]; - seed[3] ^= seed[1]; - seed[1] ^= seed[2]; - seed[0] ^= seed[3]; - - seed[2] ^= t; - - seed[3] = rotl(seed[3], 11); - - return result_starstar; -} - -static void -isc__random_initialize(void) { - if (initialized) { - return; - } - -#if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION - /* - * A fixed seed helps with problem reproduction when fuzzing. It must be - * non-zero else xoshiro128starstar will generate only zeroes, and the - * first result needs to be non-zero as expected by random_test.c - */ - seed[0] = 1; -#endif /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ - - while (seed[0] == 0 && seed[1] == 0 && seed[2] == 0 && seed[3] == 0) { - isc_entropy_get(seed, sizeof(seed)); - } - initialized = true; -} - -uint8_t -isc_random8(void) { - isc__random_initialize(); - return (uint8_t)next(); -} - -uint16_t -isc_random16(void) { - isc__random_initialize(); - return (uint16_t)next(); -} +thread_local static uint32_t isc__random_pool[ISC_RANDOM_BUFSIZE]; +thread_local static size_t isc__random_pos = ISC_RANDOM_BUFSIZE; uint32_t isc_random32(void) { - isc__random_initialize(); - return next(); +#if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION + /* + * A fixed stream of numbers helps with problem reproduction when + * fuzzing. The first result needs to be non-zero as expected by + * random_test.c (it starts with ISC_RANDOM_BUFSIZE, see above). + */ + return (uint32_t)(isc__random_pos++); +#endif /* if FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION */ + + if (isc__random_pos == ISC_RANDOM_BUFSIZE) { + isc_random_buf(isc__random_pool, sizeof(isc__random_pool)); + isc__random_pos = 0; + } + + return isc__random_pool[isc__random_pos++]; } void isc_random_buf(void *buf, size_t buflen) { - REQUIRE(buf != NULL); - REQUIRE(buflen > 0); + REQUIRE(buflen == 0 || buf != NULL); - int i; - uint32_t r; - - isc__random_initialize(); - - for (i = 0; i + sizeof(r) <= buflen; i += sizeof(r)) { - r = next(); - memmove((uint8_t *)buf + i, &r, sizeof(r)); + if (buf == NULL || buflen == 0) { + return; } - r = next(); - memmove((uint8_t *)buf + i, &r, buflen % sizeof(r)); - return; + + int r = uv_random(NULL, NULL, buf, buflen, 0, NULL); + UV_RUNTIME_CHECK(uv_random, r); } uint32_t isc_random_uniform(uint32_t limit) { - isc__random_initialize(); - /* * Daniel Lemire's nearly-divisionless unbiased bounded random numbers. * @@ -161,7 +90,7 @@ isc_random_uniform(uint32_t limit) { * integer part (upper 32 bits), and we will use the fraction part * (lower 32 bits) to determine whether or not we need to resample. */ - uint64_t num = (uint64_t)next() * (uint64_t)limit; + uint64_t num = (uint64_t)isc_random32() * (uint64_t)limit; /* * In the fast path, we avoid doing a division in most cases by * comparing the fraction part of `num` with the limit, which is @@ -213,7 +142,7 @@ isc_random_uniform(uint32_t limit) { * our valid range, it is superfluous, and we resample. */ while ((uint32_t)(num) < residue) { - num = (uint64_t)next() * (uint64_t)limit; + num = (uint64_t)isc_random32() * (uint64_t)limit; } } /* @@ -221,3 +150,5 @@ isc_random_uniform(uint32_t limit) { */ return (uint32_t)(num >> 32); } + +#endif /* HAVE_ARC4RANDOM && !defined(__linux__) */ diff --git a/tests/isc/random_test.c b/tests/isc/random_test.c index 21a33167d9..13fdb89c79 100644 --- a/tests/isc/random_test.c +++ b/tests/isc/random_test.c @@ -320,7 +320,9 @@ random_test(pvalue_func_t *func, isc_random_func test_func) { } break; case ISC_RANDOM_BYTES: - isc_random_buf(values, sizeof(values)); + for (i = 0; i < ARRAY_SIZE(values); i++) { + values[i] = isc_random32(); + } break; case ISC_RANDOM_UNIFORM: uniform_values = (uint16_t *)values;