From: Artem Boldariev Date: Wed, 13 Mar 2024 16:04:46 +0000 (+0200) Subject: Keep the endpoints set reference within an HTTP/2 socket X-Git-Tag: v9.20.0~10^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d80dfbf7457617c863703e071aac005f57371554;p=thirdparty%2Fbind9.git Keep the endpoints set reference within an HTTP/2 socket This commit ensures that an HTTP endpoints set reference is stored in a socket object associated with an HTTP/2 stream instead of referencing the global set stored inside a listener. This helps to prevent an issue like follows: 1. BIND is configured to serve DoH clients; 2. A client is connected and one or more HTTP/2 stream is created. Internal pointers are now pointing to the data on the associated HTTP endpoints set; 3. BIND is reconfigured - the new endpoints set object is created and promoted to all listeners; 4. The old pointers to the HTTP endpoints set data are now invalid. Instead referencing a global object that is updated on re-configurations we now store a local reference which prevents the endpoints set objects to go out of scope prematurely. --- diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index 42e2a9fda11..4dbf6080df7 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -182,6 +182,9 @@ typedef struct isc_http_send_req { #define HTTP_ENDPOINTS_MAGIC ISC_MAGIC('H', 'T', 'E', 'P') #define VALID_HTTP_ENDPOINTS(t) ISC_MAGIC_VALID(t, HTTP_ENDPOINTS_MAGIC) +#define HTTP_HANDLER_MAGIC ISC_MAGIC('H', 'T', 'H', 'L') +#define VALID_HTTP_HANDLER(t) ISC_MAGIC_VALID(t, HTTP_HANDLER_MAGIC) + static bool http_send_outgoing(isc_nm_http_session_t *session, isc_nmhandle_t *httphandle, isc_nm_cb_t cb, void *cbarg); @@ -1714,6 +1717,9 @@ server_on_begin_headers_callback(nghttp2_session *ngsession, }; isc_buffer_initnull(&socket->h2->rbuf); isc_buffer_initnull(&socket->h2->wbuf); + isc_nm_http_endpoints_attach( + http_get_listener_endpoints(session->serversocket, socket->tid), + &socket->h2->peer_endpoints); session->nsstreams++; isc__nm_httpsession_attach(session, &socket->h2->session); ISC_LIST_APPEND(session->sstreams, socket->h2, link); @@ -1723,19 +1729,6 @@ server_on_begin_headers_callback(nghttp2_session *ngsession, return (0); } -static isc_nm_httphandler_t * -find_server_request_handler(const char *request_path, - isc_nmsocket_t *serversocket, const int tid) { - isc_nm_httphandler_t *handler = NULL; - - REQUIRE(VALID_NMSOCK(serversocket)); - - handler = http_endpoints_find( - request_path, http_get_listener_endpoints(serversocket, tid)); - - return (handler); -} - static isc_http_error_responses_t server_handle_path_header(isc_nmsocket_t *socket, const uint8_t *value, const size_t valuelen) { @@ -1760,9 +1753,8 @@ server_handle_path_header(isc_nmsocket_t *socket, const uint8_t *value, return (ISC_HTTP_ERROR_BAD_REQUEST); } - handler = find_server_request_handler(socket->h2->request_path, - socket->h2->session->serversocket, - socket->tid); + handler = http_endpoints_find(socket->h2->request_path, + socket->h2->peer_endpoints); if (handler != NULL) { socket->h2->cb = handler->cb; socket->h2->cbarg = handler->cbarg; @@ -2085,9 +2077,20 @@ server_call_cb(isc_nmsocket_t *socket, const isc_result_t result, isc_nmhandle_t *handle = NULL; REQUIRE(VALID_NMSOCK(socket)); - REQUIRE(socket->h2->cb != NULL); + + /* + * In some cases the callback could not have been set (e.g. when + * the stream was closed prematurely (before processing its HTTP + * path). + */ + if (socket->h2->cb == NULL) { + return; + } handle = isc__nmhandle_get(socket, NULL, NULL); + if (result != ISC_R_SUCCESS) { + data = NULL; + } socket->h2->cb(handle, result, data, socket->h2->cbarg); isc_nmhandle_detach(&handle); } @@ -2505,7 +2508,6 @@ isc_nm_listenhttp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface, REQUIRE(VALID_NM(mgr)); REQUIRE(!ISC_LIST_EMPTY(eps->handlers)); - REQUIRE(!ISC_LIST_EMPTY(eps->handler_cbargs)); REQUIRE(atomic_load(&eps->in_use) == false); REQUIRE(isc_tid() == 0); @@ -2576,7 +2578,6 @@ isc_nm_http_endpoints_new(isc_mem_t *mctx) { *eps = (isc_nm_http_endpoints_t){ .mctx = NULL }; isc_mem_attach(mctx, &eps->mctx); - ISC_LIST_INIT(eps->handler_cbargs); ISC_LIST_INIT(eps->handlers); isc_refcount_init(&eps->references, 1); atomic_init(&eps->in_use, false); @@ -2590,7 +2591,6 @@ isc_nm_http_endpoints_detach(isc_nm_http_endpoints_t **restrict epsp) { isc_nm_http_endpoints_t *restrict eps; isc_mem_t *mctx; isc_nm_httphandler_t *handler = NULL; - isc_nm_httpcbarg_t *httpcbarg = NULL; REQUIRE(epsp != NULL); eps = *epsp; @@ -2611,20 +2611,11 @@ isc_nm_http_endpoints_detach(isc_nm_http_endpoints_t **restrict epsp) { next = ISC_LIST_NEXT(handler, link); ISC_LIST_DEQUEUE(eps->handlers, handler, link); isc_mem_free(mctx, handler->path); + handler->magic = 0; isc_mem_put(mctx, handler, sizeof(*handler)); handler = next; } - httpcbarg = ISC_LIST_HEAD(eps->handler_cbargs); - while (httpcbarg != NULL) { - isc_nm_httpcbarg_t *next = NULL; - - next = ISC_LIST_NEXT(httpcbarg, link); - ISC_LIST_DEQUEUE(eps->handler_cbargs, httpcbarg, link); - isc_mem_put(mctx, httpcbarg, sizeof(isc_nm_httpcbarg_t)); - httpcbarg = next; - } - eps->magic = 0; isc_mem_putanddetach(&mctx, eps, sizeof(*eps)); @@ -2657,6 +2648,8 @@ http_endpoints_find(const char *request_path, handler = ISC_LIST_NEXT(handler, link)) { if (!strcmp(request_path, handler->path)) { + INSIST(VALID_HTTP_HANDLER(handler)); + INSIST(handler->cb != NULL); break; } } @@ -2664,61 +2657,33 @@ http_endpoints_find(const char *request_path, return (handler); } -/* - * In DoH we just need to intercept the request - the response can be sent - * to the client code via the nmhandle directly as it's always just the - * http content. - */ -static void -http_callback(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *data, - void *arg) { - isc_nm_httpcbarg_t *httpcbarg = arg; - - REQUIRE(VALID_NMHANDLE(handle)); - - if (result != ISC_R_SUCCESS) { - /* Shut down the client, then ourselves */ - httpcbarg->cb(handle, result, NULL, httpcbarg->cbarg); - /* XXXWPK FREE */ - return; - } - httpcbarg->cb(handle, result, data, httpcbarg->cbarg); -} - isc_result_t isc_nm_http_endpoints_add(isc_nm_http_endpoints_t *restrict eps, const char *uri, const isc_nm_recv_cb_t cb, void *cbarg) { isc_mem_t *mctx; isc_nm_httphandler_t *restrict handler = NULL; - isc_nm_httpcbarg_t *restrict httpcbarg = NULL; - bool newhandler = false; REQUIRE(VALID_HTTP_ENDPOINTS(eps)); REQUIRE(isc_nm_http_path_isvalid(uri)); + REQUIRE(cb != NULL); REQUIRE(atomic_load(&eps->in_use) == false); mctx = eps->mctx; - httpcbarg = isc_mem_get(mctx, sizeof(isc_nm_httpcbarg_t)); - *httpcbarg = (isc_nm_httpcbarg_t){ .cb = cb, .cbarg = cbarg }; - ISC_LINK_INIT(httpcbarg, link); - if (http_endpoints_find(uri, eps) == NULL) { handler = isc_mem_get(mctx, sizeof(*handler)); - *handler = (isc_nm_httphandler_t){ .cb = http_callback, - .cbarg = httpcbarg, - .path = isc_mem_strdup( - mctx, uri) }; - ISC_LINK_INIT(handler, link); - - newhandler = true; - } + *handler = (isc_nm_httphandler_t){ + .cb = cb, + .cbarg = cbarg, + .path = isc_mem_strdup(mctx, uri), + .link = ISC_LINK_INITIALIZER, + .magic = HTTP_HANDLER_MAGIC + }; - if (newhandler) { ISC_LIST_APPEND(eps->handlers, handler, link); } - ISC_LIST_APPEND(eps->handler_cbargs, httpcbarg, link); + return (ISC_R_SUCCESS); } @@ -2802,8 +2767,6 @@ failed_httpstream_read_cb(isc_nmsocket_t *sock, isc_result_t result, return; } - INSIST(sock->h2->cbarg != NULL); - (void)nghttp2_submit_rst_stream( session->ngsession, NGHTTP2_FLAG_END_STREAM, sock->h2->stream_id, NGHTTP2_REFUSED_STREAM); @@ -3252,6 +3215,12 @@ isc__nm_http_cleanup_data(isc_nmsocket_t *sock) { http_cleanup_listener_endpoints(sock); } + if (sock->type == isc_nm_httpsocket && + sock->h2->peer_endpoints != NULL) + { + isc_nm_http_endpoints_detach(&sock->h2->peer_endpoints); + } + if (sock->h2->request_path != NULL) { isc_mem_free(sock->worker->mctx, sock->h2->request_path); diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 13485dfb58d..56e5bef4a95 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -410,13 +410,8 @@ typedef enum isc_http_scheme_type { ISC_HTTP_SCHEME_UNSUPPORTED } isc_http_scheme_type_t; -typedef struct isc_nm_httpcbarg { - isc_nm_recv_cb_t cb; - void *cbarg; - LINK(struct isc_nm_httpcbarg) link; -} isc_nm_httpcbarg_t; - typedef struct isc_nm_httphandler { + int magic; char *path; isc_nm_recv_cb_t cb; void *cbarg; @@ -428,7 +423,6 @@ struct isc_nm_http_endpoints { isc_mem_t *mctx; ISC_LIST(isc_nm_httphandler_t) handlers; - ISC_LIST(isc_nm_httpcbarg_t) handler_cbargs; isc_refcount_t references; atomic_bool in_use; @@ -440,7 +434,6 @@ typedef struct isc_nmsocket_h2 { char *query_data; size_t query_data_len; bool query_too_large; - isc_nm_httphandler_t *handler; isc_buffer_t rbuf; isc_buffer_t wbuf; @@ -471,6 +464,8 @@ typedef struct isc_nmsocket_h2 { isc_nm_http_endpoints_t **listener_endpoints; size_t n_listener_endpoints; + isc_nm_http_endpoints_t *peer_endpoints; + bool response_submitted; struct { char *uri;