From 38264b6a4d272dbbe948cf66eef9634041d14f98 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 2 Jul 2020 16:27:38 +0200 Subject: [PATCH 01/24] Use different allocators for UDP and TCP Each worker has a receive buffer with space for 20 DNS messages of up to 2^16 bytes each, and the allocator function passed to uv_read_start() or uv_udp_recv_start() will reserve a portion of it for use by sockets. UDP can use recvmmsg() and so it needs that entire space, but TCP reads one message at a time. This commit introduces separate allocator functions for TCP and UDP setting different buffer size limits, so that libuv will provide the correct buffer sizes to each of them. --- lib/isc/netmgr/netmgr-int.h | 10 ---------- lib/isc/netmgr/netmgr.c | 19 +------------------ lib/isc/netmgr/tcp.c | 26 +++++++++++++++++++++++++- lib/isc/netmgr/udp.c | 28 +++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 30 deletions(-) diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index b5593a7f59..6f4d1ff3e7 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -559,16 +559,6 @@ isc__nm_enqueue_ievent(isc__networker_t *worker, isc__netievent_t *event); * way to use an isc__networker_t from another thread.) */ -void -isc__nm_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf); -/*%< - * Allocator for recv operations. - * - * Note that as currently implemented, this doesn't actually - * allocate anything, it just assigns the the isc__networker's UDP - * receive buffer to a socket, and marks it as "in use". - */ - void isc__nm_free_uvbuf(isc_nmsocket_t *sock, const uv_buf_t *buf); /*%< diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 30260afb29..bb10557e9c 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -1003,23 +1003,6 @@ isc__nmsocket_clearcb(isc_nmsocket_t *sock) { sock->accept_cbarg = NULL; } -void -isc__nm_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) { - isc_nmsocket_t *sock = uv_handle_get_data(handle); - isc__networker_t *worker = NULL; - - REQUIRE(VALID_NMSOCK(sock)); - REQUIRE(isc__nm_in_netthread()); - REQUIRE(size <= ISC_NETMGR_RECVBUF_SIZE); - - worker = &sock->mgr->workers[sock->tid]; - INSIST(!worker->recvbuf_inuse); - - buf->base = worker->recvbuf; - worker->recvbuf_inuse = true; - buf->len = ISC_NETMGR_RECVBUF_SIZE; -} - void isc__nm_free_uvbuf(isc_nmsocket_t *sock, const uv_buf_t *buf) { isc__networker_t *worker = NULL; @@ -1032,7 +1015,7 @@ isc__nm_free_uvbuf(isc_nmsocket_t *sock, const uv_buf_t *buf) { worker = &sock->mgr->workers[sock->tid]; REQUIRE(worker->recvbuf_inuse); - if (buf->base > worker->recvbuf && + if (sock->type == isc_nm_udpsocket && buf->base > worker->recvbuf && buf->base <= worker->recvbuf + ISC_NETMGR_RECVBUF_SIZE) { /* Can happen in case of out-of-order recvmmsg in libuv1.36 */ diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 558b685caf..73b8b1fa0b 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -637,6 +637,30 @@ isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) { return (ISC_R_SUCCESS); } +/*%< + * Allocator for TCP read operations. Limited to size 2^16. + * + * Note this doesn't actually allocate anything, it just assigns the + * worker's receive buffer to a socket, and marks it as "in use". + */ +static void +tcp_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) { + isc_nmsocket_t *sock = uv_handle_get_data(handle); + isc__networker_t *worker = NULL; + + REQUIRE(VALID_NMSOCK(sock)); + REQUIRE(sock->type == isc_nm_tcpsocket); + REQUIRE(isc__nm_in_netthread()); + REQUIRE(size <= 65536); + + worker = &sock->mgr->workers[sock->tid]; + INSIST(!worker->recvbuf_inuse); + + buf->base = worker->recvbuf; + buf->len = size; + worker->recvbuf_inuse = true; +} + void isc__nm_async_tcp_startread(isc__networker_t *worker, isc__netievent_t *ev0) { isc__netievent_startread_t *ievent = (isc__netievent_startread_t *)ev0; @@ -654,7 +678,7 @@ isc__nm_async_tcp_startread(isc__networker_t *worker, isc__netievent_t *ev0) { 0); } - r = uv_read_start(&sock->uv_handle.stream, isc__nm_alloc_cb, read_cb); + r = uv_read_start(&sock->uv_handle.stream, tcp_alloc_cb, read_cb); if (r != 0) { isc__nm_incstats(sock->mgr, sock->statsindex[STATID_RECVFAIL]); } diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 544a967ae9..ff981f920f 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -132,6 +132,32 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb, return (ISC_R_SUCCESS); } +/*%< + * Allocator for UDP recv operations. Limited to size 20 * (2^16 + 2), + * which allows enough space for recvmmsg() to get multiple messages at + * a time. + * + * Note this doesn't actually allocate anything, it just assigns the + * worker's receive buffer to a socket, and marks it as "in use". + */ +static void +udp_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) { + isc_nmsocket_t *sock = uv_handle_get_data(handle); + isc__networker_t *worker = NULL; + + REQUIRE(VALID_NMSOCK(sock)); + REQUIRE(sock->type == isc_nm_udpsocket); + REQUIRE(isc__nm_in_netthread()); + REQUIRE(size <= ISC_NETMGR_RECVBUF_SIZE); + + worker = &sock->mgr->workers[sock->tid]; + INSIST(!worker->recvbuf_inuse); + + buf->base = worker->recvbuf; + buf->len = ISC_NETMGR_RECVBUF_SIZE; + worker->recvbuf_inuse = true; +} + /* * handle 'udplisten' async call - start listening on a socket. */ @@ -193,7 +219,7 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) { uv_send_buffer_size(&sock->uv_handle.handle, &(int){ ISC_SEND_BUFFER_SIZE }); #endif - uv_udp_recv_start(&sock->uv_handle.udp, isc__nm_alloc_cb, udp_recv_cb); + uv_udp_recv_start(&sock->uv_handle.udp, udp_alloc_cb, udp_recv_cb); } static void From f2b41e11b4fda394131635bf58bc575b56e5512f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 31 Jul 2020 09:39:46 +0200 Subject: [PATCH 02/24] Add CHANGES and release note for GL #1996 --- CHANGES | 4 +++- doc/notes/notes-current.rst | 6 +++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index b01a399729..855a42e48b 100644 --- a/CHANGES +++ b/CHANGES @@ -18,7 +18,9 @@ 5479. [placeholder] -5478. [placeholder] +5478. [security] It was possible to trigger an assertion failure by + sending a specially crafted large TCP DNS message. + (CVE-2020-8620) [GL #1996] 5477. [bug] The idle timeout for connected TCP sockets is now derived from the client query processing timeout diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 3ee01476ab..0b21089508 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -14,7 +14,11 @@ Notes for BIND 9.17.4 Security Fixes ~~~~~~~~~~~~~~ -- None. +- It was possible to trigger an assertion failure by sending a specially + crafted large TCP DNS message. This was disclosed in CVE-2020-8620. + + ISC would like to thank Emanuel Almeida of Cisco Systems, Inc. for + bringing this vulnerability to our attention. [GL #1996] Known Issues ~~~~~~~~~~~~ From 51c9ea98a332bcd41185b07cf9ee9fba2af063a3 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 10 Jul 2020 13:53:30 -0700 Subject: [PATCH 03/24] permanently disable QNAME minimization in a fetch when forwarding QNAME minimization is normally disabled when forwarding. if, in the course of processing a fetch, we switch back to normal recursion at some point, we can't safely start minimizing because we may have been left in an inconsistent state. --- lib/dns/resolver.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 21f8d96e0e..df43b0a09a 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -4013,6 +4013,15 @@ fctx_nextaddress(fetchctx_t *fctx) { addrinfo->flags |= FCTX_ADDRINFO_MARK; fctx->find = NULL; fctx->forwarding = true; + + /* + * QNAME minimization is disabled when + * forwarding, and has to remain disabled if + * we switch back to normal recursion; otherwise + * forwarding could leave us in an inconsistent + * state. + */ + fctx->minimized = false; return (addrinfo); } } From a3e42f8599ac33b1bf0256e29bdab50c0340ad0f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 10 Jul 2020 14:14:07 -0700 Subject: [PATCH 04/24] Add CHANGES and release note for GL #1997 --- CHANGES | 4 +++- doc/notes/notes-current.rst | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 855a42e48b..902e52fbd8 100644 --- a/CHANGES +++ b/CHANGES @@ -16,7 +16,9 @@ 5480. [placeholder] -5479. [placeholder] +5479. [security] named could crash in certain query resolution scenarios + where QNAME minimization and forwarding were both + enabled. (CVE-2020-8621) [GL #1997] 5478. [security] It was possible to trigger an assertion failure by sending a specially crafted large TCP DNS message. diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 0b21089508..7fc7d91bd8 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -20,6 +20,15 @@ Security Fixes ISC would like to thank Emanuel Almeida of Cisco Systems, Inc. for bringing this vulnerability to our attention. [GL #1996] +- ``named`` could crash after failing an assertion check in certain + query resolution scenarios where QNAME minimization and forwarding + were both enabled. To prevent such crashes, QNAME minimization is now + always disabled for a given query resolution process, if forwarders + are used at any point. This was disclosed in CVE-2020-8621. + + ISC would like to thank Joseph Gullo for bringing this vulnerability + to our attention. [GL #1997] + Known Issues ~~~~~~~~~~~~ From 70a71de9c95a0375262e607e8347e42ca463210e Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 15 Jul 2020 16:07:51 +1000 Subject: [PATCH 05/24] Always keep a copy of the message this allows it to be available even when dns_message_parse() returns a error. --- lib/dns/message.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/dns/message.c b/lib/dns/message.c index dd9dd23a12..97425c753b 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -1709,6 +1709,16 @@ dns_message_parse(dns_message_t *msg, isc_buffer_t *source, msg->header_ok = 0; msg->question_ok = 0; + if ((options & DNS_MESSAGEPARSE_CLONEBUFFER) == 0) { + isc_buffer_usedregion(&origsource, &msg->saved); + } else { + msg->saved.length = isc_buffer_usedlength(&origsource); + msg->saved.base = isc_mem_get(msg->mctx, msg->saved.length); + memmove(msg->saved.base, isc_buffer_base(&origsource), + msg->saved.length); + msg->free_saved = 1; + } + isc_buffer_remainingregion(source, &r); if (r.length < DNS_MESSAGE_HEADERLEN) { return (ISC_R_UNEXPECTEDEND); @@ -1793,15 +1803,6 @@ dns_message_parse(dns_message_t *msg, isc_buffer_t *source, } truncated: - if ((options & DNS_MESSAGEPARSE_CLONEBUFFER) == 0) { - isc_buffer_usedregion(&origsource, &msg->saved); - } else { - msg->saved.length = isc_buffer_usedlength(&origsource); - msg->saved.base = isc_mem_get(msg->mctx, msg->saved.length); - memmove(msg->saved.base, isc_buffer_base(&origsource), - msg->saved.length); - msg->free_saved = 1; - } if (ret == ISC_R_UNEXPECTEDEND && ignore_tc) { return (DNS_R_RECOVERABLE); From e576baad9d7e545160fb15b052ab1699775dead8 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 16 Jul 2020 09:15:20 +1000 Subject: [PATCH 06/24] Add CHANGES and release notes for GL #2028 --- CHANGES | 4 +++- doc/notes/notes-current.rst | 7 +++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 902e52fbd8..cabd1eac71 100644 --- a/CHANGES +++ b/CHANGES @@ -28,7 +28,9 @@ derived from the client query processing timeout configured for a resolver. [GL #2024] -5476. [placeholder] +5476. [security] It was possible to trigger an assertion failure when + verifying the response to a TSIG-signed request. + (CVE-2020-8622) [GL #2028] 5475. [bug] Fix RPZ wildcard passthru ignored when a rejection would overwrite a passthru action matching some diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 7fc7d91bd8..f5fdc44bee 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -29,6 +29,13 @@ Security Fixes ISC would like to thank Joseph Gullo for bringing this vulnerability to our attention. [GL #1997] +- It was possible to trigger an assertion failure when verifying the + response to a TSIG-signed request. This was disclosed in + CVE-2020-8622. + + ISC would like to thank Dave Feldman, Jeff Warren, and Joel Cunningham + of Oracle for bringing this vulnerability to our attention. [GL #2028] + Known Issues ~~~~~~~~~~~~ From 6b7629f323024cf08d268d3fa683b6954f629102 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 21 Jul 2020 14:42:47 +0200 Subject: [PATCH 07/24] Fix crash in pk11_numbits() when native-pkcs11 is used When pk11_numbits() is passed a user provided input that contains all zeroes (via crafted DNS message), it would crash with assertion failure. Fix that by properly handling such input. --- lib/dns/pkcs11rsa_link.c | 75 ++++++++++++++++++++++++--------- lib/isc/include/pk11/internal.h | 4 +- lib/isc/pk11.c | 34 +++++++++------ 3 files changed, 79 insertions(+), 34 deletions(-) diff --git a/lib/dns/pkcs11rsa_link.c b/lib/dns/pkcs11rsa_link.c index fc5efe3bb8..c31ae33da9 100644 --- a/lib/dns/pkcs11rsa_link.c +++ b/lib/dns/pkcs11rsa_link.c @@ -292,6 +292,7 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits, key->key_alg == DST_ALG_NSEC3RSASHA1 || key->key_alg == DST_ALG_RSASHA256 || key->key_alg == DST_ALG_RSASHA512); + REQUIRE(maxbits <= RSA_MAX_PUBEXP_BITS); /* * Reject incorrect RSA key lengths. @@ -334,6 +335,7 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits, for (attr = pk11_attribute_first(rsa); attr != NULL; attr = pk11_attribute_next(rsa, attr)) + { switch (attr->type) { case CKA_MODULUS: INSIST(keyTemplate[5].type == attr->type); @@ -350,13 +352,16 @@ pkcs11rsa_createctx_verify(dst_key_t *key, unsigned int maxbits, memmove(keyTemplate[6].pValue, attr->pValue, attr->ulValueLen); keyTemplate[6].ulValueLen = attr->ulValueLen; - if (pk11_numbits(attr->pValue, attr->ulValueLen) > - maxbits && - maxbits != 0) { + unsigned int bits; + ret = pk11_numbits(attr->pValue, attr->ulValueLen, + &bits); + if (ret != ISC_R_SUCCESS || + (bits > maxbits && maxbits != 0)) { DST_RET(DST_R_VERIFYFAILURE); } break; } + } pk11_ctx->object = CK_INVALID_HANDLE; pk11_ctx->ontoken = false; PK11_RET(pkcs_C_CreateObject, @@ -957,14 +962,17 @@ pkcs11rsa_verify(dst_context_t *dctx, const isc_region_t *sig) { keyTemplate[5].ulValueLen = attr->ulValueLen; break; case CKA_PUBLIC_EXPONENT: + unsigned int bits; INSIST(keyTemplate[6].type == attr->type); keyTemplate[6].pValue = isc_mem_get(dctx->mctx, attr->ulValueLen); memmove(keyTemplate[6].pValue, attr->pValue, attr->ulValueLen); keyTemplate[6].ulValueLen = attr->ulValueLen; - if (pk11_numbits(attr->pValue, attr->ulValueLen) > - RSA_MAX_PUBEXP_BITS) { + ret = pk11_numbits(attr->pValue, attr->ulValueLen, + &bits); + if (ret != ISC_R_SUCCESS || bits > RSA_MAX_PUBEXP_BITS) + { DST_RET(DST_R_VERIFYFAILURE); } break; @@ -1335,6 +1343,8 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { CK_BYTE *exponent = NULL, *modulus = NULL; CK_ATTRIBUTE *attr; unsigned int length; + unsigned int bits; + isc_result_t ret = ISC_R_SUCCESS; isc_buffer_remainingregion(data, &r); if (r.length == 0) { @@ -1351,9 +1361,7 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { if (e_bytes == 0) { if (r.length < 2) { - isc_safe_memwipe(rsa, sizeof(*rsa)); - isc_mem_put(key->mctx, rsa, sizeof(*rsa)); - return (DST_R_INVALIDPUBLICKEY); + DST_RET(DST_R_INVALIDPUBLICKEY); } e_bytes = (*r.base) << 8; isc_region_consume(&r, 1); @@ -1362,16 +1370,18 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { } if (r.length < e_bytes) { - isc_safe_memwipe(rsa, sizeof(*rsa)); - isc_mem_put(key->mctx, rsa, sizeof(*rsa)); - return (DST_R_INVALIDPUBLICKEY); + DST_RET(DST_R_INVALIDPUBLICKEY); } exponent = r.base; isc_region_consume(&r, e_bytes); modulus = r.base; mod_bytes = r.length; - key->key_size = pk11_numbits(modulus, mod_bytes); + ret = pk11_numbits(modulus, mod_bytes, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; isc_buffer_forward(data, length); @@ -1391,6 +1401,10 @@ pkcs11rsa_fromdns(dst_key_t *key, isc_buffer_t *data) { key->keydata.pkey = rsa; return (ISC_R_SUCCESS); +err: + isc_safe_memwipe(rsa, sizeof(*rsa)); + isc_mem_put(key->mctx, rsa, sizeof(*rsa)); + return (ret); } static isc_result_t @@ -1564,6 +1578,7 @@ pkcs11rsa_fetch(dst_key_t *key, const char *engine, const char *label, pk11_object_t *pubrsa; pk11_context_t *pk11_ctx = NULL; isc_result_t ret; + unsigned int bits; if (label == NULL) { return (DST_R_NOENGINE); @@ -1642,7 +1657,11 @@ pkcs11rsa_fetch(dst_key_t *key, const char *engine, const char *label, attr = pk11_attribute_bytype(rsa, CKA_MODULUS); INSIST(attr != NULL); - key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen); + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; return (ISC_R_SUCCESS); @@ -1734,6 +1753,7 @@ pkcs11rsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { CK_ATTRIBUTE *attr; isc_mem_t *mctx = key->mctx; const char *engine = NULL, *label = NULL; + unsigned int bits; /* read private key file */ ret = dst__privstruct_parse(key, DST_ALG_RSA, lexer, mctx, &priv); @@ -1871,12 +1891,20 @@ pkcs11rsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) { attr = pk11_attribute_bytype(rsa, CKA_MODULUS); INSIST(attr != NULL); - key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen); + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; attr = pk11_attribute_bytype(rsa, CKA_PUBLIC_EXPONENT); INSIST(attr != NULL); - if (pk11_numbits(attr->pValue, attr->ulValueLen) > RSA_MAX_PUBEXP_BITS) - { + + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + if (bits > RSA_MAX_PUBEXP_BITS) { DST_RET(ISC_R_RANGE); } @@ -1911,6 +1939,7 @@ pkcs11rsa_fromlabel(dst_key_t *key, const char *engine, const char *label, pk11_context_t *pk11_ctx = NULL; isc_result_t ret; unsigned int i; + unsigned int bits; UNUSED(pin); @@ -1996,14 +2025,22 @@ pkcs11rsa_fromlabel(dst_key_t *key, const char *engine, const char *label, attr = pk11_attribute_bytype(rsa, CKA_PUBLIC_EXPONENT); INSIST(attr != NULL); - if (pk11_numbits(attr->pValue, attr->ulValueLen) > RSA_MAX_PUBEXP_BITS) - { + + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + if (bits > RSA_MAX_PUBEXP_BITS) { DST_RET(ISC_R_RANGE); } attr = pk11_attribute_bytype(rsa, CKA_MODULUS); INSIST(attr != NULL); - key->key_size = pk11_numbits(attr->pValue, attr->ulValueLen); + ret = pk11_numbits(attr->pValue, attr->ulValueLen, &bits); + if (ret != ISC_R_SUCCESS) { + goto err; + } + key->key_size = bits; pk11_return_session(pk11_ctx); isc_safe_memwipe(pk11_ctx, sizeof(*pk11_ctx)); diff --git a/lib/isc/include/pk11/internal.h b/lib/isc/include/pk11/internal.h index dfdf526eeb..05be8997d3 100644 --- a/lib/isc/include/pk11/internal.h +++ b/lib/isc/include/pk11/internal.h @@ -30,8 +30,8 @@ pk11_mem_put(void *ptr, size_t size); CK_SLOT_ID pk11_get_best_token(pk11_optype_t optype); -unsigned int -pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt); +isc_result_t +pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt, unsigned int *bits); CK_ATTRIBUTE * pk11_attribute_first(const pk11_object_t *obj); diff --git a/lib/isc/pk11.c b/lib/isc/pk11.c index bc910d7fdf..315f1170e9 100644 --- a/lib/isc/pk11.c +++ b/lib/isc/pk11.c @@ -652,13 +652,14 @@ pk11_get_best_token(pk11_optype_t optype) { return (token->slotid); } -unsigned int -pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt) { +isc_result_t +pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt, unsigned int *bits) { unsigned int bitcnt, i; CK_BYTE top; if (bytecnt == 0) { - return (0); + *bits = 0; + return (ISC_R_SUCCESS); } bitcnt = bytecnt * 8; for (i = 0; i < bytecnt; i++) { @@ -668,33 +669,40 @@ pk11_numbits(CK_BYTE_PTR data, unsigned int bytecnt) { continue; } if (top & 0x80) { - return (bitcnt); + *bits = bitcnt; + return (ISC_R_SUCCESS); } if (top & 0x40) { - return (bitcnt - 1); + *bits = bitcnt - 1; + return (ISC_R_SUCCESS); } if (top & 0x20) { - return (bitcnt - 2); + *bits = bitcnt - 2; + return (ISC_R_SUCCESS); } if (top & 0x10) { - return (bitcnt - 3); + *bits = bitcnt - 3; + return (ISC_R_SUCCESS); } if (top & 0x08) { - return (bitcnt - 4); + *bits = bitcnt - 4; + return (ISC_R_SUCCESS); } if (top & 0x04) { - return (bitcnt - 5); + *bits = bitcnt - 5; + return (ISC_R_SUCCESS); } if (top & 0x02) { - return (bitcnt - 6); + *bits = bitcnt - 6; + return (ISC_R_SUCCESS); } if (top & 0x01) { - return (bitcnt - 7); + *bits = bitcnt - 7; + return (ISC_R_SUCCESS); } break; } - INSIST(0); - ISC_UNREACHABLE(); + return (ISC_R_RANGE); } CK_ATTRIBUTE * From 52733368fd34ded29d74d565c76d0bc89b9cebd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 21 Jul 2020 16:03:44 +0200 Subject: [PATCH 08/24] Don't strip the SOFTHSM2_CONF and SLOT environment variables when using ./run.sh --- bin/tests/system/run.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/tests/system/run.sh.in b/bin/tests/system/run.sh.in index c59a3e2cfd..06c8d72011 100644 --- a/bin/tests/system/run.sh.in +++ b/bin/tests/system/run.sh.in @@ -70,7 +70,7 @@ if ! $do_run; then if [ "$baseport" -eq 0 ]; then log_flags="$log_flags -p 5300" fi - env - PATH="$PATH" ${LD_LIBRARY_PATH:+"LD_LIBRARY_PATH=${LD_LIBRARY_PATH}"} TESTS="$*" TEST_SUITE_LOG=run.log LOG_DRIVER_FLAGS="--verbose yes --color-tests yes" LOG_FLAGS="$log_flags" make -e check + env - SLOT="$SLOT" SOFTHSM2_CONF="$SOFTHSM2_CONF" PATH="$PATH" ${LD_LIBRARY_PATH:+"LD_LIBRARY_PATH=${LD_LIBRARY_PATH}"} TESTS="$*" TEST_SUITE_LOG=run.log LOG_DRIVER_FLAGS="--verbose yes --color-tests yes" LOG_FLAGS="$log_flags" make -e check exit $? fi From aaeea046ed9d080a0ee4b8df2649ae6f103c6a61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 21 Jul 2020 15:24:21 +0200 Subject: [PATCH 09/24] Add CHANGES and release note for GL #2037 --- CHANGES | 6 +++++- doc/notes/notes-current.rst | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index cabd1eac71..27cf14612f 100644 --- a/CHANGES +++ b/CHANGES @@ -14,7 +14,11 @@ 5481. [placeholder] -5480. [placeholder] +5480. [security] When BIND 9 was compiled with native PKCS#11 support, it + was possible to trigger an assertion failure in code + determining the number of bits in the PKCS#11 RSA public + key with a specially crafted packet. (CVE-2020-8623) + [GL #2037] 5479. [security] named could crash in certain query resolution scenarios where QNAME minimization and forwarding were both diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index f5fdc44bee..175a15b362 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -36,6 +36,14 @@ Security Fixes ISC would like to thank Dave Feldman, Jeff Warren, and Joel Cunningham of Oracle for bringing this vulnerability to our attention. [GL #2028] +- When BIND 9 was compiled with native PKCS#11 support, it was possible + to trigger an assertion failure in code determining the number of bits + in the PKCS#11 RSA public key with a specially crafted packet. This + was disclosed in CVE-2020-8623. + + ISC would like to thank Lyu Chiy for bringing this vulnerability to + our attention. [GL #2037] + Known Issues ~~~~~~~~~~~~ From 952955aa4c7dc53b92b5ee3f8545990aa113cc90 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 29 Jul 2020 23:36:03 +1000 Subject: [PATCH 10/24] Update-policy 'subdomain' was incorrectly treated as 'zonesub' resulting in names outside the specified subdomain having the wrong restrictions for the given key. --- bin/named/zoneconf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 8abbbc462b..39ef13fdcf 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -252,7 +252,8 @@ configure_zone_ssutable(const cfg_obj_t *zconfig, dns_zone_t *zone, str = cfg_obj_asstring(matchtype); CHECK(dns_ssu_mtypefromstring(str, &mtype)); - if (mtype == dns_ssumatchtype_subdomain) { + if (mtype == dns_ssumatchtype_subdomain && + strcasecmp(str, "zonesub") == 0) { usezone = true; } From 9b242cc70735d128b5cda223a9e8701a0b530846 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 29 Jul 2020 23:36:03 +1000 Subject: [PATCH 11/24] Add a test for update-policy 'subdomain' The new test checks that 'update-policy subdomain' is properly enforced. --- bin/tests/system/nsupdate/ns1/named.conf.in | 6 +++++ bin/tests/system/nsupdate/tests.sh | 25 +++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/bin/tests/system/nsupdate/ns1/named.conf.in b/bin/tests/system/nsupdate/ns1/named.conf.in index b20912e86c..85d20bfd53 100644 --- a/bin/tests/system/nsupdate/ns1/named.conf.in +++ b/bin/tests/system/nsupdate/ns1/named.conf.in @@ -37,6 +37,11 @@ key altkey { secret "1234abcd8765"; }; +key restricted.example.nil { + algorithm hmac-md5; + secret "1234abcd8765"; +}; + include "ddns.key"; zone "example.nil" { @@ -46,6 +51,7 @@ zone "example.nil" { check-mx ignore; update-policy { grant ddns-key.example.nil subdomain example.nil ANY; + grant restricted.example.nil subdomain restricted.example.nil ANY; }; allow-transfer { any; }; }; diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index 2cc2a4ec30..794aafbbda 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -639,6 +639,31 @@ then echo_i "failed"; status=1 fi +n=`expr $n + 1` +ret=0 +echo_i "check that 'update-policy subdomain' is properly enforced ($n)" +# "restricted.example.nil" matches "grant ... subdomain restricted.example.nil" +# and thus this UPDATE should succeed. +$NSUPDATE -d < nsupdate.out1-$n 2>&1 || ret=1 +server 10.53.0.1 ${PORT} +key restricted.example.nil 1234abcd8765 +update add restricted.example.nil 0 IN TXT everywhere. +send +END +$DIG $DIGOPTS +tcp @10.53.0.1 restricted.example.nil TXT > dig.out.1.test$n || ret=1 +grep "TXT.*everywhere" dig.out.1.test$n > /dev/null || ret=1 +# "example.nil" does not match "grant ... subdomain restricted.example.nil" and +# thus this UPDATE should fail. +$NSUPDATE -d < nsupdate.out2-$n 2>&1 && ret=1 +server 10.53.0.1 ${PORT} +key restricted.example.nil 1234abcd8765 +update add example.nil 0 IN TXT everywhere. +send +END +$DIG $DIGOPTS +tcp @10.53.0.1 example.nil TXT > dig.out.2.test$n || ret=1 +grep "TXT.*everywhere" dig.out.2.test$n > /dev/null && ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + n=`expr $n + 1` ret=0 echo_i "check that changes to the DNSKEY RRset TTL do not have side effects ($n)" From 94bc07cf053d9ceac6f2c4d5c3028d9f1d6ae179 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 4 Aug 2020 11:41:33 +1000 Subject: [PATCH 12/24] Add a test for update-policy 'zonesub' The new test checks that 'update-policy zonesub' is properly enforced. --- bin/tests/system/nsupdate/ns1/named.conf.in | 6 ++++ bin/tests/system/nsupdate/tests.sh | 35 ++++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/bin/tests/system/nsupdate/ns1/named.conf.in b/bin/tests/system/nsupdate/ns1/named.conf.in index 85d20bfd53..f79a6f223a 100644 --- a/bin/tests/system/nsupdate/ns1/named.conf.in +++ b/bin/tests/system/nsupdate/ns1/named.conf.in @@ -42,6 +42,11 @@ key restricted.example.nil { secret "1234abcd8765"; }; +key zonesub-key.example.nil { + algorithm hmac-md5; + secret "1234subk8765"; +}; + include "ddns.key"; zone "example.nil" { @@ -50,6 +55,7 @@ zone "example.nil" { check-integrity no; check-mx ignore; update-policy { + grant zonesub-key.example.nil zonesub TXT; grant ddns-key.example.nil subdomain example.nil ANY; grant restricted.example.nil subdomain restricted.example.nil ANY; }; diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index 794aafbbda..9e0e618541 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -429,7 +429,7 @@ EOF # this also proves that the server is still running. $DIG $DIGOPTS +tcp +noadd +nosea +nostat +noquest +nocmd +norec example.\ @10.53.0.3 nsec3param > dig.out.ns3.$n || ret=1 -grep "ANSWER: 0" dig.out.ns3.$n > /dev/null || ret=1 +grep "ANSWER: 0," dig.out.ns3.$n > /dev/null || ret=1 grep "flags:[^;]* aa[ ;]" dig.out.ns3.$n > /dev/null || ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } @@ -444,7 +444,7 @@ EOF $DIG $DIGOPTS +tcp +noadd +nosea +nostat +noquest +nocmd +norec nsec3param.test.\ @10.53.0.3 nsec3param > dig.out.ns3.$n || ret=1 -grep "ANSWER: 1" dig.out.ns3.$n > /dev/null || ret=1 +grep "ANSWER: 1," dig.out.ns3.$n > /dev/null || ret=1 grep "3600.*NSEC3PARAM" dig.out.ns3.$n > /dev/null || ret=1 grep "flags:[^;]* aa[ ;]" dig.out.ns3.$n > /dev/null || ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } @@ -461,7 +461,7 @@ EOF _ret=1 for i in 0 1 2 3 4 5 6 7 8 9; do $DIG $DIGOPTS +tcp +norec +time=1 +tries=1 @10.53.0.3 nsec3param.test. NSEC3PARAM > dig.out.ns3.$n || _ret=1 - if grep "ANSWER: 2" dig.out.ns3.$n > /dev/null; then + if grep "ANSWER: 2," dig.out.ns3.$n > /dev/null; then _ret=0 break fi @@ -486,7 +486,7 @@ EOF _ret=1 for i in 0 1 2 3 4 5 6 7 8 9; do $DIG $DIGOPTS +tcp +norec +time=1 +tries=1 @10.53.0.3 nsec3param.test. NSEC3PARAM > dig.out.ns3.$n || _ret=1 - if grep "ANSWER: 1" dig.out.ns3.$n > /dev/null; then + if grep "ANSWER: 1," dig.out.ns3.$n > /dev/null; then _ret=0 break fi @@ -664,6 +664,33 @@ $DIG $DIGOPTS +tcp @10.53.0.1 example.nil TXT > dig.out.2.test$n || ret=1 grep "TXT.*everywhere" dig.out.2.test$n > /dev/null && ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } +n=`expr $n + 1` +ret=0 +echo_i "check that 'update-policy zonesub' is properly enforced ($n)" +# grant zonesub-key.example.nil zonesub TXT; +# the A record update should be rejected as it is not in the type list +$NSUPDATE -d < nsupdate.out1-$n 2>&1 && ret=1 +server 10.53.0.1 ${PORT} +key zonesub-key.example.nil 1234subk8765 +update add zonesub.example.nil 0 IN A 1.2.3.4 +send +END +$DIG $DIGOPTS +tcp @10.53.0.1 zonesub.example.nil A > dig.out.1.test$n || ret=1 +grep "status: REFUSED" nsupdate.out1-$n > /dev/null || ret=1 +grep "ANSWER: 0," dig.out.1.test$n > /dev/null || ret=1 +# the TXT record update should be accepted as it is in the type list +$NSUPDATE -d < nsupdate.out2-$n 2>&1 || ret=1 +server 10.53.0.1 ${PORT} +key zonesub-key.example.nil 1234subk8765 +update add zonesub.example.nil 0 IN TXT everywhere. +send +END +$DIG $DIGOPTS +tcp @10.53.0.1 zonesub.example.nil TXT > dig.out.2.test$n || ret=1 +grep "status: REFUSED" nsupdate.out2-$n > /dev/null && ret=1 +grep "ANSWER: 1," dig.out.2.test$n > /dev/null || ret=1 +grep "TXT.*everywhere" dig.out.2.test$n > /dev/null || ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + n=`expr $n + 1` ret=0 echo_i "check that changes to the DNSKEY RRset TTL do not have side effects ($n)" From 4fb94906fae19692722b51f6a0ea8d7115cec76f Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 29 Jul 2020 23:36:03 +1000 Subject: [PATCH 13/24] Add CHANGES and release note for GL #2055 --- CHANGES | 7 ++++++- doc/notes/notes-current.rst | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 27cf14612f..8284bdbd74 100644 --- a/CHANGES +++ b/CHANGES @@ -12,7 +12,12 @@ system, but the Duplicate Address Detection (DAD) mechanism had not yet finished. [GL #2038] -5481. [placeholder] +5481. [security] "update-policy" rules of type "subdomain" were + incorrectly treated as "zonesub" rules, which allowed + keys used in "subdomain" rules to update names outside + of the specified subdomains. The problem was fixed by + making sure "subdomain" rules are again processed as + described in the ARM. (CVE-2020-8624) [GL #2055] 5480. [security] When BIND 9 was compiled with native PKCS#11 support, it was possible to trigger an assertion failure in code diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 175a15b362..f7b490b80e 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -44,6 +44,15 @@ Security Fixes ISC would like to thank Lyu Chiy for bringing this vulnerability to our attention. [GL #2037] +- ``update-policy`` rules of type ``subdomain`` were incorrectly treated + as ``zonesub`` rules, which allowed keys used in ``subdomain`` rules + to update names outside of the specified subdomains. The problem was + fixed by making sure ``subdomain`` rules are again processed as + described in the ARM. This was disclosed in CVE-2020-8624. + + ISC would like to thank Joop Boonen of credativ GmbH for bringing this + vulnerability to our attention. [GL #2055] + Known Issues ~~~~~~~~~~~~ From 76421c885eb6d74df6382cff32ebca276b8d8b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 5 Aug 2020 16:02:38 +0200 Subject: [PATCH 14/24] Tweak and reword recent CHANGES entries --- CHANGES | 86 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 45 insertions(+), 41 deletions(-) diff --git a/CHANGES b/CHANGES index 8284bdbd74..01592c62b7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,16 +1,16 @@ 5485. [placeholder] -5484. [func] Expire the 0 TTL RRSet quickly rather using them for - stale answers. [GL #1829] +5484. [func] Expire zero TTL records quickly rather than using them + for stale answers. [GL #1829] 5483. [func] Keeping "stale" answers in cache has been disabled by default and can be re-enabled with a new configuration option "stale-cache-enable". [GL #1712] -5482. [bug] BIND 9 would fail to bind to IPv6 addresses in a - tentative state when a new IPv6 address was added to the - system, but the Duplicate Address Detection (DAD) - mechanism had not yet finished. [GL #2038] +5482. [bug] If the Duplicate Address Detection (DAD) mechanism had + not yet finished after adding a new IPv6 address to the + system, BIND 9 would fail to bind to IPv6 addresses in a + tentative state. [GL #2038] 5481. [security] "update-policy" rules of type "subdomain" were incorrectly treated as "zonesub" rules, which allowed @@ -33,53 +33,57 @@ sending a specially crafted large TCP DNS message. (CVE-2020-8620) [GL #1996] -5477. [bug] The idle timeout for connected TCP sockets is now - derived from the client query processing timeout - configured for a resolver. [GL #2024] +5477. [bug] The idle timeout for connected TCP sockets, which was + previously set to a high fixed value, is now derived + from the client query processing timeout configured for + a resolver. [GL #2024] 5476. [security] It was possible to trigger an assertion failure when verifying the response to a TSIG-signed request. (CVE-2020-8622) [GL #2028] -5475. [bug] Fix RPZ wildcard passthru ignored when a rejection - would overwrite a passthru action matching some - rule in a previously loaded passthru rpz zone. - [GL #1619] +5475. [bug] Wildcard RPZ passthru rules could incorrectly be + overridden by other rules that were loaded from RPZ + zones which appeared later in the "response-policy" + statement. This has been fixed. [GL #1619] 5474. [bug] dns_rdata_hip_next() failed to return ISC_R_NOMORE when it should have. [GL !3880] -5473. [func] The rbt hashtable implementation has been changed - to use faster hash-function (HalfSipHash2-4) and - uses Fibonacci hashing for better distribution. - Setting the max-cache-size now preallocates fixed - size hashtable, so the rehashing doesn't cause - resolution brownouts when growing the hashtable. - [GL #1775] +5473. [func] The RBT hash table implementation has been changed + to use a faster hash function (HalfSipHash2-4) and + Fibonacci hashing for better distribution. Setting + "max-cache-size" now preallocates a fixed-size hash + table so that rehashing does not cause resolution + brownouts while the hash table is grown. [GL #1775] 5472. [func] The statistics channel has been updated to use the new network manager. [GL #2022] -5471. [bug] The introduction of KASP support broke whether the - second field of sig-validity-interval was treated as - days or hours. (Thanks to Tony Finch.) [GL !3735] +5471. [bug] The introduction of KASP support inadvertently caused + the second field of "sig-validity-interval" to always be + calculated in hours, even in cases when it should have + been calculated in days. This has been fixed. (Thanks to + Tony Finch.) [GL !3735] -5470. [port] illumos: only call gsskrb5_register_acceptor_identity - if we have gssapi_krb5.h. [GL #1995] +5470. [port] gsskrb5_register_acceptor_identity() is now only called + if gssapi_krb5.h is present. [GL #1995] -5469. [port] illumos: SEC is defined in which - conflicted with our use of SEC. [GL #1993] +5469. [port] On illumos, a constant called SEC is already defined in + , which conflicts with an identically named + constant in libbind9. This conflict has been resolved. + [GL #1993] -5468. [bug] Address potential double unlock in process_fd(). +5468. [bug] Addressed potential double unlock in process_fd(). [GL #2005] 5467. [func] The control channel and the rndc utility have been updated to use the new network manager. To support this, the network manager was updated to enable - wthe initiation of client TCP connections. Its + the initiation of client TCP connections. Its internal reference counting has been refactored. - Note: As side effects of this change, rndc cannot + Note: As a side effect of this change, rndc cannot currently be used with UNIX-domain sockets, and its default timeout has changed from 60 seconds to 30. These will be addressed in a future release. @@ -88,30 +92,30 @@ 5466. [bug] Addressed an error in recursive clients stats reporting. [GL #1719] -5465. [func] Fallback to built in trust-anchors, managed-keys, or - trusted-keys if the bindkeys-file (bind.keys) cannot +5465. [func] Added fallback to built-in trust-anchors, managed-keys, + or trusted-keys if the bindkeys-file (bind.keys) cannot be parsed. [GL #1235] -5464. [bug] Specifying saving more than 128 files when rolling - dnstap / log files would cause buffer overflow. - [GL #1989] +5464. [bug] Requesting more than 128 files to be saved when rolling + dnstap log files caused a buffer overflow. This has been + fixed. [GL #1989] 5463. [placeholder] 5462. [bug] Move LMDB locking from LMDB itself to named. [GL #1976] -5461. [bug] The header STALE attribute was not being updated with - the write lock being held leading to incorrect - statistics. Convert the header attributes to use atomic - operations. [GL #1475] +5461. [bug] The STALE rdataset header attribute was updated while + the write lock was not being held, leading to incorrect + statistics. The header attributes are now converted to + use atomic operations. [GL #1475] 5460. [cleanup] tsig-keygen was previously an alias for ddns-confgen and was documented in the ddns-confgen man page. This has been reversed; tsig-keygen is now the primary name. [GL #1998] -5459. [bug] Bad isc_mem_put() size when an invalid type was - specified in a update-policy rule. [GL #1990] +5459. [bug] Fixed bad isc_mem_put() size when an invalid type was + specified in an "update-policy" rule. [GL #1990] --- 9.17.3 released --- From bc212cf1634aab7f3b9c60ec5860163a35ad8d51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 5 Aug 2020 16:02:38 +0200 Subject: [PATCH 15/24] Tweak and reword release notes --- doc/notes/notes-current.rst | 61 +++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index f7b490b80e..8d2a63cd8d 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -73,44 +73,53 @@ New Features - Statistics channels have also been updated to use the new BIND network manager API. [GL #2022] -- A new configuration option ``stale-cache-enable`` has been introduced to - enable or disable the keeping of stale answers in cache. [GL #1712] +- A new configuration option ``stale-cache-enable`` has been introduced + to enable or disable keeping stale answers in cache. [GL #1712] Feature Changes ~~~~~~~~~~~~~~~ - BIND's cache database implementation has been updated to use a faster - hash-function with better distribution. In addition, the effective - max-cache-size (configured explicitly, defaulting to a value based on system - memory or set to 'unlimited') now pre-allocates fixed size hash tables. This - prevents interruption to query resolution when the hash tables need to be - increased in size. [GL #1775] + hash function with better distribution. In addition, the effective + ``max-cache-size`` (configured explicitly, defaulting to a value based + on system memory or set to ``unlimited``) now pre-allocates fixed-size + hash tables. This prevents interruption to query resolution when the + hash table sizes need to be increased. [GL #1775] - Keeping stale answers in cache has been disabled by default. + [GL #1712] -- The resource records received with 0 TTL are no longer kept in the cache +- Resource records received with 0 TTL are no longer kept in the cache to be used for stale answers. [GL #1829] Bug Fixes ~~~~~~~~~ -- Addressed an error in recursive clients stats reporting. - There were occasions when an incoming query could trigger a prefetch for - some eligible rrset, and if the prefetch code were executed before recursion, - no increment in recursive clients stats would take place. Conversely, - when processing the answers, if the recursion code were executed before the - prefetch, the same counter would be decremented without a matching increment. - [GL #1719] +- Addressed an error in recursive clients stats reporting which could + cause underflow, and even negative statistics. There were occasions + when an incoming query could trigger a prefetch for some eligible + RRset, and if the prefetch code were executed before recursion, no + increment in recursive clients stats would take place. Conversely, + when processing the answers, if the recursion code were executed + before the prefetch, the same counter would be decremented without a + matching increment. [GL #1719] -- The introduction of KASP support broke whether the second field - of sig-validity-interval was treated as days or hours. (Thanks to - Tony Finch.) [GL !3735] +- The introduction of KASP support inadvertently caused the second field + of ``sig-validity-interval`` to always be calculated in hours, even in + cases when it should have been calculated in days. This has been + fixed. (Thanks to Tony Finch.) [GL !3735] -- The IPv6 Duplicate Address Detection (DAD) mechanism could cause the operating - system to report the new IPv6 addresses to the applications via the - getifaddrs() API in a tentative (DAD not yet finished) or duplicate (DAD - failed) state. Such addresses cannot be bound by an application, and named - failed to listen on IPv6 addresses after the DAD mechanism finished. It is - possible to work around the issue by setting the IP_FREEBIND option on the - socket and trying to bind() to the IPv6 address again if the first bind() call - fails with EADDRNOTAVAIL. [GL #2038] +- The IPv6 Duplicate Address Detection (DAD) mechanism could + inadvertently prevent ``named`` from binding to new IPv6 interfaces, + by causing multiple route socket messages to be sent for each IPv6 + address. ``named`` monitors for new interfaces to ``bind()`` to when + it is configured to listen on ``any`` or on a specific range of + addresses. New IPv6 interfaces can be in a "tentative" state before + they are fully available for use. When DAD is in use, two messages are + emitted by the route socket: one when the interface first appears and + then a second one when it is fully "up." An attempt by ``named`` to + ``bind()`` to the new interface prematurely would fail, causing it + thereafter to ignore that address/interface. The problem was worked + around by setting the ``IP_FREEBIND`` option on the socket and trying + to ``bind()`` to each IPv6 address again if the first ``bind()`` call + for that address failed with ``EADDRNOTAVAIL``. [GL #2038] From 23a60ecd153d8e85f709c2356da059fb28eda6de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 5 Aug 2020 16:02:38 +0200 Subject: [PATCH 16/24] Add release note for #1619 --- doc/notes/notes-current.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 8d2a63cd8d..7b8d02065e 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -95,6 +95,10 @@ Feature Changes Bug Fixes ~~~~~~~~~ +- Wildcard RPZ passthru rules could incorrectly be overridden by other + rules that were loaded from RPZ zones which appeared later in the + ``response-policy`` statement. This has been fixed. [GL #1619] + - Addressed an error in recursive clients stats reporting which could cause underflow, and even negative statistics. There were occasions when an incoming query could trigger a prefetch for some eligible From 9d932c6ddc5f7d9b73ace9b18dac06221a82c1da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 5 Aug 2020 16:02:38 +0200 Subject: [PATCH 17/24] Add release note for #1976 --- doc/notes/notes-current.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 7b8d02065e..c32499c42d 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -127,3 +127,6 @@ Bug Fixes around by setting the ``IP_FREEBIND`` option on the socket and trying to ``bind()`` to each IPv6 address again if the first ``bind()`` call for that address failed with ``EADDRNOTAVAIL``. [GL #2038] + +- LMDB locking code was revised to make ``rndc reconfig`` work properly + on FreeBSD and with LMDB >= 0.9.26. [GL #1976] From e0f394bbc4bcdd5eee92d40a795608139ad6f934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 5 Aug 2020 16:02:38 +0200 Subject: [PATCH 18/24] Prepare release notes for BIND 9.17.4 --- doc/arm/notes.rst | 2 +- doc/notes/{notes-current.rst => notes-9.17.4.rst} | 5 ----- util/copyrights | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) rename doc/notes/{notes-current.rst => notes-9.17.4.rst} (99%) diff --git a/doc/arm/notes.rst b/doc/arm/notes.rst index 16a8227d75..334d021820 100644 --- a/doc/arm/notes.rst +++ b/doc/arm/notes.rst @@ -52,7 +52,7 @@ https://www.isc.org/download/. There you will find additional information about each release, source code, and pre-compiled versions for Microsoft Windows operating systems. -.. include:: ../notes/notes-current.rst +.. include:: ../notes/notes-9.17.4.rst .. include:: ../notes/notes-9.17.3.rst .. include:: ../notes/notes-9.17.2.rst .. include:: ../notes/notes-9.17.1.rst diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-9.17.4.rst similarity index 99% rename from doc/notes/notes-current.rst rename to doc/notes/notes-9.17.4.rst index c32499c42d..e57813e7a6 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-9.17.4.rst @@ -53,11 +53,6 @@ Security Fixes ISC would like to thank Joop Boonen of credativ GmbH for bringing this vulnerability to our attention. [GL #2055] -Known Issues -~~~~~~~~~~~~ - -- None. - New Features ~~~~~~~~~~~~ diff --git a/util/copyrights b/util/copyrights index d50730d93e..4236cbfc8a 100644 --- a/util/copyrights +++ b/util/copyrights @@ -1231,7 +1231,7 @@ ./doc/notes/notes-9.17.1.rst RST 2020 ./doc/notes/notes-9.17.2.rst RST 2020 ./doc/notes/notes-9.17.3.rst RST 2020 -./doc/notes/notes-current.rst RST 2020 +./doc/notes/notes-9.17.4.rst RST 2020 ./docutil/HTML_COPYRIGHT X 2001,2004,2016,2018,2019,2020 ./docutil/MAN_COPYRIGHT X 2001,2004,2016,2018,2019,2020 ./docutil/patch-db2latex-duplicate-template-bug X 2007,2018,2019,2020 From 8980d219c7e9e1482986ad9e8afb9f40eb223874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Wed, 5 Aug 2020 16:02:38 +0200 Subject: [PATCH 19/24] Reorder release notes --- doc/notes/notes-9.17.4.rst | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/doc/notes/notes-9.17.4.rst b/doc/notes/notes-9.17.4.rst index e57813e7a6..0571babf09 100644 --- a/doc/notes/notes-9.17.4.rst +++ b/doc/notes/notes-9.17.4.rst @@ -56,6 +56,9 @@ Security Fixes New Features ~~~~~~~~~~~~ +- A new configuration option ``stale-cache-enable`` has been introduced + to enable or disable keeping stale answers in cache. [GL #1712] + - ``rndc`` has been updated to use the new BIND network manager API. This change had the side effect of altering the TCP timeout for RNDC connections from 60 seconds to the ``tcp-idle-timeout`` value, which @@ -68,9 +71,6 @@ New Features - Statistics channels have also been updated to use the new BIND network manager API. [GL #2022] -- A new configuration option ``stale-cache-enable`` has been introduced - to enable or disable keeping stale answers in cache. [GL #1712] - Feature Changes ~~~~~~~~~~~~~~~ @@ -94,20 +94,6 @@ Bug Fixes rules that were loaded from RPZ zones which appeared later in the ``response-policy`` statement. This has been fixed. [GL #1619] -- Addressed an error in recursive clients stats reporting which could - cause underflow, and even negative statistics. There were occasions - when an incoming query could trigger a prefetch for some eligible - RRset, and if the prefetch code were executed before recursion, no - increment in recursive clients stats would take place. Conversely, - when processing the answers, if the recursion code were executed - before the prefetch, the same counter would be decremented without a - matching increment. [GL #1719] - -- The introduction of KASP support inadvertently caused the second field - of ``sig-validity-interval`` to always be calculated in hours, even in - cases when it should have been calculated in days. This has been - fixed. (Thanks to Tony Finch.) [GL !3735] - - The IPv6 Duplicate Address Detection (DAD) mechanism could inadvertently prevent ``named`` from binding to new IPv6 interfaces, by causing multiple route socket messages to be sent for each IPv6 @@ -123,5 +109,19 @@ Bug Fixes to ``bind()`` to each IPv6 address again if the first ``bind()`` call for that address failed with ``EADDRNOTAVAIL``. [GL #2038] +- Addressed an error in recursive clients stats reporting which could + cause underflow, and even negative statistics. There were occasions + when an incoming query could trigger a prefetch for some eligible + RRset, and if the prefetch code were executed before recursion, no + increment in recursive clients stats would take place. Conversely, + when processing the answers, if the recursion code were executed + before the prefetch, the same counter would be decremented without a + matching increment. [GL #1719] + +- The introduction of KASP support inadvertently caused the second field + of ``sig-validity-interval`` to always be calculated in hours, even in + cases when it should have been calculated in days. This has been + fixed. (Thanks to Tony Finch.) [GL !3735] + - LMDB locking code was revised to make ``rndc reconfig`` work properly on FreeBSD and with LMDB >= 0.9.26. [GL #1976] From b096a038e3148e06921fc353ac578109eb5effea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 6 Aug 2020 09:10:06 +0200 Subject: [PATCH 20/24] Update library API versions --- lib/bind9/api | 2 +- lib/dns/api | 4 ++-- lib/isc/api | 2 +- lib/isccc/api | 2 +- lib/isccfg/api | 2 +- lib/ns/api | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/bind9/api b/lib/bind9/api index 1612f27a5d..334285afed 100644 --- a/lib/bind9/api +++ b/lib/bind9/api @@ -12,5 +12,5 @@ # 9.15/9.16: 1500-1699 # 9.17/9.18: 1700-1899 LIBINTERFACE = 1701 -LIBREVISION = 1 +LIBREVISION = 2 LIBAGE = 0 diff --git a/lib/dns/api b/lib/dns/api index c1c1be9b85..0708982280 100644 --- a/lib/dns/api +++ b/lib/dns/api @@ -11,6 +11,6 @@ # 9.13/9.14: 1300-1499 # 9.15/9.16: 1500-1699 # 9.17/9.18: 1700-1899 -LIBINTERFACE = 1703 +LIBINTERFACE = 1704 LIBREVISION = 0 -LIBAGE = 0 +LIBAGE = 1 diff --git a/lib/isc/api b/lib/isc/api index c1c1be9b85..2a38956a54 100644 --- a/lib/isc/api +++ b/lib/isc/api @@ -11,6 +11,6 @@ # 9.13/9.14: 1300-1499 # 9.15/9.16: 1500-1699 # 9.17/9.18: 1700-1899 -LIBINTERFACE = 1703 +LIBINTERFACE = 1704 LIBREVISION = 0 LIBAGE = 0 diff --git a/lib/isccc/api b/lib/isccc/api index 88c9b9df52..ac7231dc34 100644 --- a/lib/isccc/api +++ b/lib/isccc/api @@ -11,6 +11,6 @@ # 9.13/9.14: 1300-1499 # 9.15/9.16: 1500-1699 # 9.17/9.18: 1700-1899 -LIBINTERFACE = 1701 +LIBINTERFACE = 1702 LIBREVISION = 0 LIBAGE = 0 diff --git a/lib/isccfg/api b/lib/isccfg/api index 1612f27a5d..334285afed 100644 --- a/lib/isccfg/api +++ b/lib/isccfg/api @@ -12,5 +12,5 @@ # 9.15/9.16: 1500-1699 # 9.17/9.18: 1700-1899 LIBINTERFACE = 1701 -LIBREVISION = 1 +LIBREVISION = 2 LIBAGE = 0 diff --git a/lib/ns/api b/lib/ns/api index c1c1be9b85..6c720712b8 100644 --- a/lib/ns/api +++ b/lib/ns/api @@ -12,5 +12,5 @@ # 9.15/9.16: 1500-1699 # 9.17/9.18: 1700-1899 LIBINTERFACE = 1703 -LIBREVISION = 0 +LIBREVISION = 1 LIBAGE = 0 From 8e212e96af3bca09630f045447752cdcd044a8be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 6 Aug 2020 09:10:06 +0200 Subject: [PATCH 21/24] Add a CHANGES marker --- CHANGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES b/CHANGES index 01592c62b7..fa60aa5516 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ + --- 9.17.4 released --- + 5485. [placeholder] 5484. [func] Expire zero TTL records quickly rather than using them From 6707a8558b0e182d7caee909a066fd7dc9652f89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 6 Aug 2020 09:10:06 +0200 Subject: [PATCH 22/24] Update BIND version to 9.17.4 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index fdde51e2e3..165d54b542 100644 --- a/configure.ac +++ b/configure.ac @@ -14,7 +14,7 @@ # m4_define([bind_VERSION_MAJOR], 9)dnl m4_define([bind_VERSION_MINOR], 17)dnl -m4_define([bind_VERSION_PATCH], 3)dnl +m4_define([bind_VERSION_PATCH], 4)dnl m4_define([bind_VERSION_EXTRA], )dnl m4_define([bind_DESCRIPTION], [(Development Release)])dnl m4_define([bind_SRCID], [m4_esyscmd_s([git rev-parse --short HEAD | cut -b1-7])])dnl From 94d9ffd4a23686e61c11a3208348197cefeea15a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 6 Aug 2020 09:10:06 +0200 Subject: [PATCH 23/24] Include fuzz/fuzz.h in source tarballs --- fuzz/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/fuzz/Makefile.am b/fuzz/Makefile.am index d3c858a606..9eecd6111c 100644 --- a/fuzz/Makefile.am +++ b/fuzz/Makefile.am @@ -15,6 +15,7 @@ LDADD = \ check_LTLIBRARIES = libfuzzmain.la libfuzzmain_la_SOURCES = \ + fuzz.h \ main.c check_PROGRAMS = \ From 9a2a819817405ad42fd99b64ff8eeedb28ce4604 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 10 Aug 2020 09:02:30 +0200 Subject: [PATCH 24/24] Reduce the default RBT hash table size to 16 entries (4 bits) The hash table rework MRs (!3865, !3871) increased the default RBT hash table size from 64 to 65,536 entries (for 64-bit architectures, that is 512 bytes before vs. 524,288 bytes after). This works fine for RBTs used for cache databases, but since three separate RBT databases are created for every zone loaded (RRs, NSEC, NSEC3), memory usage would skyrocket when BIND 9 is used as an authoritative DNS server with many zones. The default RBT hash table size before the rework was 64 entries, this commit reduces it to 16 entries because our educated guess is that most zones are just couple of entries (SOA, NS, A, AAAA, MX) and rehashing small hash tables is actually cheap. The rework we did in the previous MRs tries to avoid growing the hash tables for big-to-huge caches where growing the hash table comes at a price because the whole cache needs to be locked. (cherry picked from commit 1e043a011b9fe3f62f9f5c7a9b74b44adc03ca44) --- lib/dns/rbt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index eb7029355e..aaa1acf6c7 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -59,7 +59,7 @@ #define CHAIN_MAGIC ISC_MAGIC('0', '-', '0', '-') #define VALID_CHAIN(chain) ISC_MAGIC_VALID(chain, CHAIN_MAGIC) -#define RBT_HASH_MIN_BITS 16 +#define RBT_HASH_MIN_BITS 4 #define RBT_HASH_MAX_BITS 32 #define RBT_HASH_OVERCOMMIT 3 #define RBT_HASH_BUCKETSIZE 4096 /* FIXME: What would be a good value here? */