From 63a4c12227dc1119659fc3438495a06920d82532 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 22 Jun 2022 14:52:58 +0300 Subject: [PATCH 1/6] Store HTTP quota size inside a listenlist instead of the quota This way only quota size is passed to the interface/listener management code instead of a quota object. Thus, we can implement updating the quota object size instead of recreating the object. (cherry picked from commit 3f0b3107720dd53b60c1ca456e5508dd2fde4dc1) --- bin/named/server.c | 23 +---------------------- lib/ns/include/ns/interfacemgr.h | 1 + lib/ns/include/ns/listenlist.h | 4 ++-- lib/ns/include/ns/server.h | 11 +++++++++++ lib/ns/interfacemgr.c | 20 +++++++++++++++++--- lib/ns/listenlist.c | 14 +++++++++++--- lib/ns/server.c | 13 +++++++++++++ 7 files changed, 56 insertions(+), 30 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 6f63a85365..4318c5ca99 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -11383,8 +11383,6 @@ listenelt_http(const cfg_obj_t *http, const uint16_t family, bool tls, size_t len = 1, i = 0; uint32_t max_clients = named_g_http_listener_clients; uint32_t max_streams = named_g_http_streams_per_conn; - ns_server_t *server = NULL; - isc_quota_t *quota = NULL; REQUIRE(target != NULL && *target == NULL); @@ -11439,24 +11437,13 @@ listenelt_http(const cfg_obj_t *http, const uint16_t family, bool tls, INSIST(i == len); - INSIST(named_g_server != NULL); - ns_server_attach(named_g_server->sctx, &server); - if (max_clients > 0) { - quota = isc_mem_get(mctx, sizeof(isc_quota_t)); - isc_quota_init(quota, max_clients); - } result = ns_listenelt_create_http( mctx, port, named_g_dscp, NULL, family, tls, tls_params, - tlsctx_cache, endpoints, len, quota, max_streams, &delt); + tlsctx_cache, endpoints, len, max_clients, max_streams, &delt); if (result != ISC_R_SUCCESS) { goto error; } - if (quota != NULL) { - ISC_LIST_APPEND(server->http_quotas, quota, link); - } - ns_server_detach(&server); - *target = delt; return (result); @@ -11464,14 +11451,6 @@ error: if (delt != NULL) { ns_listenelt_destroy(delt); } - if (quota != NULL) { - isc_quota_destroy(quota); - isc_mem_put(mctx, quota, sizeof(*quota)); - } - - if (server != NULL) { - ns_server_detach(&server); - } return (result); } #endif /* HAVE_LIBNGHTTP2 */ diff --git a/lib/ns/include/ns/interfacemgr.h b/lib/ns/include/ns/interfacemgr.h index c10696d0c0..77cbbf64f4 100644 --- a/lib/ns/include/ns/interfacemgr.h +++ b/lib/ns/include/ns/interfacemgr.h @@ -79,6 +79,7 @@ struct ns_interface { isc_nmsocket_t *tcplistensocket; isc_nmsocket_t *http_listensocket; isc_nmsocket_t *http_secure_listensocket; + isc_quota_t *http_quota; isc_dscp_t dscp; /*%< "listen-on" DSCP value */ isc_refcount_t ntcpaccepting; /*%< Number of clients * ready to accept new diff --git a/lib/ns/include/ns/listenlist.h b/lib/ns/include/ns/listenlist.h index cf433b5028..cbb099c238 100644 --- a/lib/ns/include/ns/listenlist.h +++ b/lib/ns/include/ns/listenlist.h @@ -50,7 +50,7 @@ struct ns_listenelt { isc_tlsctx_cache_t *sslctx_cache; char **http_endpoints; size_t http_endpoints_number; - isc_quota_t *http_quota; + uint32_t http_max_clients; uint32_t max_concurrent_streams; ISC_LINK(ns_listenelt_t) link; }; @@ -98,7 +98,7 @@ ns_listenelt_create_http(isc_mem_t *mctx, in_port_t http_port, isc_dscp_t dscp, dns_acl_t *acl, const uint16_t family, bool tls, const ns_listen_tls_params_t *tls_params, isc_tlsctx_cache_t *tlsctx_cache, char **endpoints, - size_t nendpoints, isc_quota_t *quota, + size_t nendpoints, const uint32_t max_clients, const uint32_t max_streams, ns_listenelt_t **target); /*%< * Create a listen-on list element for HTTP(S). diff --git a/lib/ns/include/ns/server.h b/lib/ns/include/ns/server.h index b1db46f173..0f11612d1e 100644 --- a/lib/ns/include/ns/server.h +++ b/lib/ns/include/ns/server.h @@ -85,6 +85,7 @@ struct ns_server { isc_quota_t tcpquota; isc_quota_t xfroutquota; ISC_LIST(isc_quota_t) http_quotas; + isc_mutex_t http_quotas_lock; /*% Test options and other configurables */ uint32_t options; @@ -184,3 +185,13 @@ ns_server_getoption(ns_server_t *sctx, unsigned int option); * Requires: *\li 'sctx' is valid. */ + +void +ns_server_append_http_quota(ns_server_t *sctx, isc_quota_t *http_quota); +/*%< + * Add a quota to the list of HTTP quotas to destroy it safely later. + * + * Requires: + *\li 'sctx' is valid; + *\li 'http_quota' is not 'NULL'. + */ diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 875b89f3c3..0de572070b 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -570,12 +570,13 @@ ns_interface_listentls(ns_interface_t *ifp, isc_tlsctx_t *sslctx) { static isc_result_t ns_interface_listenhttp(ns_interface_t *ifp, isc_tlsctx_t *sslctx, char **eps, - size_t neps, isc_quota_t *quota, + size_t neps, uint32_t max_clients, uint32_t max_concurrent_streams) { #if HAVE_LIBNGHTTP2 isc_result_t result = ISC_R_FAILURE; isc_nmsocket_t *sock = NULL; isc_nm_http_endpoints_t *epset = NULL; + isc_quota_t *quota = NULL; epset = isc_nm_http_endpoints_new(ifp->mgr->mctx); @@ -589,6 +590,8 @@ ns_interface_listenhttp(ns_interface_t *ifp, isc_tlsctx_t *sslctx, char **eps, } if (result == ISC_R_SUCCESS) { + quota = isc_mem_get(ifp->mgr->mctx, sizeof(*quota)); + isc_quota_init(quota, max_clients); result = isc_nm_listenhttp( ifp->mgr->nm, &ifp->addr, ifp->mgr->backlog, quota, sslctx, epset, max_concurrent_streams, &sock); @@ -596,6 +599,16 @@ ns_interface_listenhttp(ns_interface_t *ifp, isc_tlsctx_t *sslctx, char **eps, isc_nm_http_endpoints_detach(&epset); + if (quota != NULL) { + if (result != ISC_R_SUCCESS) { + isc_quota_destroy(quota); + isc_mem_put(ifp->mgr->mctx, quota, sizeof(*quota)); + } else { + ifp->http_quota = quota; + ns_server_append_http_quota(ifp->mgr->sctx, quota); + } + } + if (result != ISC_R_SUCCESS) { isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_ERROR, "creating %s socket: %s", @@ -628,7 +641,7 @@ ns_interface_listenhttp(ns_interface_t *ifp, isc_tlsctx_t *sslctx, char **eps, UNUSED(sslctx); UNUSED(eps); UNUSED(neps); - UNUSED(quota); + UNUSED(max_clients); UNUSED(max_concurrent_streams); return (ISC_R_NOTIMPLEMENTED); #endif @@ -658,7 +671,7 @@ interface_setup(ns_interfacemgr_t *mgr, isc_sockaddr_t *addr, const char *name, if (elt->is_http) { result = ns_interface_listenhttp( ifp, elt->sslctx, elt->http_endpoints, - elt->http_endpoints_number, elt->http_quota, + elt->http_endpoints_number, elt->http_max_clients, elt->max_concurrent_streams); if (result != ISC_R_SUCCESS) { goto cleanup_interface; @@ -729,6 +742,7 @@ ns_interface_shutdown(ns_interface_t *ifp) { isc_nm_stoplistening(ifp->http_secure_listensocket); isc_nmsocket_close(&ifp->http_secure_listensocket); } + ifp->http_quota = NULL; } static void diff --git a/lib/ns/listenlist.c b/lib/ns/listenlist.c index f852f06d56..85fc12eb63 100644 --- a/lib/ns/listenlist.c +++ b/lib/ns/listenlist.c @@ -171,7 +171,8 @@ listenelt_create(isc_mem_t *mctx, in_port_t port, isc_dscp_t dscp, } elt->http_endpoints = NULL; elt->http_endpoints_number = 0; - elt->http_quota = NULL; + elt->http_max_clients = 0; + elt->max_concurrent_streams = 0; *target = elt; return (ISC_R_SUCCESS); @@ -200,7 +201,7 @@ ns_listenelt_create_http(isc_mem_t *mctx, in_port_t http_port, isc_dscp_t dscp, dns_acl_t *acl, const uint16_t family, bool tls, const ns_listen_tls_params_t *tls_params, isc_tlsctx_cache_t *tlsctx_cache, char **endpoints, - size_t nendpoints, isc_quota_t *quota, + size_t nendpoints, const uint32_t max_clients, const uint32_t max_streams, ns_listenelt_t **target) { isc_result_t result; @@ -214,7 +215,14 @@ ns_listenelt_create_http(isc_mem_t *mctx, in_port_t http_port, isc_dscp_t dscp, (*target)->is_http = true; (*target)->http_endpoints = endpoints; (*target)->http_endpoints_number = nendpoints; - (*target)->http_quota = quota; + /* + * 0 sized quota - means unlimited quota. We used to not + * create a quota object in such a case, but we might need to + * update the value of the quota during reconfiguration, so we + * need to have a quota object in place anyway. + */ + (*target)->http_max_clients = max_clients == 0 ? UINT32_MAX + : max_clients; (*target)->max_concurrent_streams = max_streams; } else { size_t i; diff --git a/lib/ns/server.c b/lib/ns/server.c index 162e344f88..7baf2efc37 100644 --- a/lib/ns/server.c +++ b/lib/ns/server.c @@ -55,6 +55,7 @@ ns_server_create(isc_mem_t *mctx, ns_matchview_t matchingview, isc_quota_init(&sctx->tcpquota, 10); isc_quota_init(&sctx->recursionquota, 100); ISC_LIST_INIT(sctx->http_quotas); + isc_mutex_init(&sctx->http_quotas_lock); CHECKFATAL(dns_tkeyctx_create(mctx, &sctx->tkeyctx)); @@ -150,6 +151,7 @@ ns_server_detach(ns_server_t **sctxp) { sizeof(*http_quota)); http_quota = next; } + isc_mutex_destroy(&sctx->http_quotas_lock); if (sctx->server_id != NULL) { isc_mem_free(sctx->mctx, sctx->server_id); @@ -243,3 +245,14 @@ ns_server_getoption(ns_server_t *sctx, unsigned int option) { return ((sctx->options & option) != 0); } + +void +ns_server_append_http_quota(ns_server_t *sctx, isc_quota_t *http_quota) { + REQUIRE(SCTX_VALID(sctx)); + REQUIRE(http_quota != NULL); + + LOCK(&sctx->http_quotas_lock); + ISC_LINK_INIT(http_quota, link); + ISC_LIST_APPEND(sctx->http_quotas, http_quota, link); + UNLOCK(&sctx->http_quotas_lock); +} From 1ccbb240782fc0108f785b8e242a4429dc0dbbe7 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 22 Jun 2022 15:28:57 +0300 Subject: [PATCH 2/6] Update HTTP listeners quotas on reconfiguration This commit ensures that on reconfiguration a proper value for HTTP connections limit is picked up. The commit also refactors how listeners settings are updated so that there is less code duplication. (cherry picked from commit a2379135fa85a91183a64bd024c758ded9a6d1b0) --- lib/ns/interfacemgr.c | 65 ++++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 0de572070b..caca36ed12 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -927,12 +927,9 @@ clearlistenon(ns_interfacemgr_t *mgr) { } static void -replace_listener_tlsctx(ns_interfacemgr_t *mgr, ns_interface_t *ifp, - isc_tlsctx_t *newctx) { +replace_listener_tlsctx(ns_interface_t *ifp, isc_tlsctx_t *newctx) { char sabuf[ISC_SOCKADDR_FORMATSIZE]; - REQUIRE(NS_INTERFACE_VALID(ifp)); - LOCK(&mgr->lock); isc_sockaddr_format(&ifp->addr, sabuf, sizeof(sabuf)); isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_INFO, "updating TLS context on %s", sabuf); @@ -942,6 +939,41 @@ replace_listener_tlsctx(ns_interfacemgr_t *mgr, ns_interface_t *ifp, } else if (ifp->http_secure_listensocket != NULL) { isc_nmsocket_set_tlsctx(ifp->http_secure_listensocket, newctx); } +} + +static void +update_http_settings(ns_interface_t *ifp, ns_listenelt_t *le) { + REQUIRE(le->is_http); + + INSIST(ifp->http_quota != NULL); + isc_quota_max(ifp->http_quota, le->http_max_clients); +} + +static void +update_listener_configuration(ns_interfacemgr_t *mgr, ns_interface_t *ifp, + ns_listenelt_t *le) { + REQUIRE(NS_INTERFACEMGR_VALID(mgr)); + REQUIRE(NS_INTERFACE_VALID(ifp)); + REQUIRE(le != NULL); + + LOCK(&mgr->lock); + /* + * We need to update the TLS contexts + * inside the TLS/HTTPS listeners during + * a reconfiguration because the + * certificates could have been changed. + */ + if (le->sslctx != NULL) { + replace_listener_tlsctx(ifp, le->sslctx); + } + + /* + * Let's update HTTP listener settings + * on reconfiguration. + */ + if (le->is_http) { + update_http_settings(ifp, le); + } UNLOCK(&mgr->lock); } @@ -1025,15 +1057,9 @@ do_scan(ns_interfacemgr_t *mgr, bool verbose, bool config) { sabuf, ifp->dscp); } if (LISTENING(ifp)) { - /* - * We need to update the TLS contexts - * inside the TLS/HTTPS listeners during - * a reconfiguration because the - * certificates could have been changed. - */ - if (config && le->sslctx != NULL) { - replace_listener_tlsctx( - mgr, ifp, le->sslctx); + if (config) { + update_listener_configuration( + mgr, ifp, le); } continue; } @@ -1190,17 +1216,10 @@ do_scan(ns_interfacemgr_t *mgr, bool verbose, bool config) { sabuf, ifp->dscp); } if (LISTENING(ifp)) { - /* - * We need to update the TLS contexts - * inside the TLS/HTTPS listeners during - * a reconfiguration because the - * certificates could have been changed. - */ - if (config && le->sslctx != NULL) { - replace_listener_tlsctx( - mgr, ifp, le->sslctx); + if (config) { + update_listener_configuration( + mgr, ifp, le); } - continue; } } From bb8ba2c0274683eaf7436a5cd6ad66ea4e4cfcc1 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 22 Jun 2022 16:45:28 +0300 Subject: [PATCH 3/6] Update max concurrent streams limit in HTTP listeners on reconfig This commit ensures that HTTP listeners concurrent streams limit gets updated properly on reconfiguration. (cherry picked from commit e72962d5f1a4e157800519b5ac878cc72052c3a8) --- lib/isc/include/isc/netmgr.h | 34 ++++++++++++++++++++++++++++++++++ lib/isc/netmgr/http.c | 29 +++++++++++++++++++++-------- lib/isc/netmgr/netmgr-int.h | 6 +++++- lib/isc/netmgr/netmgr.c | 17 +++++++++++++++++ lib/ns/interfacemgr.c | 16 ++++++++++++++++ 5 files changed, 93 insertions(+), 9 deletions(-) diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index e5f6d94cff..8dc8360662 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -121,6 +121,25 @@ isc_nmsocket_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx); * \li 'tlsctx' is a valid pointer to a TLS context object. */ +void +isc_nmsocket_set_max_streams(isc_nmsocket_t *listener, + const uint32_t max_streams); +/*%< + * Set the maximum allowed number of concurrent streams for accepted + * client connections. The implementation might be asynchronous + * depending on the listener socket type. + * + * The call is a no-op for any listener socket type that does not + * support concept of multiple sessions per a client + * connection. Currently, it works only for HTTP/2 listeners. + * + * Setting 'max_streams' to '0' instructs the listener that there is + * no limit for concurrent streams. + * + * Requires: + * \li 'listener' is a pointer to a valid network manager listener socket. + */ + #ifdef NETMGR_TRACE #define isc_nmhandle_attach(handle, dest) \ isc__nmhandle_attach(handle, dest, __FILE__, __LINE__, __func__) @@ -645,6 +664,21 @@ isc_nm_http_makeuri(const bool https, const isc_sockaddr_t *sa, * \li 'outbuf' is a valid pointer to a buffer which will get the result; * \li 'outbuf_len' is a size of the result buffer and is greater than zero. */ + +void +isc_nm_http_set_endpoints(isc_nmsocket_t *listener, + isc_nm_http_endpoints_t *eps); +/*%< + * Asynchronously replace the set of HTTP endpoints (paths) within + * the listener socket object. The function is intended to be used + * during reconfiguration. + * + * Requires: + * \li 'listener' is a pointer to a valid network manager listener socket + object with TLS support; + * \li 'eps' is a valid pointer to an HTTP endpoints set. + */ + #endif /* HAVE_LIBNGHTTP2 */ void diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index a3738106dd..79f21c19cf 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -2454,7 +2454,7 @@ httplisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { new_session(httplistensock->mgr->mctx, NULL, &session); session->max_concurrent_streams = - httplistensock->h2.max_concurrent_streams; + atomic_load(&httplistensock->h2.max_concurrent_streams); initialize_nghttp2_server_session(session); handle->sock->h2.session = session; @@ -2481,14 +2481,10 @@ isc_nm_listenhttp(isc_nm_t *mgr, isc_sockaddr_t *iface, int backlog, sock = isc_mem_get(mgr->mctx, sizeof(*sock)); isc__nmsocket_init(sock, mgr, isc_nm_httplistener, iface); - sock->h2.max_concurrent_streams = - NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS; + atomic_init(&sock->h2.max_concurrent_streams, + NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS); - if (max_concurrent_streams > 0 && - max_concurrent_streams < NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS) - { - sock->h2.max_concurrent_streams = max_concurrent_streams; - } + isc_nmsocket_set_max_streams(sock, max_concurrent_streams); atomic_store(&eps->in_use, true); isc_nm_http_endpoints_attach(eps, &sock->h2.listener_endpoints); @@ -2955,6 +2951,23 @@ isc__nm_http_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx) { isc_nmsocket_set_tlsctx(listener->outer, tlsctx); } +void +isc__nm_http_set_max_streams(isc_nmsocket_t *listener, + const uint32_t max_concurrent_streams) { + uint32_t max_streams = NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS; + + REQUIRE(VALID_NMSOCK(listener)); + REQUIRE(listener->type == isc_nm_httplistener); + + if (max_concurrent_streams > 0 && + max_concurrent_streams < NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS) + { + max_streams = max_concurrent_streams; + } + + atomic_store(&listener->h2.max_concurrent_streams, max_streams); +} + static const bool base64url_validation_table[256] = { false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 6b88638011..973af0e985 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -880,7 +880,7 @@ typedef struct isc_nmsocket_h2 { isc_nmsocket_t *httpserver; /* maximum concurrent streams (server-side) */ - uint32_t max_concurrent_streams; + atomic_uint_fast32_t max_concurrent_streams; uint32_t min_ttl; /* used to set "max-age" in responses */ @@ -1833,6 +1833,10 @@ isc__nm_httpsession_detach(isc_nm_http_session_t **sessionp); void isc__nm_http_set_tlsctx(isc_nmsocket_t *sock, isc_tlsctx_t *tlsctx); +void +isc__nm_http_set_max_streams(isc_nmsocket_t *listener, + const uint32_t max_concurrent_streams); + #endif void diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 8bad215f9d..2eaa7d1dd0 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -3733,6 +3733,23 @@ isc_nm_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { return (NULL); } +void +isc_nmsocket_set_max_streams(isc_nmsocket_t *listener, + const uint32_t max_streams) { + REQUIRE(VALID_NMSOCK(listener)); + switch (listener->type) { +#if HAVE_LIBNGHTTP2 + case isc_nm_httplistener: + isc__nm_http_set_max_streams(listener, max_streams); + break; +#endif /* HAVE_LIBNGHTTP2 */ + default: + UNUSED(max_streams); + break; + }; + return; +} + void isc__nmsocket_log_tls_session_reuse(isc_nmsocket_t *sock, isc_tls_t *tls) { const int log_level = ISC_LOG_DEBUG(1); diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index caca36ed12..72e0f47867 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -941,13 +941,26 @@ replace_listener_tlsctx(ns_interface_t *ifp, isc_tlsctx_t *newctx) { } } +#ifdef HAVE_LIBNGHTTP2 static void update_http_settings(ns_interface_t *ifp, ns_listenelt_t *le) { + isc_nmsocket_t *listener; + REQUIRE(le->is_http); INSIST(ifp->http_quota != NULL); isc_quota_max(ifp->http_quota, le->http_max_clients); + + if (ifp->http_secure_listensocket != NULL) { + listener = ifp->http_secure_listensocket; + } else { + INSIST(ifp->http_listensocket != NULL); + listener = ifp->http_listensocket; + } + + isc_nmsocket_set_max_streams(listener, le->max_concurrent_streams); } +#endif /* HAVE_LIBNGHTTP2 */ static void update_listener_configuration(ns_interfacemgr_t *mgr, ns_interface_t *ifp, @@ -967,6 +980,7 @@ update_listener_configuration(ns_interfacemgr_t *mgr, ns_interface_t *ifp, replace_listener_tlsctx(ifp, le->sslctx); } +#ifdef HAVE_LIBNGHTTP2 /* * Let's update HTTP listener settings * on reconfiguration. @@ -974,6 +988,8 @@ update_listener_configuration(ns_interfacemgr_t *mgr, ns_interface_t *ifp, if (le->is_http) { update_http_settings(ifp, le); } +#endif /* HAVE_LIBNGHTTP2 */ + UNLOCK(&mgr->lock); } From b6b07c5646d0db697867b309dc11f27541dac61e Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 22 Jun 2022 19:31:18 +0300 Subject: [PATCH 4/6] Update the set of HTTP endpoints on reconfiguration This commit ensures that on reconfiguration the set of HTTP endpoints (=paths) is being updated within HTTP listeners. (cherry picked from commit d2e13ddf225af1cc285742ab3f6aa3c594810045) --- lib/isc/include/isc/netmgr.h | 3 +- lib/isc/netmgr/http.c | 124 ++++++++++++++++++++++++++++++++--- lib/isc/netmgr/netmgr-int.h | 53 ++++++++++++++- lib/isc/netmgr/netmgr.c | 4 ++ lib/ns/interfacemgr.c | 41 +++++++++--- 5 files changed, 206 insertions(+), 19 deletions(-) diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 8dc8360662..b8e67b6135 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -674,8 +674,7 @@ isc_nm_http_set_endpoints(isc_nmsocket_t *listener, * during reconfiguration. * * Requires: - * \li 'listener' is a pointer to a valid network manager listener socket - object with TLS support; + * \li 'listener' is a pointer to a valid network manager HTTP listener socket; * \li 'eps' is a valid pointer to an HTTP endpoints set. */ diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index 79f21c19cf..e604ee7d63 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -179,6 +179,9 @@ typedef struct isc_http_send_req { isc__nm_http_pending_callbacks_t pending_write_callbacks; } isc_http_send_req_t; +#define HTTP_ENDPOINTS_MAGIC ISC_MAGIC('H', 'T', 'E', 'P') +#define VALID_HTTP_ENDPOINTS(t) ISC_MAGIC_VALID(t, HTTP_ENDPOINTS_MAGIC) + static bool http_send_outgoing(isc_nm_http_session_t *session, isc_nmhandle_t *httphandle, isc_nm_cb_t cb, void *cbarg); @@ -225,6 +228,16 @@ static isc_nm_httphandler_t * http_endpoints_find(const char *request_path, const isc_nm_http_endpoints_t *restrict eps); +static void +http_init_listener_endpoints(isc_nmsocket_t *listener, + isc_nm_http_endpoints_t *epset); + +static void +http_cleanup_listener_endpoints(isc_nmsocket_t *listener); + +static isc_nm_http_endpoints_t * +http_get_listener_endpoints(isc_nmsocket_t *listener, const int tid); + static bool http_session_active(isc_nm_http_session_t *session) { REQUIRE(VALID_HTTP2_SESSION(session)); @@ -1654,14 +1667,15 @@ server_on_begin_headers_callback(nghttp2_session *ngsession, static isc_nm_httphandler_t * find_server_request_handler(const char *request_path, - const isc_nmsocket_t *serversocket) { + isc_nmsocket_t *serversocket, const int tid) { isc_nm_httphandler_t *handler = NULL; REQUIRE(VALID_NMSOCK(serversocket)); if (atomic_load(&serversocket->listening)) { handler = http_endpoints_find( - request_path, serversocket->h2.listener_endpoints); + request_path, + http_get_listener_endpoints(serversocket, tid)); } return (handler); } @@ -1691,7 +1705,8 @@ server_handle_path_header(isc_nmsocket_t *socket, const uint8_t *value, } handler = find_server_request_handler(socket->h2.request_path, - socket->h2.session->serversocket); + socket->h2.session->serversocket, + socket->tid); if (handler != NULL) { socket->h2.cb = handler->cb; socket->h2.cbarg = handler->cbarg; @@ -2487,7 +2502,7 @@ isc_nm_listenhttp(isc_nm_t *mgr, isc_sockaddr_t *iface, int backlog, isc_nmsocket_set_max_streams(sock, max_concurrent_streams); atomic_store(&eps->in_use, true); - isc_nm_http_endpoints_attach(eps, &sock->h2.listener_endpoints); + http_init_listener_endpoints(sock, eps); if (ctx != NULL) { result = isc_nm_listentls(mgr, iface, httplisten_acceptcb, sock, @@ -2530,6 +2545,7 @@ isc_nm_http_endpoints_new(isc_mem_t *mctx) { ISC_LIST_INIT(eps->handlers); isc_refcount_init(&eps->references, 1); atomic_init(&eps->in_use, false); + eps->magic = HTTP_ENDPOINTS_MAGIC; return eps; } @@ -2543,9 +2559,10 @@ isc_nm_http_endpoints_detach(isc_nm_http_endpoints_t **restrict epsp) { REQUIRE(epsp != NULL); eps = *epsp; - REQUIRE(eps != NULL); + REQUIRE(VALID_HTTP_ENDPOINTS(eps)); if (isc_refcount_decrement(&eps->references) > 1) { + *epsp = NULL; return; } @@ -2573,6 +2590,8 @@ isc_nm_http_endpoints_detach(isc_nm_http_endpoints_t **restrict epsp) { httpcbarg = next; } + eps->magic = 0; + isc_mem_putanddetach(&mctx, eps, sizeof(*eps)); *epsp = NULL; } @@ -2580,6 +2599,7 @@ isc_nm_http_endpoints_detach(isc_nm_http_endpoints_t **restrict epsp) { void isc_nm_http_endpoints_attach(isc_nm_http_endpoints_t *source, isc_nm_http_endpoints_t **targetp) { + REQUIRE(VALID_HTTP_ENDPOINTS(source)); REQUIRE(targetp != NULL && *targetp == NULL); isc_refcount_increment(&source->references); @@ -2592,6 +2612,8 @@ http_endpoints_find(const char *request_path, const isc_nm_http_endpoints_t *restrict eps) { isc_nm_httphandler_t *handler = NULL; + REQUIRE(VALID_HTTP_ENDPOINTS(eps)); + if (request_path == NULL || *request_path == '\0') { return (NULL); } @@ -2637,7 +2659,7 @@ isc_nm_http_endpoints_add(isc_nm_http_endpoints_t *restrict eps, isc_nm_httpcbarg_t *restrict httpcbarg = NULL; bool newhandler = false; - REQUIRE(eps != NULL); + REQUIRE(VALID_HTTP_ENDPOINTS(eps)); REQUIRE(isc_nm_http_path_isvalid(uri)); REQUIRE(atomic_load(&eps->in_use) == false); @@ -2968,6 +2990,93 @@ isc__nm_http_set_max_streams(isc_nmsocket_t *listener, atomic_store(&listener->h2.max_concurrent_streams, max_streams); } +void +isc_nm_http_set_endpoints(isc_nmsocket_t *listener, + isc_nm_http_endpoints_t *eps) { + size_t nworkers; + + REQUIRE(VALID_NMSOCK(listener)); + REQUIRE(listener->type == isc_nm_httplistener); + REQUIRE(VALID_HTTP_ENDPOINTS(eps)); + + atomic_store(&eps->in_use, true); + + nworkers = (size_t)listener->mgr->nworkers; + for (size_t i = 0; i < nworkers; i++) { + isc__netievent__http_eps_t *ievent = + isc__nm_get_netievent_httpendpoints(listener->mgr, + listener, eps); + isc__nm_enqueue_ievent(&listener->mgr->workers[i], + (isc__netievent_t *)ievent); + } +} + +void +isc__nm_async_httpendpoints(isc__networker_t *worker, isc__netievent_t *ev0) { + isc__netievent__http_eps_t *ievent = (isc__netievent__http_eps_t *)ev0; + const int tid = isc_nm_tid(); + isc_nmsocket_t *listener = ievent->sock; + isc_nm_http_endpoints_t *eps = ievent->endpoints; + UNUSED(worker); + + isc_nm_http_endpoints_detach(&listener->h2.listener_endpoints[tid]); + isc_nm_http_endpoints_attach(eps, + &listener->h2.listener_endpoints[tid]); +} + +static void +http_init_listener_endpoints(isc_nmsocket_t *listener, + isc_nm_http_endpoints_t *epset) { + size_t nworkers; + + REQUIRE(VALID_NMSOCK(listener)); + REQUIRE(VALID_NM(listener->mgr)); + REQUIRE(VALID_HTTP_ENDPOINTS(epset)); + + nworkers = (size_t)listener->mgr->nworkers; + INSIST(nworkers > 0); + + listener->h2.listener_endpoints = + isc_mem_get(listener->mgr->mctx, + sizeof(isc_nm_http_endpoints_t *) * nworkers); + listener->h2.n_listener_endpoints = nworkers; + for (size_t i = 0; i < nworkers; i++) { + listener->h2.listener_endpoints[i] = NULL; + isc_nm_http_endpoints_attach( + epset, &listener->h2.listener_endpoints[i]); + } +} + +static void +http_cleanup_listener_endpoints(isc_nmsocket_t *listener) { + REQUIRE(VALID_NM(listener->mgr)); + + if (listener->h2.listener_endpoints == NULL) { + return; + } + + for (size_t i = 0; i < listener->h2.n_listener_endpoints; i++) { + isc_nm_http_endpoints_detach( + &listener->h2.listener_endpoints[i]); + } + isc_mem_put(listener->mgr->mctx, listener->h2.listener_endpoints, + sizeof(isc_nm_http_endpoints_t *) * + listener->h2.n_listener_endpoints); + listener->h2.n_listener_endpoints = 0; +} + +static isc_nm_http_endpoints_t * +http_get_listener_endpoints(isc_nmsocket_t *listener, const int tid) { + isc_nm_http_endpoints_t *eps; + REQUIRE(VALID_NMSOCK(listener)); + REQUIRE(tid >= 0); + REQUIRE((size_t)tid < listener->h2.n_listener_endpoints); + + eps = listener->h2.listener_endpoints[tid]; + INSIST(eps != NULL); + return (eps); +} + static const bool base64url_validation_table[256] = { false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, @@ -3121,8 +3230,7 @@ isc__nm_http_cleanup_data(isc_nmsocket_t *sock) { if (sock->type == isc_nm_httplistener && sock->h2.listener_endpoints != NULL) { /* Delete all handlers */ - isc_nm_http_endpoints_detach( - &sock->h2.listener_endpoints); + http_cleanup_listener_endpoints(sock); } if (sock->h2.request_path != NULL) { diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 973af0e985..624530a35c 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -319,6 +319,7 @@ typedef enum isc__netievent_type { netievent_httpclose, netievent_httpsend, + netievent_httpendpoints, netievent_shutdown, netievent_stop, @@ -702,6 +703,42 @@ typedef struct isc__netievent__tlsctx { isc__nm_put_netievent(nm, ievent); \ } +#ifdef HAVE_LIBNGHTTP2 +typedef struct isc__netievent__http_eps { + NETIEVENT__SOCKET; + isc_nm_http_endpoints_t *endpoints; +} isc__netievent__http_eps_t; + +#define NETIEVENT_SOCKET_HTTP_EPS_TYPE(type) \ + typedef isc__netievent__http_eps_t isc__netievent_##type##_t; + +#define NETIEVENT_SOCKET_HTTP_EPS_DECL(type) \ + isc__netievent_##type##_t *isc__nm_get_netievent_##type( \ + isc_nm_t *nm, isc_nmsocket_t *sock, \ + isc_nm_http_endpoints_t *endpoints); \ + void isc__nm_put_netievent_##type(isc_nm_t *nm, \ + isc__netievent_##type##_t *ievent); + +#define NETIEVENT_SOCKET_HTTP_EPS_DEF(type) \ + isc__netievent_##type##_t *isc__nm_get_netievent_##type( \ + isc_nm_t *nm, isc_nmsocket_t *sock, \ + isc_nm_http_endpoints_t *endpoints) { \ + isc__netievent_##type##_t *ievent = \ + isc__nm_get_netievent(nm, netievent_##type); \ + isc__nmsocket_attach(sock, &ievent->sock); \ + isc_nm_http_endpoints_attach(endpoints, &ievent->endpoints); \ + \ + return (ievent); \ + } \ + \ + void isc__nm_put_netievent_##type(isc_nm_t *nm, \ + isc__netievent_##type##_t *ievent) { \ + isc_nm_http_endpoints_detach(&ievent->endpoints); \ + isc__nmsocket_detach(&ievent->sock); \ + isc__nm_put_netievent(nm, ievent); \ + } +#endif /* HAVE_LIBNGHTTP2 */ + typedef union { isc__netievent_t ni; isc__netievent__socket_t nis; @@ -710,6 +747,9 @@ typedef union { isc__netievent__socket_quota_t nisq; isc__netievent_tlsconnect_t nitc; isc__netievent__tlsctx_t nitls; +#ifdef HAVE_LIBNGHTTP2 + isc__netievent__http_eps_t nihttpeps; +#endif /* HAVE_LIBNGHTTP2 */ } isc__netievent_storage_t; /* @@ -854,6 +894,7 @@ typedef struct isc_nm_httphandler { } isc_nm_httphandler_t; struct isc_nm_http_endpoints { + uint32_t magic; isc_mem_t *mctx; ISC_LIST(isc_nm_httphandler_t) handlers; @@ -899,7 +940,8 @@ typedef struct isc_nmsocket_h2 { void *cbarg; LINK(struct isc_nmsocket_h2) link; - isc_nm_http_endpoints_t *listener_endpoints; + isc_nm_http_endpoints_t **listener_endpoints; + size_t n_listener_endpoints; bool response_submitted; struct { @@ -1812,6 +1854,9 @@ isc__nm_async_httpstop(isc__networker_t *worker, isc__netievent_t *ev0); void isc__nm_async_httpclose(isc__networker_t *worker, isc__netievent_t *ev0); +void +isc__nm_async_httpendpoints(isc__networker_t *worker, isc__netievent_t *ev0); + bool isc__nm_parse_httpquery(const char *query_string, const char **start, size_t *len); @@ -2005,9 +2050,12 @@ NETIEVENT_SOCKET_HANDLE_TYPE(tlsdnscancel); NETIEVENT_SOCKET_QUOTA_TYPE(tlsdnsaccept); NETIEVENT_SOCKET_TYPE(tlsdnscycle); +#ifdef HAVE_LIBNGHTTP2 NETIEVENT_SOCKET_TYPE(httpstop); NETIEVENT_SOCKET_REQ_TYPE(httpsend); NETIEVENT_SOCKET_TYPE(httpclose); +NETIEVENT_SOCKET_HTTP_EPS_TYPE(httpendpoints); +#endif /* HAVE_LIBNGHTTP2 */ NETIEVENT_SOCKET_REQ_TYPE(tcpconnect); NETIEVENT_SOCKET_REQ_TYPE(tcpsend); @@ -2076,9 +2124,12 @@ NETIEVENT_SOCKET_HANDLE_DECL(tlsdnscancel); NETIEVENT_SOCKET_QUOTA_DECL(tlsdnsaccept); NETIEVENT_SOCKET_DECL(tlsdnscycle); +#ifdef HAVE_LIBNGHTTP2 NETIEVENT_SOCKET_DECL(httpstop); NETIEVENT_SOCKET_REQ_DECL(httpsend); NETIEVENT_SOCKET_DECL(httpclose); +NETIEVENT_SOCKET_HTTP_EPS_DECL(httpendpoints); +#endif /* HAVE_LIBNGHTTP2 */ NETIEVENT_SOCKET_REQ_DECL(tcpconnect); NETIEVENT_SOCKET_REQ_DECL(tcpsend); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 2eaa7d1dd0..cc7263d914 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -955,6 +955,7 @@ process_netievent(isc__networker_t *worker, isc__netievent_t *ievent) { NETIEVENT_CASE(httpstop); NETIEVENT_CASE(httpsend); NETIEVENT_CASE(httpclose); + NETIEVENT_CASE(httpendpoints); #endif NETIEVENT_CASE(settlsctx); @@ -1068,9 +1069,12 @@ NETIEVENT_SOCKET_QUOTA_DEF(tlsdnsaccept); NETIEVENT_SOCKET_DEF(tlsdnscycle); NETIEVENT_SOCKET_DEF(tlsdnsshutdown); +#ifdef HAVE_LIBNGHTTP2 NETIEVENT_SOCKET_DEF(httpstop); NETIEVENT_SOCKET_REQ_DEF(httpsend); NETIEVENT_SOCKET_DEF(httpclose); +NETIEVENT_SOCKET_HTTP_EPS_DEF(httpendpoints); +#endif /* HAVE_LIBNGHTTP2 */ NETIEVENT_SOCKET_REQ_DEF(tcpconnect); NETIEVENT_SOCKET_REQ_DEF(tcpsend); diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 72e0f47867..1bd684126f 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -568,6 +568,25 @@ ns_interface_listentls(ns_interface_t *ifp, isc_tlsctx_t *sslctx) { return (result); } +#ifdef HAVE_LIBNGHTTP2 +static isc_result_t +load_http_endpoints(isc_nm_http_endpoints_t *epset, ns_interface_t *ifp, + char **eps, size_t neps) { + isc_result_t result = ISC_R_FAILURE; + + for (size_t i = 0; i < neps; i++) { + result = isc_nm_http_endpoints_add(epset, eps[i], + ns__client_request, ifp, + sizeof(ns_client_t)); + if (result != ISC_R_SUCCESS) { + break; + } + } + + return (result); +} +#endif /* HAVE_LIBNGHTTP2 */ + static isc_result_t ns_interface_listenhttp(ns_interface_t *ifp, isc_tlsctx_t *sslctx, char **eps, size_t neps, uint32_t max_clients, @@ -580,14 +599,7 @@ ns_interface_listenhttp(ns_interface_t *ifp, isc_tlsctx_t *sslctx, char **eps, epset = isc_nm_http_endpoints_new(ifp->mgr->mctx); - for (size_t i = 0; i < neps; i++) { - result = isc_nm_http_endpoints_add(epset, eps[i], - ns__client_request, ifp, - sizeof(ns_client_t)); - if (result != ISC_R_SUCCESS) { - break; - } - } + result = load_http_endpoints(epset, ifp, eps, neps); if (result == ISC_R_SUCCESS) { quota = isc_mem_get(ifp->mgr->mctx, sizeof(*quota)); @@ -944,7 +956,9 @@ replace_listener_tlsctx(ns_interface_t *ifp, isc_tlsctx_t *newctx) { #ifdef HAVE_LIBNGHTTP2 static void update_http_settings(ns_interface_t *ifp, ns_listenelt_t *le) { + isc_result_t result; isc_nmsocket_t *listener; + isc_nm_http_endpoints_t *epset; REQUIRE(le->is_http); @@ -959,6 +973,17 @@ update_http_settings(ns_interface_t *ifp, ns_listenelt_t *le) { } isc_nmsocket_set_max_streams(listener, le->max_concurrent_streams); + + epset = isc_nm_http_endpoints_new(ifp->mgr->mctx); + + result = load_http_endpoints(epset, ifp, le->http_endpoints, + le->http_endpoints_number); + + if (result == ISC_R_SUCCESS) { + isc_nm_http_set_endpoints(listener, epset); + } + + isc_nm_http_endpoints_detach(&epset); } #endif /* HAVE_LIBNGHTTP2 */ From 12a6fafae2697b18e842ea9cf51b4c614e2a2012 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 22 Jun 2022 21:38:15 +0300 Subject: [PATCH 5/6] Update CHANGES [GL #3415] Mention that the settings are now applied properly on reconfiguration. (cherry picked from commit 502c78c339956bb68961cc81d56f824d5b498521) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index c663b53fec..1df0150d87 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5911. [bug] Update HTTP listener settings on reconfiguration. + [GL #3415] + 5910. [cleanup] Move built-in dnssec-policies into the defaultconf. These are now printed with 'named -C'. [GL !6467] From 69e1d3804e809e7d3252cd38182bbab412f17edd Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Mon, 27 Jun 2022 16:23:26 +0300 Subject: [PATCH 6/6] doth test: extend with HTTP endpoints reconfiguration check This commit add a check which verifies that HTTP endpoints are being picked up properly by the BIND instance on a reconfiguration. (cherry picked from commit 7822670d0f2c7bc8e592aacb627dfc2c7de13d21) --- bin/tests/system/doth/tests.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/bin/tests/system/doth/tests.sh b/bin/tests/system/doth/tests.sh index 45170aba4a..35f6799814 100644 --- a/bin/tests/system/doth/tests.sh +++ b/bin/tests/system/doth/tests.sh @@ -627,6 +627,25 @@ grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) +n=$((n + 1)) +echo_i "doing rndc reconfig to see if HTTP endpoints have gotten reconfigured ($n)" +ret=0 +# 'sed -i ...' is not portable. Sigh... +sed 's/\/dns-query/\/dns-query-test/g' "ns4/named.conf" > "ns4/named.conf.sed" +mv -f "ns4/named.conf.sed" "ns4/named.conf" +rndc_reconfig ns4 10.53.0.4 60 +retry_quiet 15 wait_for_tlsctx_update_ns4 || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + +n=$((n + 1)) +echo_i "checking DoH query (POST) to verify HTTP endpoint reconfiguration ($n)" +ret=0 +dig_with_https_opts +https='/dns-query-test' @10.53.0.4 example SOA > dig.out.test$n +grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=$((status + ret)) + n=$((n + 1)) echo_i "checking DoT query (with TLS verification enabled) ($n)" ret=0