2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 05:57:52 +00:00

Merge branch 'artem/doh-tests-fix' into 'main'

Fix flawed DoH unit tests logic and some corner cases in the DoH code. Fix doh_test failure on FreeBSD 13.0

Closes #2632

See merge request isc-projects/bind9!5005
This commit is contained in:
Artem Boldariev 2021-05-07 13:25:56 +00:00
commit f23afce683
7 changed files with 171 additions and 172 deletions

View File

@ -165,6 +165,12 @@ static void
failed_httpstream_read_cb(isc_nmsocket_t *sock, isc_result_t result,
isc_nm_http_session_t *session);
static void
client_call_failed_read_cb(isc_result_t result, isc_nm_http_session_t *session);
static void
server_call_failed_read_cb(isc_result_t result, isc_nm_http_session_t *session);
static void
failed_read_cb(isc_result_t result, isc_nm_http_session_t *session);
@ -420,23 +426,10 @@ finish_http_session(isc_nm_http_session_t *session) {
isc_nm_cancelread(session->handle);
}
if (!ISC_LIST_EMPTY(session->cstreams)) {
http_cstream_t *cstream =
ISC_LIST_HEAD(session->cstreams);
while (cstream != NULL) {
http_cstream_t *next = ISC_LIST_NEXT(cstream,
link);
ISC_LIST_DEQUEUE(session->cstreams, cstream,
link);
cstream->read_cb(
session->client_httphandle,
ISC_R_UNEXPECTED,
&(isc_region_t){ cstream->rbuf,
cstream->rbufsize },
cstream->read_cbarg);
put_http_cstream(session->mctx, cstream);
cstream = next;
}
if (session->client) {
client_call_failed_read_cb(ISC_R_UNEXPECTED, session);
} else {
server_call_failed_read_cb(ISC_R_UNEXPECTED, session);
}
isc_nmhandle_detach(&session->handle);
}
@ -528,10 +521,10 @@ call_unlink_cstream_readcb(http_cstream_t *cstream,
isc_result_t result) {
REQUIRE(VALID_HTTP2_SESSION(session));
REQUIRE(cstream != NULL);
ISC_LIST_UNLINK(session->cstreams, cstream, link);
cstream->read_cb(session->handle, result,
&(isc_region_t){ cstream->rbuf, cstream->rbufsize },
cstream->read_cbarg);
ISC_LIST_UNLINK(session->cstreams, cstream, link);
put_http_cstream(session->mctx, cstream);
}
@ -925,6 +918,9 @@ http_writecb(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
http_do_bio(session, NULL, NULL, NULL);
session->sending--;
isc_nmhandle_detach(&transphandle);
if (result != ISC_R_SUCCESS && session->sending == 0) {
finish_http_session(session);
}
isc__nm_httpsession_detach(&session);
}
@ -1065,6 +1061,7 @@ http_call_connect_cb(isc_nmsocket_t *sock, isc_nm_http_session_t *session,
REQUIRE(sock->connect_cb != NULL);
if (result == ISC_R_SUCCESS) {
req = isc__nm_uvreq_get(sock->mgr, sock);
req->cb.connect = sock->connect_cb;
req->cbarg = sock->connect_cbarg;
@ -1078,6 +1075,14 @@ http_call_connect_cb(isc_nmsocket_t *sock, isc_nm_http_session_t *session,
isc__nmsocket_clearcb(sock);
isc__nm_connectcb(sock, req, result, true);
} else {
void *cbarg = sock->connect_cbarg;
isc_nm_cb_t connect_cb = sock->connect_cb;
isc__nmsocket_clearcb(sock);
connect_cb(httphandle, result, cbarg);
isc_nmhandle_detach(&httphandle);
}
}
static void
@ -1165,6 +1170,7 @@ error:
isc_mem_free(mctx, http_sock->h2.connect.uri);
}
isc__nmsocket_prep_destroy(http_sock);
isc__nmsocket_detach(&http_sock);
}
@ -2306,6 +2312,7 @@ http_close_direct(isc_nmsocket_t *sock) {
REQUIRE(VALID_NMSOCK(sock));
atomic_store(&sock->closed, true);
atomic_store(&sock->active, false);
session = sock->h2.session;
if (session != NULL && session->handle) {
@ -2315,6 +2322,7 @@ http_close_direct(isc_nmsocket_t *sock) {
void
isc__nm_http_close(isc_nmsocket_t *sock) {
bool destroy = false;
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->type == isc_nm_httpsocket);
REQUIRE(!isc__nmsocket_active(sock));
@ -2324,6 +2332,21 @@ isc__nm_http_close(isc_nmsocket_t *sock) {
return;
}
if (sock->h2.session != NULL && sock->h2.session->closed &&
sock->tid == isc_nm_tid())
{
isc__nm_httpsession_detach(&sock->h2.session);
destroy = true;
} else if (sock->h2.session == NULL && sock->tid == isc_nm_tid()) {
destroy = true;
}
if (destroy) {
http_close_direct(sock);
isc__nmsocket_prep_destroy(sock);
return;
}
isc__netievent_httpclose_t *ievent =
isc__nm_get_netievent_httpclose(sock->mgr, sock);
@ -2371,44 +2394,44 @@ failed_httpstream_read_cb(isc_nmsocket_t *sock, isc_result_t result,
}
static void
failed_read_cb(isc_result_t result, isc_nm_http_session_t *session) {
client_call_failed_read_cb(isc_result_t result,
isc_nm_http_session_t *session) {
http_cstream_t *cstream = NULL;
REQUIRE(VALID_HTTP2_SESSION(session));
REQUIRE(result != ISC_R_SUCCESS);
if (session->client) {
http_cstream_t *cstream = NULL;
cstream = ISC_LIST_HEAD(session->cstreams);
while (cstream != NULL) {
http_cstream_t *next = ISC_LIST_NEXT(cstream, link);
cstream->read_cb(session->client_httphandle, result,
&(isc_region_t){ cstream->rbuf,
cstream->rbufsize },
cstream->read_cb(
session->client_httphandle, result,
&(isc_region_t){ cstream->rbuf, cstream->rbufsize },
cstream->read_cbarg);
if (result != ISC_R_TIMEDOUT ||
!isc__nmsocket_timer_running(session->handle->sock))
{
ISC_LIST_DEQUEUE(session->cstreams, cstream,
link);
ISC_LIST_DEQUEUE(session->cstreams, cstream, link);
put_http_cstream(session->mctx, cstream);
}
cstream = next;
}
/*
* If result was ISC_R_TIMEDOUT and the timer was reset,
* then we still have active streams and should not close
* the session.
*/
if (ISC_LIST_EMPTY(session->cstreams)) {
finish_http_session(session);
}
} else {
static void
server_call_failed_read_cb(isc_result_t result,
isc_nm_http_session_t *session) {
isc_nmsocket_h2_t *h2data = NULL; /* stream socket */
REQUIRE(VALID_HTTP2_SESSION(session));
REQUIRE(result != ISC_R_SUCCESS);
for (h2data = ISC_LIST_HEAD(session->sstreams); h2data != NULL;
h2data = ISC_LIST_NEXT(h2data, link))
{
failed_httpstream_read_cb(h2data->psock, result,
session);
failed_httpstream_read_cb(h2data->psock, result, session);
}
h2data = ISC_LIST_HEAD(session->sstreams);
@ -2422,7 +2445,22 @@ failed_read_cb(isc_result_t result, isc_nm_http_session_t *session) {
h2data = next;
}
}
static void
failed_read_cb(isc_result_t result, isc_nm_http_session_t *session) {
if (session->client) {
client_call_failed_read_cb(result, session);
/*
* If result was ISC_R_TIMEDOUT and the timer was reset,
* then we still have active streams and should not close
* the session.
*/
if (ISC_LIST_EMPTY(session->cstreams)) {
finish_http_session(session);
}
} else {
server_call_failed_read_cb(result, session);
/*
* All streams are now destroyed; close the session.
*/

View File

@ -1416,9 +1416,6 @@ isc__nm_async_tlsclose(isc__networker_t *worker, isc__netievent_t *ev0);
void
isc__nm_async_tlssend(isc__networker_t *worker, isc__netievent_t *ev0);
void
isc__nm_async_tlsconnect(isc__networker_t *worker, isc__netievent_t *ev0);
void
isc__nm_async_tlsstartread(isc__networker_t *worker, isc__netievent_t *ev0);

View File

@ -869,7 +869,6 @@ process_netievent(isc__networker_t *worker, isc__netievent_t *ievent) {
NETIEVENT_CASE(tlsstartread);
NETIEVENT_CASE(tlssend);
NETIEVENT_CASE(tlsclose);
NETIEVENT_CASE(tlsconnect);
NETIEVENT_CASE(tlsdobio);
NETIEVENT_CASE(tlscancel);

View File

@ -317,9 +317,13 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
if (result != ISC_R_SUCCESS) {
if (isc__nm_in_netthread()) {
sock->tid = isc_nm_tid();
}
isc__nmsocket_clearcb(sock);
isc__nm_connectcb(sock, req, result, false);
} else {
isc__nmsocket_clearcb(sock);
sock->tid = isc_random_uniform(mgr->nworkers);
isc__nm_connectcb(sock, req, result, true);
}
atomic_store(&sock->closed, true);
isc__nmsocket_detach(&sock);
return;

View File

@ -86,26 +86,6 @@ inactive(isc_nmsocket_t *sock) {
atomic_load(&sock->mgr->closing));
}
static void
update_result(isc_nmsocket_t *sock, const isc_result_t result) {
if (!atomic_load(&sock->tlsstream.result_updated)) {
atomic_store(&sock->tlsstream.result_updated, true);
if (!sock->tlsstream.server) {
LOCK(&sock->lock);
sock->result = result;
SIGNAL(&sock->cond);
while (!atomic_load(&sock->active)) {
WAIT(&sock->scond, &sock->lock);
}
UNLOCK(&sock->lock);
} else {
LOCK(&sock->lock);
sock->result = result;
UNLOCK(&sock->lock);
}
}
}
static void
tls_call_connect_cb(isc_nmsocket_t *sock, isc_nmhandle_t *handle,
const isc_result_t result) {
@ -133,8 +113,14 @@ tls_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) {
send_req->tlssock = NULL;
if (send_req->cb != NULL) {
INSIST(VALID_NMHANDLE(tlssock->statichandle));
send_req->cb(send_req->handle, eresult, send_req->cbarg);
isc_nmhandle_detach(&send_req->handle);
/* The last handle has been just detached: close the underlying
* socket. */
if (tlssock->statichandle == NULL) {
finish = true;
}
}
isc_mem_put(handle->sock->mgr->mctx, send_req->data.base,
@ -197,7 +183,8 @@ tls_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result) {
}
isc__nm_readcb(sock, req, result);
if (result == ISC_R_TIMEDOUT &&
isc__nmsocket_timer_running(sock->outerhandle->sock))
(sock->outerhandle == NULL ||
isc__nmsocket_timer_running(sock->outerhandle->sock)))
{
destroy = false;
}
@ -420,7 +407,8 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data,
/* Decrypt and pass data from network to client */
if (sock->tlsstream.state >= TLS_IO && sock->recv_cb != NULL &&
!atomic_load(&sock->readpaused))
!atomic_load(&sock->readpaused) &&
sock->statichandle != NULL)
{
uint8_t recv_buf[TLS_BUF_SIZE];
INSIST(sock->tlsstream.state > TLS_HANDSHAKE);
@ -562,6 +550,7 @@ initialize_tls(isc_nmsocket_t *sock, bool server) {
sock->tlsstream.bio_out);
sock->tlsstream.server = server;
sock->tlsstream.nsending = 0;
sock->tlsstream.state = TLS_INIT;
return (ISC_R_SUCCESS);
error:
isc_tls_free(&sock->tlsstream.tls);
@ -606,7 +595,6 @@ tlslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
tlssock->peer = handle->sock->peer;
tlssock->read_timeout = atomic_load(&handle->sock->mgr->init);
tlssock->tid = isc_nm_tid();
tlssock->tlsstream.state = TLS_INIT;
tlssock->tlsstream.ctx = tlslistensock->tlsstream.ctx;
@ -823,6 +811,7 @@ tls_close_direct(isc_nmsocket_t *sock) {
/* further cleanup performed in isc__nm_tls_cleanup_data() */
atomic_store(&sock->closed, true);
atomic_store(&sock->active, false);
sock->tlsstream.state = TLS_CLOSED;
}
@ -875,12 +864,14 @@ isc__nm_tls_stoplistening(isc_nmsocket_t *sock) {
}
}
static void
tcp_connected(isc_nmhandle_t *handle, isc_result_t result, void *cbarg);
void
isc_nm_tlsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
isc_nm_cb_t cb, void *cbarg, SSL_CTX *ctx,
unsigned int timeout, size_t extrahandlesize) {
isc_nmsocket_t *nsock = NULL, *tsock = NULL;
isc__netievent_tlsconnect_t *ievent = NULL;
isc_nmsocket_t *nsock = NULL;
#if defined(NETMGR_TRACE) && defined(NETMGR_TRACE_VERBOSE)
fprintf(stderr, "TLS: isc_nm_tlsconnect(): in net thread: %s\n",
isc__nm_in_netthread() ? "yes" : "no");
@ -900,33 +891,10 @@ isc_nm_tlsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
nsock->connect_timeout = timeout;
nsock->tlsstream.ctx = ctx;
ievent = isc__nm_get_netievent_tlsconnect(mgr, nsock);
ievent->local = local->addr;
ievent->peer = peer->addr;
ievent->ctx = ctx;
isc__nmsocket_attach(nsock, &tsock);
if (isc__nm_in_netthread()) {
atomic_store(&nsock->active, true);
nsock->tid = isc_nm_tid();
isc__nm_async_tlsconnect(&mgr->workers[nsock->tid],
(isc__netievent_t *)ievent);
isc__nm_put_netievent_tlsconnect(mgr, ievent);
} else {
nsock->tid = isc_random_uniform(mgr->nworkers);
isc__nm_enqueue_ievent(&mgr->workers[nsock->tid],
(isc__netievent_t *)ievent);
}
LOCK(&nsock->lock);
while (nsock->result == ISC_R_DEFAULT) {
WAIT(&nsock->cond, &nsock->lock);
}
atomic_store(&nsock->active, true);
BROADCAST(&nsock->scond);
UNLOCK(&nsock->lock);
INSIST(VALID_NMSOCK(nsock));
isc__nmsocket_detach(&tsock);
isc_nm_tcpconnect(mgr,
(isc_nmiface_t *)&nsock->tlsstream.local_iface.addr,
(isc_nmiface_t *)&peer->addr, tcp_connected, nsock,
nsock->connect_timeout, 0);
}
static void
@ -937,38 +905,11 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
REQUIRE(VALID_NMSOCK(tlssock));
REQUIRE(VALID_NMHANDLE(handle));
tlssock->tid = isc_nm_tid();
if (result != ISC_R_SUCCESS) {
goto error;
}
result = initialize_tls(tlssock, false);
if (result != ISC_R_SUCCESS) {
goto error;
}
tlssock->peer = isc_nmhandle_peeraddr(handle);
isc_nmhandle_attach(handle, &tlssock->outerhandle);
tls_do_bio(tlssock, NULL, NULL, false);
return;
error:
tlshandle = isc__nmhandle_get(tlssock, NULL, NULL);
atomic_store(&tlssock->closed, true);
tls_call_connect_cb(tlssock, tlshandle, result);
isc_nmhandle_detach(&tlshandle);
isc__nmsocket_detach(&tlssock);
}
void
isc__nm_async_tlsconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
isc__netievent_tlsconnect_t *ievent =
(isc__netievent_tlsconnect_t *)ev0;
isc_nmsocket_t *tlssock = ievent->sock;
isc_result_t result;
isc_nmhandle_t *tlshandle = NULL;
UNUSED(worker);
if (isc__nm_closing(tlssock)) {
result = ISC_R_CANCELED;
goto error;
@ -983,21 +924,20 @@ isc__nm_async_tlsconnect(isc__networker_t *worker, isc__netievent_t *ev0) {
goto error;
}
tlssock->tid = isc_nm_tid();
tlssock->tlsstream.state = TLS_INIT;
result = initialize_tls(tlssock, false);
if (result != ISC_R_SUCCESS) {
goto error;
}
tlssock->peer = isc_nmhandle_peeraddr(handle);
isc_nmhandle_attach(handle, &tlssock->outerhandle);
atomic_store(&tlssock->active, true);
isc_nm_tcpconnect(worker->mgr, (isc_nmiface_t *)&ievent->local,
(isc_nmiface_t *)&ievent->peer, tcp_connected,
tlssock, tlssock->connect_timeout, 0);
update_result(tlssock, ISC_R_SUCCESS);
tls_do_bio(tlssock, NULL, NULL, false);
return;
error:
tlshandle = isc__nmhandle_get(tlssock, NULL, NULL);
atomic_store(&tlssock->closed, true);
tls_call_connect_cb(tlssock, tlshandle, result);
update_result(tlssock, result);
isc_nmhandle_detach(&tlshandle);
isc__nmsocket_detach(&tlssock);
}

View File

@ -55,6 +55,7 @@ static uint64_t stop_magic = 0;
static uv_buf_t send_msg = { .base = (char *)&send_magic,
.len = sizeof(send_magic) };
static atomic_int_fast64_t active_cconnects = ATOMIC_VAR_INIT(0);
static atomic_int_fast64_t nsends = ATOMIC_VAR_INIT(0);
static atomic_int_fast64_t ssends = ATOMIC_VAR_INIT(0);
static atomic_int_fast64_t sreads = ATOMIC_VAR_INIT(0);
@ -119,10 +120,10 @@ connect_send_cb(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
REQUIRE(VALID_NMHANDLE(handle));
(void)atomic_fetch_sub(&active_cconnects, 1);
memmove(&data, arg, sizeof(data));
isc_mem_put(handle->sock->mgr->mctx, arg, sizeof(data));
if (result != ISC_R_SUCCESS) {
atomic_store(&slowdown, true);
goto error;
}
@ -139,6 +140,11 @@ error:
data.reply_cb(handle, result, NULL, data.cb_arg);
isc_mem_put(handle->sock->mgr->mctx, data.region.base,
data.region.length);
if (result == ISC_R_TOOMANYOPENFILES) {
atomic_store(&slowdown, true);
} else {
atomic_store(&was_error, true);
}
}
static void
@ -296,6 +302,7 @@ nm_setup(void **state) {
atomic_store(&sreads, 0);
atomic_store(&ssends, 0);
atomic_store(&ctimeouts, 0);
atomic_store(&active_cconnects, 0);
atomic_store(&was_error, false);
@ -382,9 +389,8 @@ doh_receive_reply_cb(isc_nmhandle_t *handle, isc_result_t eresult,
UNUSED(cbarg);
UNUSED(region);
(void)atomic_fetch_sub(&nsends, 1);
if (eresult == ISC_R_SUCCESS) {
(void)atomic_fetch_sub(&nsends, 1);
atomic_fetch_add(&csends, 1);
atomic_fetch_add(&creads, 1);
isc_nm_resumeread(handle);
@ -678,11 +684,13 @@ doh_timeout_recovery_GET(void **state) {
static void
doh_receive_send_reply_cb(isc_nmhandle_t *handle, isc_result_t eresult,
isc_region_t *region, void *cbarg) {
int_fast64_t sends = atomic_fetch_sub(&nsends, 1);
isc_nmhandle_t *thandle = NULL;
assert_non_null(handle);
UNUSED(region);
isc_nmhandle_attach(handle, &thandle);
if (eresult == ISC_R_SUCCESS) {
int_fast64_t sends = atomic_fetch_sub(&nsends, 1);
atomic_fetch_add(&csends, 1);
atomic_fetch_add(&creads, 1);
if (sends > 0) {
@ -694,12 +702,16 @@ doh_receive_send_reply_cb(isc_nmhandle_t *handle, isc_result_t eresult,
.base = (uint8_t *)send_msg.base,
.length = send_msg.len },
doh_receive_send_reply_cb, cbarg);
if (eresult == ISC_R_CANCELED) {
break;
}
assert_true(eresult == ISC_R_SUCCESS);
}
}
} else {
atomic_store(&was_error, true);
}
isc_nmhandle_detach(&thandle);
}
static isc_threadresult_t
@ -716,8 +728,9 @@ doh_connect_thread(isc_threadarg_t arg) {
* We need to back off and slow down if we start getting
* errors, to prevent a thundering herd problem.
*/
if (atomic_load(&slowdown)) {
isc_test_nap(1000 * workers);
int_fast64_t active = atomic_fetch_add(&active_cconnects, 1);
if (atomic_load(&slowdown) || active > workers) {
isc_test_nap(1000 * (active - workers));
atomic_store(&slowdown, false);
}
connect_send_request(
@ -1018,6 +1031,8 @@ doh_recv_half_send(void **state) {
size_t nthreads = ISC_MAX(ISC_MIN(workers, 32), 1);
isc_thread_t threads[32] = { 0 };
atomic_store(&nsends, (NSENDS * NWRITES) / 2);
result = isc_nm_listenhttp(
listen_nm, (isc_nmiface_t *)&tcp_listen_addr, 0, NULL,
atomic_load(&use_TLS) ? server_tlsctx : NULL, &listen_sock);
@ -1031,7 +1046,7 @@ doh_recv_half_send(void **state) {
isc_thread_create(doh_connect_thread, connect_nm, &threads[i]);
}
while (atomic_load(&nsends) >= (NSENDS * NWRITES) / 2) {
while (atomic_load(&nsends) > 0) {
isc_thread_yield();
}
@ -1092,6 +1107,8 @@ doh_half_recv_send(void **state) {
size_t nthreads = ISC_MAX(ISC_MIN(workers, 32), 1);
isc_thread_t threads[32] = { 0 };
atomic_store(&nsends, (NSENDS * NWRITES) / 2);
result = isc_nm_listenhttp(
listen_nm, (isc_nmiface_t *)&tcp_listen_addr, 0, NULL,
atomic_load(&use_TLS) ? server_tlsctx : NULL, &listen_sock);
@ -1105,7 +1122,7 @@ doh_half_recv_send(void **state) {
isc_thread_create(doh_connect_thread, connect_nm, &threads[i]);
}
while (atomic_load(&nsends) >= (NSENDS * NWRITES) / 2) {
while (atomic_load(&nsends) > 0) {
isc_thread_yield();
}
@ -1166,6 +1183,8 @@ doh_half_recv_half_send(void **state) {
size_t nthreads = ISC_MAX(ISC_MIN(workers, 32), 1);
isc_thread_t threads[32] = { 0 };
atomic_store(&nsends, (NSENDS * NWRITES) / 2);
result = isc_nm_listenhttp(
listen_nm, (isc_nmiface_t *)&tcp_listen_addr, 0, NULL,
atomic_load(&use_TLS) ? server_tlsctx : NULL, &listen_sock);
@ -1179,7 +1198,7 @@ doh_half_recv_half_send(void **state) {
isc_thread_create(doh_connect_thread, connect_nm, &threads[i]);
}
while (atomic_load(&nsends) >= (NSENDS * NWRITES) / 2) {
while (atomic_load(&nsends) > 0) {
isc_thread_yield();
}

View File

@ -2275,6 +2275,7 @@ static void
tls_half_recv_send_quota(void **state) {
SKIP_IN_CI;
stream_use_TLS = true;
atomic_store(&check_listener_quota, true);
stream_half_recv_send(state);
}
@ -2282,6 +2283,7 @@ static void
tls_half_recv_half_send_quota(void **state) {
SKIP_IN_CI;
stream_use_TLS = true;
atomic_store(&check_listener_quota, true);
stream_half_recv_half_send(state);
}