From 82a81287f9d5845450d692398a6c24e0f9c0a19c Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 22 Jun 2021 17:01:35 +0200 Subject: [PATCH 1/3] Handle UDP send errors when sending DNS message larger than MTU When the fragmentation is disabled on UDP sockets, the uv_udp_send() call can fail with UV_EMSGSIZE for messages larger than path MTU. Previously, this error would end with just discarding the response. In this commit, a proper handling of such case is added and on such error, a new DNS response with truncated bit set is generated and sent to the client. This change allows us to disable the fragmentation on the UDP sockets again. (cherry picked from commit a3ba95116ed04594ea59a8124bf781b30367a7a2) --- lib/isc/netmgr/uverr2result.c | 2 ++ lib/ns/client.c | 39 ++++++++++++++++++++++++++++++----- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/lib/isc/netmgr/uverr2result.c b/lib/isc/netmgr/uverr2result.c index 8a5c8f6699..2c484fa1a7 100644 --- a/lib/isc/netmgr/uverr2result.c +++ b/lib/isc/netmgr/uverr2result.c @@ -85,6 +85,8 @@ isc___nm_uverr2result(int uverr, bool dolog, const char *file, return (ISC_R_CANCELED); case UV_EOF: return (ISC_R_EOF); + case UV_EMSGSIZE: + return (ISC_R_MAXSIZE); default: if (dolog) { UNEXPECTED_ERROR( diff --git a/lib/ns/client.c b/lib/ns/client.c index 01e0d96895..d76b7b91f0 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -279,13 +279,33 @@ client_senddone(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { REQUIRE(client->sendhandle == handle); CTRACE("senddone"); + + /* + * Set sendhandle to NULL, but don't detach it immediately, in + * case we need to retry the send. If we do resend, then + * sendhandle will be reattached. Whether or not we resend, + * we will then detach the handle from *this* send by detaching + * 'handle' directly below. + */ + client->sendhandle = NULL; + if (result != ISC_R_SUCCESS) { - ns_client_log(client, DNS_LOGCATEGORY_SECURITY, - NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3), - "send failed: %s", isc_result_totext(result)); + if (!TCP_CLIENT(client) && result == ISC_R_MAXSIZE) { + ns_client_log(client, DNS_LOGCATEGORY_SECURITY, + NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3), + "send exceeded maximum size: truncating"); + client->query.attributes &= ~NS_QUERYATTR_ANSWERED; + client->rcode_override = dns_rcode_noerror; + ns_client_error(client, ISC_R_MAXSIZE); + } else { + ns_client_log(client, DNS_LOGCATEGORY_SECURITY, + NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3), + "send failed: %s", + isc_result_totext(result)); + } } - isc_nmhandle_detach(&client->sendhandle); + isc_nmhandle_detach(&handle); } static void @@ -721,8 +741,9 @@ ns_client_dropport(in_port_t port) { void ns_client_error(ns_client_t *client, isc_result_t result) { + dns_message_t *message = NULL; dns_rcode_t rcode; - dns_message_t *message; + bool trunc = false; REQUIRE(NS_CLIENT_VALID(client)); @@ -736,6 +757,10 @@ ns_client_error(ns_client_t *client, isc_result_t result) { rcode = (dns_rcode_t)(client->rcode_override & 0xfff); } + if (result == ISC_R_MAXSIZE) { + trunc = true; + } + #if NS_CLIENT_DROPPORT /* * Don't send FORMERR to ports on the drop port list. @@ -832,7 +857,11 @@ ns_client_error(ns_client_t *client, isc_result_t result) { return; } } + message->rcode = rcode; + if (trunc) { + message->flags |= DNS_MESSAGEFLAG_TC; + } if (rcode == dns_rcode_formerr) { /* From 66a058838cd6982a99dd2bcb88966836a8bb850d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 22 Jun 2021 16:12:44 +0200 Subject: [PATCH 2/3] Disable IP fragmentation on the UDP sockets In DNS Flag Day 2020, we started setting the DF (Don't Fragment socket option on the UDP sockets. It turned out, that this code was incomplete leading to dropping the outgoing UDP packets. This has been now remedied, so it is possible to disable the fragmentation on the UDP sockets again as the sending error is now handled by sending back an empty response with TC (truncated) bit set. This reverts commit 66eefac78c92b64b6689a1655cc677a2b1d13496. (cherry picked from commit b9414110722cf60615c872551f42b99378724392) --- lib/isc/netmgr/netmgr.c | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index cdb4536820..c6bcaea16d 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -2944,22 +2944,14 @@ isc__nm_socket_dontfrag(uv_os_sock_t fd, sa_family_t sa_family) { */ if (sa_family == AF_INET6) { #if defined(IPV6_DONTFRAG) - if (setsockopt_off(fd, IPPROTO_IPV6, IPV6_DONTFRAG) == -1) { + if (setsockopt_on(fd, IPPROTO_IPV6, IPV6_DONTFRAG) == -1) { return (ISC_R_FAILURE); } else { return (ISC_R_SUCCESS); } -#elif defined(IPV6_MTU_DISCOVER) && defined(IP_PMTUDISC_OMIT) +#elif defined(IPV6_MTU_DISCOVER) if (setsockopt(fd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, - &(int){ IP_PMTUDISC_OMIT }, sizeof(int)) == -1) - { - return (ISC_R_FAILURE); - } else { - return (ISC_R_SUCCESS); - } -#elif defined(IPV6_MTU_DISCOVER) && defined(IP_PMTUDISC_DONT) - if (setsockopt(fd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, - &(int){ IP_PMTUDISC_DONT }, sizeof(int)) == -1) + &(int){ IP_PMTUDISC_DO }, sizeof(int)) == -1) { return (ISC_R_FAILURE); } else { @@ -2970,22 +2962,14 @@ isc__nm_socket_dontfrag(uv_os_sock_t fd, sa_family_t sa_family) { #endif } else if (sa_family == AF_INET) { #if defined(IP_DONTFRAG) - if (setsockopt_off(fd, IPPROTO_IP, IP_DONTFRAG) == -1) { + if (setsockopt_on(fd, IPPROTO_IP, IP_DONTFRAG) == -1) { return (ISC_R_FAILURE); } else { return (ISC_R_SUCCESS); } -#elif defined(IP_MTU_DISCOVER) && defined(IP_PMTUDISC_OMIT) +#elif defined(IP_MTU_DISCOVER) if (setsockopt(fd, IPPROTO_IP, IP_MTU_DISCOVER, - &(int){ IP_PMTUDISC_OMIT }, sizeof(int)) == -1) - { - return (ISC_R_FAILURE); - } else { - return (ISC_R_SUCCESS); - } -#elif defined(IP_MTU_DISCOVER) && defined(IP_PMTUDISC_DONT) - if (setsockopt(fd, IPPROTO_IP, IP_MTU_DISCOVER, - &(int){ IP_PMTUDISC_DONT }, sizeof(int)) == -1) + &(int){ IP_PMTUDISC_DO }, sizeof(int)) == -1) { return (ISC_R_FAILURE); } else { From ae21e00eedd0fb24ec4a60fda923a65ef184b928 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 23 Jun 2021 08:56:26 +0200 Subject: [PATCH 3/3] Add CHANGES and release note for [GL #2790] (cherry picked from commit 5d9c0a3721b24f3a53bb0e63948fb20a257d1485) --- CHANGES | 6 ++++++ doc/notes/notes-current.rst | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 5a56833e53..fbf37523ce 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,9 @@ +5664. [func] Handle a UDP sending error on UDP messages larger + than the path MTU; in such a case an empty response is + sent back with the TC (TrunCated) bit set. Re-enable + setting the DF (Don't Fragment) flag on outgoing + UDP sockets. [GL #2790] + 5662. [bug] Views with recursion disabled are now configured with a default cache size of 2 MB, unless "max-cache-size" is explicitly set. This prevents cache RBT hash tables from diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 9a4f1fa9af..3065c8d130 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -34,7 +34,10 @@ Removed Features Feature Changes ~~~~~~~~~~~~~~~ -- None. +- IP fragmentation on outgoing UDP sockets has been disabled. Errors from + sending DNS messages larger than the specified path MTU are properly handled; + ``named`` now sends back empty DNS messages with the TC (TrunCated) bit set, + forcing the DNS client to fall back to TCP. :gl:`#2790` Bug Fixes ~~~~~~~~~