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.

diff --git a/lib/dns/transport.c b/lib/dns/transport.c
index c7e01c0af424be9de96714a1f45c3ab8e385c0f4..fb0c4ffb3d4242bdd0a6b817617373e033c6b994 100644 (file)
--- 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;