From a7a482c1b17ae3063b631d0d5de1fc8c924788ec Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 22 Feb 2022 17:07:57 +0200 Subject: [PATCH 1/8] Add isc_tlsctx_attach() The implementation is done on top of the reference counting functionality found in OpenSSL/LibreSSL, which allows for avoiding wrapping the object. Adding this function allows using reference counting for TLS contexts in BIND 9's codebase. --- configure.ac | 1 + lib/isc/include/isc/tls.h | 11 +++++++++++ lib/isc/openssl_shim.c | 7 +++++++ lib/isc/openssl_shim.h | 5 +++++ lib/isc/tls.c | 10 ++++++++++ 5 files changed, 34 insertions(+) diff --git a/configure.ac b/configure.ac index 14f176bf3a..1ae38d2ef0 100644 --- a/configure.ac +++ b/configure.ac @@ -651,6 +651,7 @@ AC_CHECK_FUNCS([SSL_CTX_set_min_proto_version]) AC_CHECK_FUNCS([SSL_CTX_up_ref]) AC_CHECK_FUNCS([SSL_read_ex SSL_peek_ex SSL_write_ex]) AC_CHECK_FUNCS([SSL_CTX_set1_cert_store X509_STORE_up_ref]) +AC_CHECK_FUNCS([SSL_CTX_up_ref]) # # Check for algorithm support in OpenSSL diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h index fc75157bbe..e05e2c4228 100644 --- a/lib/isc/include/isc/tls.h +++ b/lib/isc/include/isc/tls.h @@ -32,6 +32,17 @@ isc_tlsctx_free(isc_tlsctx_t **ctpx); *\li 'ctxp' != NULL and '*ctxp' != NULL. */ +void +isc_tlsctx_attach(isc_tlsctx_t *src, isc_tlsctx_t **ptarget); +/*%< + * Attach to the TLS context. + * + * Requires: + *\li 'src' != NULL; + *\li 'ptarget' != NULL; + *\li '*ptarget' == NULL. + */ + isc_result_t isc_tlsctx_createserver(const char *keyfile, const char *certfile, isc_tlsctx_t **ctxp); diff --git a/lib/isc/openssl_shim.c b/lib/isc/openssl_shim.c index 3d6cbeed89..c39ba8c682 100644 --- a/lib/isc/openssl_shim.c +++ b/lib/isc/openssl_shim.c @@ -189,3 +189,10 @@ SSL_CTX_set1_cert_store(SSL_CTX *ctx, X509_STORE *store) { } #endif /* !HAVE_SSL_CTX_SET1_CERT_STORE */ + +#if !HAVE_SSL_CTX_UP_REF +int +SSL_CTX_up_ref(SSL_CTX *ctx) { + return (CRYPTO_add(&ctx->references, 1, CRYPTO_LOCK_SSL_CTX) > 0); +} +#endif /* !HAVE_SSL_CTX_UP_REF */ diff --git a/lib/isc/openssl_shim.h b/lib/isc/openssl_shim.h index 0755fbb49d..b2916e20a9 100644 --- a/lib/isc/openssl_shim.h +++ b/lib/isc/openssl_shim.h @@ -130,3 +130,8 @@ X509_STORE_up_ref(X509_STORE *v); void SSL_CTX_set1_cert_store(SSL_CTX *ctx, X509_STORE *store); #endif /* !HAVE_SSL_CTX_SET1_CERT_STORE */ + +#if !HAVE_SSL_CTX_UP_REF +int +SSL_CTX_up_ref(SSL_CTX *store); +#endif /* !HAVE_SSL_CTX_UP_REF */ diff --git a/lib/isc/tls.c b/lib/isc/tls.c index 19bed66efb..3eb0af155f 100644 --- a/lib/isc/tls.c +++ b/lib/isc/tls.c @@ -188,6 +188,16 @@ isc_tlsctx_free(isc_tlsctx_t **ctxp) { SSL_CTX_free(ctx); } +void +isc_tlsctx_attach(isc_tlsctx_t *src, isc_tlsctx_t **ptarget) { + REQUIRE(src != NULL); + REQUIRE(ptarget != NULL && *ptarget == NULL); + + RUNTIME_CHECK(SSL_CTX_up_ref(src) == 1); + + *ptarget = src; +} + #if HAVE_SSL_CTX_SET_KEYLOG_CALLBACK /* * Callback invoked by the SSL library whenever a new TLS pre-master secret From b52d46612fb104a2eef37e02380ea0f0d6bdcac5 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 22 Feb 2022 21:22:04 +0200 Subject: [PATCH 2/8] Use isc_tlsctx_attach() in TLS stream code This commit adds proper reference counting for TLS contexts into generic TLS stream code. --- lib/isc/netmgr/tlsstream.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c index d0c4868fae..dfff57f0dd 100644 --- a/lib/isc/netmgr/tlsstream.c +++ b/lib/isc/netmgr/tlsstream.c @@ -604,7 +604,8 @@ tlslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { &handle->sock->iface); /* We need to initialize SSL now to reference SSL_CTX properly */ - tlssock->tlsstream.ctx = tlslistensock->tlsstream.ctx; + isc_tlsctx_attach(tlslistensock->tlsstream.ctx, + &tlssock->tlsstream.ctx); tlssock->tlsstream.tls = isc_tls_create(tlssock->tlsstream.ctx); if (tlssock->tlsstream.tls == NULL) { atomic_store(&tlssock->closed, true); @@ -618,8 +619,6 @@ tlslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { tlssock->read_timeout = atomic_load(&handle->sock->mgr->init); tlssock->tid = isc_nm_tid(); - tlssock->tlsstream.ctx = tlslistensock->tlsstream.ctx; - result = initialize_tls(tlssock, true); RUNTIME_CHECK(result == ISC_R_SUCCESS); /* TODO: catch failure code, detach tlssock, and log the error */ @@ -644,7 +643,7 @@ isc_nm_listentls(isc_nm_t *mgr, isc_sockaddr_t *iface, tlssock->result = ISC_R_UNSET; tlssock->accept_cb = accept_cb; tlssock->accept_cbarg = accept_cbarg; - tlssock->tlsstream.ctx = sslctx; + isc_tlsctx_attach(sslctx, &tlssock->tlsstream.ctx); tlssock->tlsstream.tls = NULL; /* @@ -868,7 +867,7 @@ isc__nm_tls_stoplistening(isc_nmsocket_t *sock) { sock->recv_cbarg = NULL; if (sock->tlsstream.tls != NULL) { isc_tls_free(&sock->tlsstream.tls); - sock->tlsstream.ctx = NULL; + isc_tlsctx_free(&sock->tlsstream.ctx); } if (sock->outer != NULL) { @@ -898,7 +897,7 @@ isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, nsock->connect_cb = cb; nsock->connect_cbarg = cbarg; nsock->connect_timeout = timeout; - nsock->tlsstream.ctx = ctx; + isc_tlsctx_attach(ctx, &nsock->tlsstream.ctx); isc_nm_tcpconnect(mgr, local, peer, tcp_connected, nsock, nsock->connect_timeout); @@ -1011,13 +1010,19 @@ isc__nm_tls_cleanup_data(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock->tlsstream.tlslistener)); isc__nmsocket_detach(&sock->tlsstream.tlslistener); } else if (sock->type == isc_nm_tlssocket) { + if (sock->tlsstream.ctx != NULL) { + isc_tlsctx_free(&sock->tlsstream.ctx); + } if (sock->tlsstream.tls != NULL) { isc_tls_free(&sock->tlsstream.tls); /* These are destroyed when we free SSL */ - sock->tlsstream.ctx = NULL; sock->tlsstream.bio_out = NULL; sock->tlsstream.bio_in = NULL; } + } else if (sock->type == isc_nm_tlslistener) { + if (sock->tlsstream.ctx != NULL) { + isc_tlsctx_free(&sock->tlsstream.ctx); + } } } From 9256026d18eac53c74567ecd51acfe3f269aa329 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 16 Mar 2022 17:02:46 +0200 Subject: [PATCH 3/8] Use isc_tlsctx_attach() in TLS DNS code This commit adds proper reference counting for TLS contexts into generic TLS DNS (DoT) code. --- lib/isc/netmgr/netmgr-int.h | 3 +++ lib/isc/netmgr/netmgr.c | 1 + lib/isc/netmgr/tlsdns.c | 25 ++++++++++++++++++++----- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 5ff067a425..1ce685cc0f 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -1620,6 +1620,9 @@ isc__nm_tlsdns_xfr_allowed(isc_nmsocket_t *sock); * \li 'sock' is a valid TLSDNS socket. */ +void +isc__nm_tlsdns_cleanup_data(isc_nmsocket_t *sock); + #if HAVE_LIBNGHTTP2 void isc__nm_tls_send(isc_nmhandle_t *handle, const isc_region_t *region, diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index c52094350f..66b77939c5 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -1219,6 +1219,7 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) { isc_condition_destroy(&sock->scond); isc_condition_destroy(&sock->cond); isc_mutex_destroy(&sock->lock); + isc__nm_tlsdns_cleanup_data(sock); #if HAVE_LIBNGHTTP2 isc__nm_tls_cleanup_data(sock); isc__nm_http_cleanup_data(sock); diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index 0b67472795..021647f5b1 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -336,7 +336,7 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, sock->connect_timeout = timeout; sock->result = ISC_R_UNSET; - sock->tls.ctx = sslctx; + isc_tlsctx_attach(sslctx, &sock->tls.ctx); atomic_init(&sock->client, true); atomic_init(&sock->connecting, true); @@ -438,7 +438,7 @@ start_tlsdns_child(isc_nm_t *mgr, isc_sockaddr_t *iface, isc_nmsocket_t *sock, csock->recv_cbarg = sock->recv_cbarg; csock->backlog = sock->backlog; csock->tid = tid; - csock->tls.ctx = sock->tls.ctx; + isc_tlsctx_attach(sock->tls.ctx, &csock->tls.ctx); /* * We don't attach to quota, just assign - to avoid @@ -499,7 +499,7 @@ isc_nm_listentlsdns(isc_nm_t *mgr, isc_sockaddr_t *iface, sock->backlog = backlog; sock->pquota = quota; - sock->tls.ctx = sslctx; + isc_tlsctx_attach(sslctx, &sock->tls.ctx); sock->tid = 0; sock->fd = -1; @@ -1788,7 +1788,9 @@ tlsdns_stop_cb(uv_handle_t *handle) { BIO_free_all(sock->tls.app_rbio); BIO_free_all(sock->tls.app_wbio); - sock->tls.ctx = NULL; + if (sock->tls.ctx != NULL) { + isc_tlsctx_free(&sock->tls.ctx); + } isc__nmsocket_detach(&sock); } @@ -1819,7 +1821,9 @@ tlsdns_close_sock(isc_nmsocket_t *sock) { BIO_free_all(sock->tls.app_rbio); BIO_free_all(sock->tls.app_wbio); - sock->tls.ctx = NULL; + if (sock->tls.ctx != NULL) { + isc_tlsctx_free(&sock->tls.ctx); + } isc__nmsocket_prep_destroy(sock); } @@ -2110,3 +2114,14 @@ isc__nm_tlsdns_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { return (isc_tls_verify_peer_result_string(sock->tls.tls)); } + + +void +isc__nm_tlsdns_cleanup_data(isc_nmsocket_t *sock) { + if ((sock->type == isc_nm_tlsdnslistener || + sock->type == isc_nm_tlsdnssocket) && + sock->tls.ctx != NULL) + { + isc_tlsctx_free(&sock->tls.ctx); + } +} \ No newline at end of file From 25609156a555cad8f5fcfb886536aa06849ffed0 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 29 Mar 2022 20:42:16 +0300 Subject: [PATCH 4/8] Maintain a per-thread TLS ctx reference in TLS stream code This commit changes the generic TLS stream code to maintain a per-worker thread TLS context reference. --- lib/isc/netmgr/netmgr-int.h | 2 + lib/isc/netmgr/tlsstream.c | 85 ++++++++++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 1ce685cc0f..a8a7e93ad4 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -928,6 +928,8 @@ struct isc_nmsocket { BIO *bio_out; isc_tls_t *tls; isc_tlsctx_t *ctx; + isc_tlsctx_t **listener_tls_ctx; /*%< A context reference per + worker */ isc_nmsocket_t *tlslistener; atomic_bool result_updated; enum { diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c index dfff57f0dd..0a46214a03 100644 --- a/lib/isc/netmgr/tlsstream.c +++ b/lib/isc/netmgr/tlsstream.c @@ -77,6 +77,15 @@ tls_close_direct(isc_nmsocket_t *sock); static void async_tls_do_bio(isc_nmsocket_t *sock); +static void +tls_init_listener_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *ctx); + +static void +tls_cleanup_listener_tlsctx(isc_nmsocket_t *listener); + +static isc_tlsctx_t * +tls_get_listener_tlsctx(isc_nmsocket_t *listener, const int tid); + /* * The socket is closing, outerhandle has been detached, listener is * inactive, or the netmgr is closing: any operation on it should abort @@ -585,6 +594,8 @@ static isc_result_t tlslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc_nmsocket_t *tlslistensock = (isc_nmsocket_t *)cbarg; isc_nmsocket_t *tlssock = NULL; + isc_tlsctx_t *tlsctx = NULL; + int tid; /* If accept() was unsuccessful we can't do anything */ if (result != ISC_R_SUCCESS) { @@ -603,12 +614,15 @@ tlslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc__nmsocket_init(tlssock, handle->sock->mgr, isc_nm_tlssocket, &handle->sock->iface); + tid = isc_nm_tid(); /* We need to initialize SSL now to reference SSL_CTX properly */ - isc_tlsctx_attach(tlslistensock->tlsstream.ctx, - &tlssock->tlsstream.ctx); + tlsctx = tls_get_listener_tlsctx(tlslistensock, tid); + RUNTIME_CHECK(tlsctx != NULL); + isc_tlsctx_attach(tlsctx, &tlssock->tlsstream.ctx); tlssock->tlsstream.tls = isc_tls_create(tlssock->tlsstream.ctx); if (tlssock->tlsstream.tls == NULL) { atomic_store(&tlssock->closed, true); + isc_tlsctx_free(&tlssock->tlsstream.ctx); isc__nmsocket_detach(&tlssock); return (ISC_R_TLSERROR); } @@ -617,7 +631,7 @@ tlslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc_nmhandle_attach(handle, &tlssock->outerhandle); tlssock->peer = handle->sock->peer; tlssock->read_timeout = atomic_load(&handle->sock->mgr->init); - tlssock->tid = isc_nm_tid(); + tlssock->tid = tid; result = initialize_tls(tlssock, true); RUNTIME_CHECK(result == ISC_R_SUCCESS); @@ -643,7 +657,7 @@ isc_nm_listentls(isc_nm_t *mgr, isc_sockaddr_t *iface, tlssock->result = ISC_R_UNSET; tlssock->accept_cb = accept_cb; tlssock->accept_cbarg = accept_cbarg; - isc_tlsctx_attach(sslctx, &tlssock->tlsstream.ctx); + tls_init_listener_tlsctx(tlssock, sslctx); tlssock->tlsstream.tls = NULL; /* @@ -865,10 +879,9 @@ isc__nm_tls_stoplistening(isc_nmsocket_t *sock) { atomic_store(&sock->closed, true); sock->recv_cb = NULL; sock->recv_cbarg = NULL; - if (sock->tlsstream.tls != NULL) { - isc_tls_free(&sock->tlsstream.tls); - isc_tlsctx_free(&sock->tlsstream.ctx); - } + + INSIST(sock->tlsstream.tls == NULL); + INSIST(sock->tlsstream.ctx == NULL); if (sock->outer != NULL) { isc_nm_stoplistening(sock->outer); @@ -1020,9 +1033,7 @@ isc__nm_tls_cleanup_data(isc_nmsocket_t *sock) { sock->tlsstream.bio_in = NULL; } } else if (sock->type == isc_nm_tlslistener) { - if (sock->tlsstream.ctx != NULL) { - isc_tlsctx_free(&sock->tlsstream.ctx); - } + tls_cleanup_listener_tlsctx(sock); } } @@ -1087,3 +1098,55 @@ isc__nm_tls_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { return (isc_tls_verify_peer_result_string(sock->tlsstream.tls)); } + +static void +tls_init_listener_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *ctx) { + uint32_t nworkers; + + REQUIRE(VALID_NM(listener->mgr)); + + nworkers = isc_nm_getnworkers(listener->mgr); + + REQUIRE(nworkers > 0); + REQUIRE(ctx != NULL); + + listener->tlsstream.listener_tls_ctx = isc_mem_get( + listener->mgr->mctx, sizeof(isc_tlsctx_t *) * nworkers); + for (size_t i = 0; i < nworkers; i++) { + listener->tlsstream.listener_tls_ctx[i] = NULL; + isc_tlsctx_attach(ctx, + &listener->tlsstream.listener_tls_ctx[i]); + } +} + +static void +tls_cleanup_listener_tlsctx(isc_nmsocket_t *listener) { + uint32_t nworkers; + + REQUIRE(VALID_NM(listener->mgr)); + + nworkers = isc_nm_getnworkers(listener->mgr); + + REQUIRE(nworkers > 0); + if (listener->tlsstream.listener_tls_ctx == NULL) { + return; + } + + for (size_t i = 0; i < nworkers; i++) { + isc_tlsctx_free(&listener->tlsstream.listener_tls_ctx[i]); + } + isc_mem_put(listener->mgr->mctx, listener->tlsstream.listener_tls_ctx, + sizeof(isc_tlsctx_t *) * nworkers); +} + +static isc_tlsctx_t * +tls_get_listener_tlsctx(isc_nmsocket_t *listener, const int tid) { + REQUIRE(VALID_NM(listener->mgr)); + REQUIRE(tid >= 0); + + if (listener->tlsstream.listener_tls_ctx == NULL) { + return (NULL); + } + + return (listener->tlsstream.listener_tls_ctx[tid]); +} From df317184eba2912f7a249d1c7e186e5e88f19b09 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Thu, 31 Mar 2022 13:47:48 +0300 Subject: [PATCH 5/8] Add isc_nmsocket_set_tlsctx() This commit adds isc_nmsocket_set_tlsctx() - an asynchronous function that replaces the TLS context within a given TLS-enabled listener socket object. It is based on the newly added reference counting functionality. The intention of adding this function is to add functionality to replace a TLS context without recreating the whole socket object, including the underlying TCP listener socket, as a BIND process might not have enough permissions to re-create it fully on reconfiguration. --- lib/isc/include/isc/netmgr.h | 12 +++++++ lib/isc/netmgr/http.c | 8 +++++ lib/isc/netmgr/netmgr-int.h | 53 ++++++++++++++++++++++++++++ lib/isc/netmgr/netmgr.c | 68 ++++++++++++++++++++++++++++++++++++ lib/isc/netmgr/tlsdns.c | 10 +++++- lib/isc/netmgr/tlsstream.c | 30 +++++++++------- 6 files changed, 168 insertions(+), 13 deletions(-) diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 0ae582a0b5..4651bbf4cf 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -109,6 +109,18 @@ isc_nmsocket_close(isc_nmsocket_t **sockp); * sockets with active handles, the socket will be closed. */ +void +isc_nmsocket_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx); +/*%< + * Asynchronously replace the TLS context 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 'tlsctx' is a valid pointer to a TLS context object. + */ + #ifdef NETMGR_TRACE #define isc_nmhandle_attach(handle, dest) \ isc__nmhandle_attach(handle, dest, __FILE__, __LINE__, __func__) diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index 6684373a68..1d59ee3c30 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -2936,6 +2936,14 @@ isc__nm_http_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { return (isc_nm_verify_tls_peer_result_string(session->handle)); } +void +isc__nm_http_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx) { + REQUIRE(VALID_NMSOCK(listener)); + REQUIRE(listener->type == isc_nm_httplistener); + + isc_nmsocket_set_tlsctx(listener->outer, tlsctx); +} + 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 a8a7e93ad4..9adb7e0084 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -328,6 +328,8 @@ typedef enum isc__netievent_type { netievent_task, + netievent_settlsctx, + /* * event type values higher than this will be treated * as high-priority events, which can be processed @@ -665,6 +667,38 @@ typedef struct isc__netievent { isc__nm_put_netievent(nm, ievent); \ } +typedef struct isc__netievent__tlsctx { + NETIEVENT__SOCKET; + isc_tlsctx_t *tlsctx; +} isc__netievent__tlsctx_t; + +#define NETIEVENT_SOCKET_TLSCTX_TYPE(type) \ + typedef isc__netievent__tlsctx_t isc__netievent_##type##_t; + +#define NETIEVENT_SOCKET_TLSCTX_DECL(type) \ + isc__netievent_##type##_t *isc__nm_get_netievent_##type( \ + isc_nm_t *nm, isc_nmsocket_t *sock, isc_tlsctx_t *tlsctx); \ + void isc__nm_put_netievent_##type(isc_nm_t *nm, \ + isc__netievent_##type##_t *ievent); + +#define NETIEVENT_SOCKET_TLSCTX_DEF(type) \ + isc__netievent_##type##_t *isc__nm_get_netievent_##type( \ + isc_nm_t *nm, isc_nmsocket_t *sock, isc_tlsctx_t *tlsctx) { \ + isc__netievent_##type##_t *ievent = \ + isc__nm_get_netievent(nm, netievent_##type); \ + isc__nmsocket_attach(sock, &ievent->sock); \ + isc_tlsctx_attach(tlsctx, &ievent->tlsctx); \ + \ + return (ievent); \ + } \ + \ + void isc__nm_put_netievent_##type(isc_nm_t *nm, \ + isc__netievent_##type##_t *ievent) { \ + isc_tlsctx_free(&ievent->tlsctx); \ + isc__nmsocket_detach(&ievent->sock); \ + isc__nm_put_netievent(nm, ievent); \ + } + typedef union { isc__netievent_t ni; isc__netievent__socket_t nis; @@ -672,6 +706,7 @@ typedef union { isc__netievent_udpsend_t nius; isc__netievent__socket_quota_t nisq; isc__netievent_tlsconnect_t nitc; + isc__netievent__tlsctx_t nitls; } isc__netievent_storage_t; /* @@ -930,6 +965,7 @@ struct isc_nmsocket { isc_tlsctx_t *ctx; isc_tlsctx_t **listener_tls_ctx; /*%< A context reference per worker */ + size_t n_listener_tls_ctx; isc_nmsocket_t *tlslistener; atomic_bool result_updated; enum { @@ -1608,6 +1644,9 @@ void isc__nm_async_tlsdnsshutdown(isc__networker_t *worker, isc__netievent_t *ev0); void isc__nm_async_tlsdnsread(isc__networker_t *worker, isc__netievent_t *ev0); +void +isc__nm_async_tlsdns_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx, + const int tid); /*%< * Callback handlers for asynchronous TLSDNS events. */ @@ -1684,6 +1723,10 @@ isc__nmhandle_tls_keepalive(isc_nmhandle_t *handle, bool value); * Set the keepalive value on the underlying TCP handle. */ +void +isc__nm_async_tls_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx, + const int tid); + void isc__nm_http_stoplistening(isc_nmsocket_t *sock); @@ -1769,8 +1812,14 @@ isc__nm_httpsession_attach(isc_nm_http_session_t *source, void isc__nm_httpsession_detach(isc_nm_http_session_t **sessionp); +void +isc__nm_http_set_tlsctx(isc_nmsocket_t *sock, isc_tlsctx_t *tlsctx); + #endif +void +isc__nm_async_settlsctx(isc__networker_t *worker, isc__netievent_t *ev0); + #define isc__nm_uverr2result(x) \ isc___nm_uverr2result(x, true, __FILE__, __LINE__, __func__) isc_result_t @@ -1963,6 +2012,8 @@ NETIEVENT_TYPE(stop); NETIEVENT_TASK_TYPE(task); +NETIEVENT_SOCKET_TLSCTX_TYPE(settlsctx); + /* Now declared the helper functions */ NETIEVENT_SOCKET_DECL(close); @@ -2030,6 +2081,8 @@ NETIEVENT_DECL(stop); NETIEVENT_TASK_DECL(task); +NETIEVENT_SOCKET_TLSCTX_DECL(settlsctx); + void isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result); void diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 66b77939c5..a73223942c 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -903,6 +903,7 @@ process_netievent(isc__networker_t *worker, isc__netievent_t *ievent) { NETIEVENT_CASE(httpsend); NETIEVENT_CASE(httpclose); #endif + NETIEVENT_CASE(settlsctx); NETIEVENT_CASE(connectcb); NETIEVENT_CASE(readcb); @@ -1041,6 +1042,8 @@ NETIEVENT_DEF(stop); NETIEVENT_TASK_DEF(task); +NETIEVENT_SOCKET_TLSCTX_DEF(settlsctx); + void isc__nm_maybe_enqueue_ievent(isc__networker_t *worker, isc__netievent_t *event) { @@ -3554,6 +3557,71 @@ isc_nm_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { return (NULL); } +void +isc__nm_async_settlsctx(isc__networker_t *worker, isc__netievent_t *ev0) { + isc__netievent__tlsctx_t *ev_tlsctx = (isc__netievent__tlsctx_t *)ev0; + const int tid = isc_nm_tid(); + isc_nmsocket_t *listener = ev_tlsctx->sock; + isc_tlsctx_t *tlsctx = ev_tlsctx->tlsctx; + + UNUSED(worker); + + switch (listener->type) { + case isc_nm_tlsdnslistener: + isc__nm_async_tlsdns_set_tlsctx(listener, tlsctx, tid); + break; +#if HAVE_LIBNGHTTP2 + case isc_nm_tlslistener: + isc__nm_async_tls_set_tlsctx(listener, tlsctx, tid); + break; +#endif /* HAVE_LIBNGHTTP2 */ + default: + UNREACHABLE(); + break; + }; +} + +static void +set_tlsctx_workers(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx) { + /* Update the TLS context reference for every worker thread. */ + for (size_t i = 0; i < isc_nm_getnworkers(listener->mgr); i++) { + isc__netievent__tlsctx_t *ievent = + isc__nm_get_netievent_settlsctx(listener->mgr, listener, + tlsctx); + isc__nm_enqueue_ievent(&listener->mgr->workers[i], + (isc__netievent_t *)ievent); + } +} + +void +isc_nmsocket_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx) { + REQUIRE(VALID_NMSOCK(listener)); + REQUIRE(tlsctx != NULL); + + switch (listener->type) { +#if HAVE_LIBNGHTTP2 + case isc_nm_httplistener: + /* + * We handle HTTP listener sockets differently, as they rely + * on underlying TLS sockets for networking. The TLS context + * will get passed to these underlying sockets via the call to + * isc__nm_http_set_tlsctx(). + */ + isc__nm_http_set_tlsctx(listener, tlsctx); + break; + case isc_nm_tlslistener: + set_tlsctx_workers(listener, tlsctx); + break; +#endif /* HAVE_LIBNGHTTP2 */ + case isc_nm_tlsdnslistener: + set_tlsctx_workers(listener, tlsctx); + break; + default: + UNREACHABLE(); + break; + }; +} + #ifdef NETMGR_TRACE /* * Dump all active sockets in netmgr. We output to stderr diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index 021647f5b1..96612c1a85 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -2115,6 +2115,14 @@ isc__nm_tlsdns_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { return (isc_tls_verify_peer_result_string(sock->tls.tls)); } +void +isc__nm_async_tlsdns_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx, + const int tid) { + REQUIRE(tid >= 0); + + isc_tlsctx_free(&listener->children[tid].tls.ctx); + isc_tlsctx_attach(tlsctx, &listener->children[tid].tls.ctx); +} void isc__nm_tlsdns_cleanup_data(isc_nmsocket_t *sock) { @@ -2124,4 +2132,4 @@ isc__nm_tlsdns_cleanup_data(isc_nmsocket_t *sock) { { isc_tlsctx_free(&sock->tls.ctx); } -} \ No newline at end of file +} diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c index 0a46214a03..9ec072897e 100644 --- a/lib/isc/netmgr/tlsstream.c +++ b/lib/isc/netmgr/tlsstream.c @@ -1101,17 +1101,17 @@ isc__nm_tls_verify_tls_peer_result_string(const isc_nmhandle_t *handle) { static void tls_init_listener_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *ctx) { - uint32_t nworkers; + size_t nworkers; REQUIRE(VALID_NM(listener->mgr)); - - nworkers = isc_nm_getnworkers(listener->mgr); - - REQUIRE(nworkers > 0); REQUIRE(ctx != NULL); + nworkers = (size_t)isc_nm_getnworkers(listener->mgr); + INSIST(nworkers > 0); + listener->tlsstream.listener_tls_ctx = isc_mem_get( listener->mgr->mctx, sizeof(isc_tlsctx_t *) * nworkers); + listener->tlsstream.n_listener_tls_ctx = nworkers; for (size_t i = 0; i < nworkers; i++) { listener->tlsstream.listener_tls_ctx[i] = NULL; isc_tlsctx_attach(ctx, @@ -1121,22 +1121,19 @@ tls_init_listener_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *ctx) { static void tls_cleanup_listener_tlsctx(isc_nmsocket_t *listener) { - uint32_t nworkers; - REQUIRE(VALID_NM(listener->mgr)); - nworkers = isc_nm_getnworkers(listener->mgr); - - REQUIRE(nworkers > 0); if (listener->tlsstream.listener_tls_ctx == NULL) { return; } - for (size_t i = 0; i < nworkers; i++) { + for (size_t i = 0; i < listener->tlsstream.n_listener_tls_ctx; i++) { isc_tlsctx_free(&listener->tlsstream.listener_tls_ctx[i]); } isc_mem_put(listener->mgr->mctx, listener->tlsstream.listener_tls_ctx, - sizeof(isc_tlsctx_t *) * nworkers); + sizeof(isc_tlsctx_t *) * + listener->tlsstream.n_listener_tls_ctx); + listener->tlsstream.n_listener_tls_ctx = 0; } static isc_tlsctx_t * @@ -1150,3 +1147,12 @@ tls_get_listener_tlsctx(isc_nmsocket_t *listener, const int tid) { return (listener->tlsstream.listener_tls_ctx[tid]); } + +void +isc__nm_async_tls_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx, + const int tid) { + REQUIRE(tid >= 0); + + isc_tlsctx_free(&listener->tlsstream.listener_tls_ctx[tid]); + isc_tlsctx_attach(tlsctx, &listener->tlsstream.listener_tls_ctx[tid]); +} From 77b2db82468d4b49ad22455ce738e7a065c0f2cd Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Thu, 31 Mar 2022 13:48:14 +0300 Subject: [PATCH 6/8] Replace listener TLS contexts on reconfiguration This commit makes use of isc_nmsocket_set_tlsctx(). Now, instead of recreating TLS-enabled listeners (including the underlying TCP listener sockets), only the TLS context in use is replaced. --- lib/ns/interfacemgr.c | 129 +++++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 66 deletions(-) diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index d3bdb953e4..395aa04e1b 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -909,6 +909,25 @@ clearlistenon(ns_interfacemgr_t *mgr) { } } +static void +replace_listener_tlsctx(ns_interfacemgr_t *mgr, 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); + if (ifp->tcplistensocket != NULL) { + /* 'tcplistensocket' is used for DoT */ + isc_nmsocket_set_tlsctx(ifp->tcplistensocket, newctx); + } else if (ifp->http_secure_listensocket != NULL) { + isc_nmsocket_set_tlsctx(ifp->http_secure_listensocket, newctx); + } + UNLOCK(&mgr->lock); +} + static isc_result_t do_scan(ns_interfacemgr_t *mgr, bool verbose, bool config) { isc_interfaceiter_t *iter = NULL; @@ -976,42 +995,30 @@ do_scan(ns_interfacemgr_t *mgr, bool verbose, bool config) { ifp = find_matching_interface(mgr, &listen_addr); if (ifp != NULL) { - /* - * We need to recreate the TLS/HTTPS listeners - * during reconfiguration because the - * certificates could have been changed. - */ - if (config && LISTENING(ifp) && - le->sslctx != NULL) { - INSIST(NS_INTERFACE_VALID(ifp)); - LOCK(&mgr->lock); - isc_sockaddr_format(&ifp->addr, sabuf, + ifp->generation = mgr->generation; + if (le->dscp != -1 && ifp->dscp == -1) { + ifp->dscp = le->dscp; + } else if (le->dscp != ifp->dscp) { + isc_sockaddr_format(&listen_addr, sabuf, sizeof(sabuf)); isc_log_write(IFMGR_COMMON_LOGARGS, - ISC_LOG_INFO, - "no longer listening on " - "%s", - sabuf); - interface_destroy(&ifp); - UNLOCK(&mgr->lock); - } else { - ifp->generation = mgr->generation; - if (le->dscp != -1 && ifp->dscp == -1) { - ifp->dscp = le->dscp; - } else if (le->dscp != ifp->dscp) { - isc_sockaddr_format( - &listen_addr, sabuf, - sizeof(sabuf)); - isc_log_write( - IFMGR_COMMON_LOGARGS, - ISC_LOG_WARNING, - "%s: conflicting DSCP " - "values, using %d", - sabuf, ifp->dscp); - } - if (LISTENING(ifp)) { - continue; + ISC_LOG_WARNING, + "%s: conflicting DSCP " + "values, using %d", + 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); } + continue; } } @@ -1152,42 +1159,32 @@ do_scan(ns_interfacemgr_t *mgr, bool verbose, bool config) { ifp = find_matching_interface(mgr, &listen_sockaddr); if (ifp != NULL) { - /* - * We need to recreate the TLS/HTTPS listeners - * during a reconfiguration because the - * certificates could have been changed. - */ - if (config && LISTENING(ifp) && - le->sslctx != NULL) { - INSIST(NS_INTERFACE_VALID(ifp)); - LOCK(&mgr->lock); - isc_sockaddr_format(&ifp->addr, sabuf, + ifp->generation = mgr->generation; + if (le->dscp != -1 && ifp->dscp == -1) { + ifp->dscp = le->dscp; + } else if (le->dscp != ifp->dscp) { + isc_sockaddr_format(&listen_sockaddr, + sabuf, sizeof(sabuf)); isc_log_write(IFMGR_COMMON_LOGARGS, - ISC_LOG_INFO, - "no longer listening on " - "%s", - sabuf); - interface_destroy(&ifp); - UNLOCK(&mgr->lock); - } else { - ifp->generation = mgr->generation; - if (le->dscp != -1 && ifp->dscp == -1) { - ifp->dscp = le->dscp; - } else if (le->dscp != ifp->dscp) { - isc_sockaddr_format( - &listen_sockaddr, sabuf, - sizeof(sabuf)); - isc_log_write( - IFMGR_COMMON_LOGARGS, - ISC_LOG_WARNING, - "%s: conflicting DSCP " - "values, using %d", - sabuf, ifp->dscp); - } - if (LISTENING(ifp)) { - continue; + ISC_LOG_WARNING, + "%s: conflicting DSCP " + "values, using %d", + 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); } + + continue; } } From a100c1ff7cf7789f6c803216d261b961646f2847 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Tue, 5 Apr 2022 18:36:19 +0300 Subject: [PATCH 7/8] Update CHANGES [GL #3122] Add an entry that reloading TLS certificates without destroying underlying TCP listening sockets. --- CHANGES | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES b/CHANGES index 18d0aa9b07..c6fb175f7e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +5854. [func] Implement reference counting for TLS contexts and + allow reloading of TLS certificates on reconfiguration + without destroying the underlying TCP listener sockets + for TLS-based DNS transports. [GL #3122] + 5853. [bug] When using both the `+qr` and `+y` options `dig` could crash if the connection to the first server was not successful. [GL #3244] From 8bec4a6bf60a05418679caf2e2fb7f9cbe8d5ae9 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 6 Apr 2022 16:10:05 +0300 Subject: [PATCH 8/8] Extend the doth system test This commit adds simple checks that the TLS contexts in question are indeed being updated on DoT and DoH listeners. --- bin/tests/system/doth/tests.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bin/tests/system/doth/tests.sh b/bin/tests/system/doth/tests.sh index a92baeff0a..45170aba4a 100644 --- a/bin/tests/system/doth/tests.sh +++ b/bin/tests/system/doth/tests.sh @@ -597,10 +597,17 @@ grep "ANSWER: 2500" dig.out.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) +wait_for_tlsctx_update_ns4 () { + grep "updating TLS context on 10.53.0.4#${HTTPSPORT}" ns4/named.run > /dev/null || return 1 + grep "updating TLS context on 10.53.0.4#${TLSPORT}" ns4/named.run > /dev/null || return 1 + return 0 +} + n=$((n + 1)) echo_i "doing rndc reconfig to see that queries keep being served after that ($n)" ret=0 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))