mirror of
https://github.com/isc-projects/bind9.git
synced 2026-05-28 04:34:54 -04:00
Merge branch '4473-fix-doh-intermittent-crash-v9.18' into 'v9.18.28-release'
[9.18] DoH: Avoid potential data races in our DoH implementation related to to HTTP/2 session object management and endpoints set object management See merge request isc-private/bind9!701
This commit is contained in:
Nicki Křížek
commit
139ff18da9
4 changed files with 87 additions and 102 deletions
6
CHANGES
6
CHANGES
|
|
@ -1,3 +1,9 @@
|
|||
6398. [bug] Fix potential data races in our DoH implementation
|
||||
related to HTTP/2 session object management and
|
||||
endpoints set object management after reconfiguration.
|
||||
We would like to thank Dzintars and Ivo from nic.lv
|
||||
for bringing this to our attention. [GL #4473]
|
||||
|
||||
6397. [bug] Clear DNS_FETCHOPT_TRYSTALE_ONTIMEOUT when looking for
|
||||
parent NS records needed to get the DS result.
|
||||
[GL #4661]
|
||||
|
|
|
|||
|
|
@ -37,6 +37,14 @@ Feature Changes
|
|||
Bug Fixes
|
||||
~~~~~~~~~
|
||||
|
||||
- Potential data races were found in our DoH implementation related
|
||||
to HTTP/2 session object management and endpoints set object
|
||||
management after reconfiguration. These issues have been
|
||||
fixed. :gl:`#4473`
|
||||
|
||||
ISC would like to thank Dzintars and Ivo from nic.lv for bringing
|
||||
this to our attention.
|
||||
|
||||
- An RPZ response's SOA record TTL was set to 1 instead of the SOA TTL, if
|
||||
``add-soa`` was used. This has been fixed. :gl:`#3323`
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
@ -221,8 +224,8 @@ call_pending_callbacks(isc__nm_http_pending_callbacks_t pending_callbacks,
|
|||
isc_result_t result);
|
||||
|
||||
static void
|
||||
server_call_cb(isc_nmsocket_t *socket, isc_nm_http_session_t *session,
|
||||
const isc_result_t result, isc_region_t *data);
|
||||
server_call_cb(isc_nmsocket_t *socket, const isc_result_t result,
|
||||
isc_region_t *data);
|
||||
|
||||
static isc_nm_httphandler_t *
|
||||
http_endpoints_find(const char *request_path,
|
||||
|
|
@ -971,25 +974,31 @@ static void
|
|||
http_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region,
|
||||
void *data) {
|
||||
isc_nm_http_session_t *session = (isc_nm_http_session_t *)data;
|
||||
isc_nm_http_session_t *tmpsess = NULL;
|
||||
ssize_t readlen;
|
||||
|
||||
REQUIRE(VALID_HTTP2_SESSION(session));
|
||||
|
||||
UNUSED(handle);
|
||||
/*
|
||||
* Let's ensure that HTTP/2 session and its associated data will
|
||||
* not go "out of scope" too early.
|
||||
*/
|
||||
isc__nm_httpsession_attach(session, &tmpsess);
|
||||
|
||||
if (result != ISC_R_SUCCESS) {
|
||||
if (result != ISC_R_TIMEDOUT) {
|
||||
session->reading = false;
|
||||
}
|
||||
failed_read_cb(result, session);
|
||||
return;
|
||||
goto done;
|
||||
}
|
||||
|
||||
readlen = nghttp2_session_mem_recv(session->ngsession, region->base,
|
||||
region->length);
|
||||
if (readlen < 0) {
|
||||
failed_read_cb(ISC_R_UNEXPECTED, session);
|
||||
return;
|
||||
goto done;
|
||||
}
|
||||
|
||||
if ((size_t)readlen < region->length) {
|
||||
|
|
@ -1006,6 +1015,9 @@ http_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region,
|
|||
|
||||
/* We might have something to receive or send, do IO */
|
||||
http_do_bio(session, NULL, NULL, NULL);
|
||||
|
||||
done:
|
||||
isc__nm_httpsession_detach(&tmpsess);
|
||||
}
|
||||
|
||||
static void
|
||||
|
|
@ -1650,6 +1662,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);
|
||||
socket->tid = session->handle->sock->tid;
|
||||
|
|
@ -1661,21 +1676,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));
|
||||
|
||||
if (atomic_load(&serversocket->listening)) {
|
||||
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) {
|
||||
|
|
@ -1700,9 +1700,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;
|
||||
|
|
@ -1974,8 +1973,6 @@ static void
|
|||
log_server_error_response(const isc_nmsocket_t *socket,
|
||||
const struct http_error_responses *response) {
|
||||
const int log_level = ISC_LOG_DEBUG(1);
|
||||
isc_sockaddr_t client_addr;
|
||||
isc_sockaddr_t local_addr;
|
||||
char client_sabuf[ISC_SOCKADDR_FORMATSIZE];
|
||||
char local_sabuf[ISC_SOCKADDR_FORMATSIZE];
|
||||
|
||||
|
|
@ -1983,10 +1980,8 @@ log_server_error_response(const isc_nmsocket_t *socket,
|
|||
return;
|
||||
}
|
||||
|
||||
client_addr = isc_nmhandle_peeraddr(socket->h2.session->handle);
|
||||
local_addr = isc_nmhandle_localaddr(socket->h2.session->handle);
|
||||
isc_sockaddr_format(&client_addr, client_sabuf, sizeof(client_sabuf));
|
||||
isc_sockaddr_format(&local_addr, local_sabuf, sizeof(local_sabuf));
|
||||
isc_sockaddr_format(&socket->peer, client_sabuf, sizeof(client_sabuf));
|
||||
isc_sockaddr_format(&socket->iface, local_sabuf, sizeof(local_sabuf));
|
||||
isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_NETMGR,
|
||||
log_level, "HTTP/2 request from %s (on %s) failed: %s %s",
|
||||
client_sabuf, local_sabuf, response->header.value,
|
||||
|
|
@ -2025,17 +2020,25 @@ server_send_error_response(const isc_http_error_responses_t error,
|
|||
}
|
||||
|
||||
static void
|
||||
server_call_cb(isc_nmsocket_t *socket, isc_nm_http_session_t *session,
|
||||
const isc_result_t result, isc_region_t *data) {
|
||||
isc_sockaddr_t addr;
|
||||
server_call_cb(isc_nmsocket_t *socket, const isc_result_t result,
|
||||
isc_region_t *data) {
|
||||
isc_nmhandle_t *handle = NULL;
|
||||
|
||||
REQUIRE(VALID_NMSOCK(socket));
|
||||
REQUIRE(VALID_HTTP2_SESSION(session));
|
||||
REQUIRE(socket->h2.cb != NULL);
|
||||
|
||||
addr = isc_nmhandle_peeraddr(session->handle);
|
||||
handle = isc__nmhandle_get(socket, &addr, 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);
|
||||
}
|
||||
|
|
@ -2056,8 +2059,7 @@ isc__nm_http_bad_request(isc_nmhandle_t *handle) {
|
|||
}
|
||||
|
||||
static int
|
||||
server_on_request_recv(nghttp2_session *ngsession,
|
||||
isc_nm_http_session_t *session, isc_nmsocket_t *socket) {
|
||||
server_on_request_recv(nghttp2_session *ngsession, isc_nmsocket_t *socket) {
|
||||
isc_result_t result;
|
||||
isc_http_error_responses_t code = ISC_HTTP_ERROR_SUCCESS;
|
||||
isc_region_t data;
|
||||
|
|
@ -2128,7 +2130,7 @@ server_on_request_recv(nghttp2_session *ngsession,
|
|||
UNREACHABLE();
|
||||
}
|
||||
|
||||
server_call_cb(socket, session, ISC_R_SUCCESS, &data);
|
||||
server_call_cb(socket, ISC_R_SUCCESS, &data);
|
||||
|
||||
return (0);
|
||||
|
||||
|
|
@ -2318,9 +2320,10 @@ isc__nm_http_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
|
|||
static int
|
||||
server_on_frame_recv_callback(nghttp2_session *ngsession,
|
||||
const nghttp2_frame *frame, void *user_data) {
|
||||
isc_nm_http_session_t *session = (isc_nm_http_session_t *)user_data;
|
||||
isc_nmsocket_t *socket = NULL;
|
||||
|
||||
UNUSED(user_data);
|
||||
|
||||
switch (frame->hd.type) {
|
||||
case NGHTTP2_DATA:
|
||||
case NGHTTP2_HEADERS:
|
||||
|
|
@ -2338,8 +2341,7 @@ server_on_frame_recv_callback(nghttp2_session *ngsession,
|
|||
return (0);
|
||||
}
|
||||
|
||||
return (server_on_request_recv(ngsession, session,
|
||||
socket));
|
||||
return (server_on_request_recv(ngsession, socket));
|
||||
}
|
||||
break;
|
||||
default:
|
||||
|
|
@ -2488,7 +2490,6 @@ isc_nm_listenhttp(isc_nm_t *mgr, isc_sockaddr_t *iface, int backlog,
|
|||
|
||||
REQUIRE(VALID_NM(mgr));
|
||||
REQUIRE(!ISC_LIST_EMPTY(eps->handlers));
|
||||
REQUIRE(!ISC_LIST_EMPTY(eps->handler_cbargs));
|
||||
REQUIRE(atomic_load(&eps->in_use) == false);
|
||||
|
||||
sock = isc_mem_get(mgr->mctx, sizeof(*sock));
|
||||
|
|
@ -2541,7 +2542,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);
|
||||
|
|
@ -2555,7 +2555,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;
|
||||
|
|
@ -2576,20 +2575,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));
|
||||
|
|
@ -2622,6 +2612,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;
|
||||
}
|
||||
}
|
||||
|
|
@ -2629,63 +2621,34 @@ 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, const size_t extrahandlesize) {
|
||||
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,
|
||||
.cb = cb,
|
||||
.cbarg = cbarg,
|
||||
.path = isc_mem_strdup(mctx, uri),
|
||||
.extrahandlesize = extrahandlesize,
|
||||
.path = isc_mem_strdup(mctx, uri)
|
||||
.link = ISC_LINK_INITIALIZER,
|
||||
.magic = HTTP_HANDLER_MAGIC
|
||||
};
|
||||
ISC_LINK_INIT(handler, link);
|
||||
|
||||
newhandler = true;
|
||||
}
|
||||
|
||||
if (newhandler) {
|
||||
ISC_LIST_APPEND(eps->handlers, handler, link);
|
||||
}
|
||||
ISC_LIST_APPEND(eps->handler_cbargs, httpcbarg, link);
|
||||
|
||||
return (ISC_R_SUCCESS);
|
||||
}
|
||||
|
||||
|
|
@ -2778,13 +2741,11 @@ 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);
|
||||
isc_buffer_usedregion(&sock->h2.rbuf, &data);
|
||||
server_call_cb(sock, session, result, &data);
|
||||
server_call_cb(sock, result, &data);
|
||||
}
|
||||
|
||||
static void
|
||||
|
|
@ -2813,7 +2774,8 @@ client_call_failed_read_cb(isc_result_t result,
|
|||
}
|
||||
|
||||
if (result != ISC_R_TIMEDOUT || cstream->read_cb == NULL ||
|
||||
!isc__nmsocket_timer_running(session->handle->sock))
|
||||
!(session->handle != NULL &&
|
||||
isc__nmsocket_timer_running(session->handle->sock)))
|
||||
{
|
||||
ISC_LIST_DEQUEUE(session->cstreams, cstream, link);
|
||||
put_http_cstream(session->mctx, cstream);
|
||||
|
|
@ -2901,6 +2863,10 @@ isc__nm_http_has_encryption(const isc_nmhandle_t *handle) {
|
|||
|
||||
INSIST(VALID_HTTP2_SESSION(session));
|
||||
|
||||
if (session->handle == NULL) {
|
||||
return (false);
|
||||
}
|
||||
|
||||
return (isc_nm_socket_type(session->handle) == isc_nm_tlssocket);
|
||||
}
|
||||
|
||||
|
|
@ -2931,6 +2897,10 @@ isc__nm_http_verify_tls_peer_result_string(const isc_nmhandle_t *handle) {
|
|||
|
||||
INSIST(VALID_HTTP2_SESSION(session));
|
||||
|
||||
if (session->handle == NULL) {
|
||||
return (NULL);
|
||||
}
|
||||
|
||||
return (isc_nm_verify_tls_peer_result_string(session->handle));
|
||||
}
|
||||
|
||||
|
|
@ -3204,6 +3174,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->mgr->mctx, sock->h2.request_path);
|
||||
sock->h2.request_path = NULL;
|
||||
|
|
|
|||
|
|
@ -883,13 +883,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;
|
||||
|
|
@ -902,7 +897,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;
|
||||
|
|
@ -914,7 +908,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;
|
||||
|
|
@ -947,6 +940,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;
|
||||
|
|
|
|||
Loading…
Reference in a new issue