From b109fa919283cc7bdd992e94d94b66e13d44f1a1 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Mon, 4 Dec 2023 14:28:28 +0200 Subject: [PATCH] Fix TLS certs store deletion on concurrent access During initialisation or reconfiguration, it is possible that multiple threads are trying to create a TLS context and associated data (like TLS certs store) concurrently. In some cases, a thread might be too late to add newly created data to the TLS contexts cache, in which case it needs to be discarded. In the code that handles that case, it was not taken into account that, in some cases, the TLS certs store could not have been created or should not be deleted, as it is being managed by the TLS contexts cache already. Deleting the store in such cases might lead to crashes. This commit fixes the issue. --- lib/dns/transport.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/dns/transport.c b/lib/dns/transport.c index c7e01c0af4..fb0c4ffb3d 100644 --- a/lib/dns/transport.c +++ b/lib/dns/transport.c @@ -524,7 +524,24 @@ dns_transport_get_tlsctx(dns_transport_t *transport, const isc_sockaddr_t *peer, */ INSIST(found != NULL); isc_tlsctx_free(&tlsctx); - isc_tls_cert_store_free(&store); + /* + * The 'store' variable can be 'NULL' when remote server + * verification is not enabled (that is, when Strict or + * Mutual TLS are not used). + * + * The 'found_store' might be equal to 'store' as there + * is one-to-many relation between a store and + * per-transport TLS contexts. In that case, the call to + * 'isc_tlsctx_cache_find()' above could have returned a + * store via the 'found_store' variable, whose value we + * can assign to 'store' later. In that case, + * 'isc_tlsctx_cache_add()' will return the same value. + * When that happens, we should not free the store + * object, as it is managed by the TLS context cache. + */ + if (store != NULL && store != found_store) { + isc_tls_cert_store_free(&store); + } isc_tlsctx_client_session_cache_detach(&sess_cache); /* Let's return the data from the cache. */ *psess_cache = found_sess_cache;