Use library constructor/destructor to initialize OpenSSL

Instead of calling isc_tls_initialize()/isc_tls_destroy() explicitly use
gcc/clang attributes on POSIX and DLLMain on Windows to initialize and
shutdown OpenSSL library.

This resolves the issue when isc_nm_create() / isc_nm_destroy() was
called multiple times and it would call OpenSSL library destructors from
isc_nm_destroy().

At the same time, since we now have introduced the ctor/dtor for libisc,
this commit moves the isc_mem API initialization (the list of the
contexts) and changes the isc_mem_checkdestroyed() to schedule the
checking of memory context on library unload instead of executing the
code immediately.

diff --git a/configure.ac b/configure.ac
index 1d1d1c7fa685cb6f5ffda1aef573030a98d9109a..a85bc0a3c7f932ccf6cada262b65a6770173560f 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -1580,6 +1580,12 @@ test -z "$with_dlz_stub" && with_dlz_stub=no
 
 AC_CHECK_HEADERS([glob.h])
 
+#
+# Support for constructor and destructor attributes
+#
+AX_GCC_FUNC_ATTRIBUTE([constructor])
+AX_GCC_FUNC_ATTRIBUTE([destructor])
+
 #
 # Files to configure.  These are listed here because we used to
 # specify them as arguments to AC_OUTPUT.

diff --git a/lib/dns/openssl_link.c b/lib/dns/openssl_link.c
index bb26adcf26dffd5069ade9d24f2246c3ac60d6aa..1060fcc4ff99dacd5af664c6edb39665d0da55de 100644 (file)
--- a/lib/dns/openssl_link.c
+++ b/lib/dns/openssl_link.c
@@ -68,8 +68,6 @@ isc_result_t
 dst__openssl_init(const char *engine) {
        isc_result_t result = ISC_R_SUCCESS;
 
-       isc_tls_initialize();
-
        enable_fips_mode();
 
 #if !defined(OPENSSL_NO_ENGINE)
@@ -110,7 +108,6 @@ dst__openssl_destroy(void) {
        }
        e = NULL;
 #endif /* if !defined(OPENSSL_NO_ENGINE) */
-       isc_tls_destroy();
 }
 
 static isc_result_t

diff --git a/lib/isc/include/isc/tls.h b/lib/isc/include/isc/tls.h
index d0318262e5f02c00341bbfae0f4f9e0a844f1121..3741f19f5efadf22d0a7940c4b40f7d53fe11ea1 100644 (file)
--- a/lib/isc/include/isc/tls.h
+++ b/lib/isc/include/isc/tls.h
@@ -19,12 +19,6 @@
 
 typedef struct ssl_ctx_st isc_tlsctx_t;
 
-void
-isc_tls_initialize(void);
-
-void
-isc_tls_destroy(void);
-
 void
 isc_tlsctx_free(isc_tlsctx_t **ctpx);
 /*%

diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h
index 8941ec125e03c19918a6dc8df7ad369ad642d649..3c8c40b67914b204f9b3f893c39be52def53cf21 100644 (file)
--- a/lib/isc/include/isc/util.h
+++ b/lib/isc/include/isc/util.h
@@ -48,6 +48,16 @@
 #define ISC_NONSTRING
 #endif /* __GNUC__ */
 
+#if HAVE_FUNC_ATTRIBUTE_CONSTRUCTOR && HAVE_FUNC_ATTRIBUTE_DESTRUCTOR
+#define ISC_CONSTRUCTOR(priority) __attribute__((constructor(priority)))
+#define ISC_DESTRUCTOR(priority)  __attribute__((destructor(priority)))
+#elif WIN32
+#define ISC_CONSTRUCTOR(priority)
+#define ISC_DESTRUCTOR(priority)
+#else
+#error Either __attribute__((constructor|destructor))__ or DllMain support needed to compile BIND 9.
+#endif
+
 /*%
  * The opposite: silent warnings about stored values which are never read.
  */

diff --git a/lib/isc/lib.c b/lib/isc/lib.c
index 8bee4c21a4988e5e8313dfe3242450dda18c5ff9..dad744903dd06d184de295c317a57e842eb50839 100644 (file)
--- a/lib/isc/lib.c
+++ b/lib/isc/lib.c
@@ -13,6 +13,12 @@
 
 #include <isc/bind9.h>
 #include <isc/lib.h>
+#include <isc/mem.h>
+#include <isc/tls.h>
+#include <isc/util.h>
+
+#include "mem_p.h"
+#include "tls_p.h"
 
 /***
  *** Functions
@@ -22,3 +28,20 @@ void
 isc_lib_register(void) {
        isc_bind9 = false;
 }
+
+void
+isc__initialize(void) ISC_CONSTRUCTOR(101);
+void
+isc__shutdown(void) ISC_DESTRUCTOR(101);
+
+void
+isc__initialize(void) {
+       isc__mem_initialize();
+       isc__tls_initialize();
+}
+
+void
+isc__shutdown(void) {
+       isc__tls_shutdown();
+       isc__mem_shutdown();
+}

diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index 9f6db54823070222d69b53670b04d9bdc9a89170..53ea7e6e3ac31809979c5634869192e20d4e739f 100644 (file)
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -117,7 +117,8 @@ struct stats {
 
 static ISC_LIST(isc_mem_t) contexts;
 
-static isc_once_t once = ISC_ONCE_INIT;
+static isc_once_t init_once = ISC_ONCE_INIT;
+static isc_once_t shut_once = ISC_ONCE_INIT;
 static isc_mutex_t contextslock;
 
 /*%
@@ -440,12 +441,29 @@ default_memfree(void *ptr) {
 }
 
 static void
-initialize_action(void) {
+mem_initialize(void) {
        isc_mutex_init(&contextslock);
        ISC_LIST_INIT(contexts);
        totallost = 0;
 }
 
+void
+isc__mem_initialize(void) {
+       RUNTIME_CHECK(isc_once_do(&init_once, mem_initialize) == ISC_R_SUCCESS);
+}
+
+static void
+mem_shutdown(void) {
+       isc__mem_checkdestroyed();
+
+       isc_mutex_destroy(&contextslock);
+}
+
+void
+isc__mem_shutdown(void) {
+       RUNTIME_CHECK(isc_once_do(&shut_once, mem_shutdown) == ISC_R_SUCCESS);
+}
+
 static void
 mem_create(isc_mem_t **ctxp, unsigned int flags) {
        REQUIRE(ctxp != NULL && *ctxp == NULL);
@@ -457,8 +475,6 @@ mem_create(isc_mem_t **ctxp, unsigned int flags) {
        STATIC_ASSERT(ALIGNMENT_SIZE >= sizeof(size_info),
                      "alignment size too small");
 
-       RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS);
-
        ctx = default_memalloc(sizeof(*ctx));
 
        *ctx = (isc_mem_t){
@@ -1458,13 +1474,20 @@ print_contexts(FILE *file) {
 }
 #endif
 
+static atomic_uintptr_t checkdestroyed = ATOMIC_VAR_INIT(0);
+
 void
 isc_mem_checkdestroyed(FILE *file) {
-#if !ISC_MEM_TRACKLINES
-       UNUSED(file);
-#endif /* if !ISC_MEM_TRACKLINES */
+       atomic_store_release(&checkdestroyed, (uintptr_t)file);
+}
 
-       RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS);
+void
+isc__mem_checkdestroyed(void) {
+       FILE *file = (FILE *)atomic_load_acquire(&checkdestroyed);
+
+       if (file == NULL) {
+               return;
+       }
 
        LOCK(&contextslock);
        if (!ISC_LIST_EMPTY(contexts)) {
@@ -1597,8 +1620,6 @@ isc_mem_renderxml(void *writer0) {
 
        TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "contexts"));
 
-       RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS);
-
        LOCK(&contextslock);
        lost = totallost;
        for (ctx = ISC_LIST_HEAD(contexts); ctx != NULL;
@@ -1739,7 +1760,6 @@ isc_mem_renderjson(void *memobj0) {
        json_object *memobj = (json_object *)memobj0;
 
        memset(&summary, 0, sizeof(summary));
-       RUNTIME_CHECK(isc_once_do(&once, initialize_action) == ISC_R_SUCCESS);
 
        ctxarray = json_object_new_array();
        CHECKMEM(ctxarray);

diff --git a/lib/isc/mem_p.h b/lib/isc/mem_p.h
index f1889c668b2df96f96fe7497d79c1661dc53ade2..dc17e06420f2c41d33c16eeca61abaf7cad404df 100644 (file)
--- a/lib/isc/mem_p.h
+++ b/lib/isc/mem_p.h
@@ -9,8 +9,11 @@
  * information regarding copyright ownership.
  */
 
-#ifndef ISC_MEM_P_H
-#define ISC_MEM_P_H
+#pragma once
+
+#include <stdio.h>
+
+#include <isc/mem.h>
 
 /*! \file */
 
@@ -21,4 +24,11 @@ isc__mem_printactive(isc_mem_t *mctx, FILE *file);
  * a single memory context.
  */
 
-#endif /* ISC_MEM_P_H */
+void
+isc__mem_checkdestroyed(void);
+
+void
+isc__mem_initialize(void);
+
+void
+isc__mem_shutdown(void);

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 5ec2ea07074b127095508655d062f15a93fe2148..eb7a23e3a47d4e35955953ff39b91b4e5ea61b97 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -216,8 +216,6 @@ isc_nm_start(isc_mem_t *mctx, uint32_t workers) {
        isc__nm_winsock_initialize();
 #endif /* WIN32 */
 
-       isc_tls_initialize();
-
        mgr = isc_mem_get(mctx, sizeof(*mgr));
        *mgr = (isc_nm_t){ .nworkers = workers };
 
@@ -374,8 +372,6 @@ nm_destroy(isc_nm_t **mgr0) {
                    mgr->nworkers * sizeof(isc__networker_t));
        isc_mem_putanddetach(&mgr->mctx, mgr, sizeof(*mgr));
 
-       isc_tls_destroy();
-
 #ifdef WIN32
        isc__nm_winsock_destroy();
 #endif /* WIN32 */

diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index 90aa605232b08033c1c2ad4e6fedac665d86fe4f..eaa8921b150acf265d220106cb9d012b0345bb1d 100644 (file)
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -398,6 +398,7 @@ isc_nm_tlsdnsconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer,
        REQUIRE(VALID_NM(mgr));
        REQUIRE(local != NULL);
        REQUIRE(peer != NULL);
+       REQUIRE(sslctx != NULL);
 
        sa_family = peer->addr.type.sa.sa_family;
 
@@ -1656,6 +1657,16 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
        csock->tls.state = TLS_STATE_NONE;
 
        csock->tls.ssl = SSL_new(ssock->tls.ctx);
+
+       if (csock->tls.ssl == NULL) {
+               char errbuf[256];
+               unsigned long err = ERR_get_error();
+
+               ERR_error_string_n(err, errbuf, sizeof(errbuf));
+               fprintf(stderr, "%s:SSL_new(%p) -> %s\n", __func__,
+                       ssock->tls.ctx, errbuf);
+       }
+
        RUNTIME_CHECK(csock->tls.ssl != NULL);
 
        r = BIO_new_bio_pair(&csock->tls.ssl_wbio, TLS_BUF_SIZE,

diff --git a/lib/isc/tests/tlsdns_test.c b/lib/isc/tests/tlsdns_test.c
index 76b89744177dec10d016c83c5fb2454acd6d23cd..b7bef12cca994bba66cdbf06051d606ead41cdff 100644 (file)
--- a/lib/isc/tests/tlsdns_test.c
+++ b/lib/isc/tests/tlsdns_test.c
@@ -217,8 +217,13 @@ nm_setup(void **state) {
        int tlsdns_listen_sock = -1;
        isc_nm_t **nm = NULL;
 
-       isc_tlsctx_createserver(NULL, NULL, &tlsdns_listen_ctx);
-       isc_tlsctx_createclient(&tlsdns_connect_ctx);
+       if (isc_tlsctx_createserver(NULL, NULL, &tlsdns_listen_ctx) !=
+           ISC_R_SUCCESS) {
+               return (-1);
+       }
+       if (isc_tlsctx_createclient(&tlsdns_connect_ctx) != ISC_R_SUCCESS) {
+               return (-1);
+       }
 
        tlsdns_listen_addr = (isc_sockaddr_t){ .length = 0 };
        tlsdns_listen_sock = setup_ephemeral_port(&tlsdns_listen_addr,

diff --git a/lib/isc/tls.c b/lib/isc/tls.c
index 57066a412167945a35b697ea9314bc6416fa83dc..cec759d924cf753c220c62fd6f7cd8acee8858ae 100644 (file)
--- a/lib/isc/tls.c
+++ b/lib/isc/tls.c
@@ -24,12 +24,12 @@
 #include <isc/util.h>
 
 #include "openssl_shim.h"
+#include "tls_p.h"
 
 static isc_once_t init_once = ISC_ONCE_INIT;
 static isc_once_t shut_once = ISC_ONCE_INIT;
 static atomic_bool init_done = ATOMIC_VAR_INIT(false);
 static atomic_bool shut_done = ATOMIC_VAR_INIT(false);
-static isc_mem_t *isc__tls_mctx = NULL;
 
 #if OPENSSL_VERSION_NUMBER < 0x10100000L
 static isc_mutex_t *locks = NULL;
@@ -52,53 +52,30 @@ isc__tls_set_thread_id(CRYPTO_THREADID *id) {
 }
 #endif
 
-#if 0
-static void *
-isc__tls_malloc(size_t size, const char *file, int line) {
-       UNUSED(file);
-       UNUSED(line);
-
-       return (isc_mem_allocate(isc__tls_mctx, size));
-}
-
-static void *
-isc__tls_realloc(void *ptr, size_t size, const char *file, int line) {
-       UNUSED(file);
-       UNUSED(line);
-
-       return (isc__mem_reallocate(isc__tls_mctx, ptr, size));
-}
-
-static void
-isc__tls_free(void *ptr, const char *file, int line) {
-       UNUSED(file);
-       UNUSED(line);
-
-       if (ptr == NULL) {
-               return;
-       }
-
-       isc__mem_free(isc__tls_mctx, ptr);
-}
-#endif
-
 static void
-isc__tls_initialize(void) {
+tls_initialize(void) {
        REQUIRE(!atomic_load(&init_done));
 
-       isc_mem_create(&isc__tls_mctx);
-       /* isc_mem_setdestroycheck(isc__tls_mctx, false); */
-
-       /* REQUIRE(CRYPTO_set_mem_functions(isc__tls_malloc, isc__tls_realloc,
-        * isc__tls_free) == 1); */
-
 #if OPENSSL_VERSION_NUMBER >= 0x10100000L
        RUNTIME_CHECK(OPENSSL_init_ssl(OPENSSL_INIT_ENGINE_ALL_BUILTIN |
                                               OPENSSL_INIT_LOAD_CONFIG,
                                       NULL) == 1);
 #else
        nlocks = CRYPTO_num_locks();
-       locks = isc_mem_get(isc__tls_mctx, nlocks * sizeof(locks[0]));
+       /*
+        * We can't use isc_mem API here, because it's called too
+        * early and when the isc_mem_debugging flags are changed
+        * later and ISC_MEM_DEBUGSIZE or ISC_MEM_DEBUGCTX flags are
+        * added, neither isc_mem_put() nor isc_mem_free() can be used
+        * to free up the memory allocated here because the flags were
+        * not set when calling isc_mem_get() or isc_mem_allocate()
+        * here.
+        *
+        * Actually, since this is a single allocation at library load
+        * and deallocation at library unload, using the standard
+        * allocator without the tracking is fine for this purpose.
+        */
+       locks = calloc(nlocks, sizeof(locks[0]));
        isc_mutexblock_init(locks, nlocks);
        CRYPTO_set_locking_callback(isc__tls_lock_callback);
        CRYPTO_THREADID_set_callback(isc__tls_set_thread_id);
@@ -127,20 +104,22 @@ isc__tls_initialize(void) {
                            "seeded' message in the OpenSSL FAQ)");
        }
 
-       atomic_compare_exchange_strong(&init_done, &(bool){ false }, true);
+       REQUIRE(atomic_compare_exchange_strong(&init_done, &(bool){ false },
+                                              true));
 }
 
 void
-isc_tls_initialize(void) {
-       isc_result_t result = isc_once_do(&init_once, isc__tls_initialize);
+isc__tls_initialize(void) {
+       isc_result_t result = isc_once_do(&init_once, tls_initialize);
        REQUIRE(result == ISC_R_SUCCESS);
        REQUIRE(atomic_load(&init_done));
 }
 
 static void
-isc__tls_destroy(void) {
+tls_shutdown(void) {
        REQUIRE(atomic_load(&init_done));
        REQUIRE(!atomic_load(&shut_done));
+
 #if OPENSSL_VERSION_NUMBER < 0x10100000L
 
        CONF_modules_unload(1);
@@ -156,24 +135,20 @@ isc__tls_destroy(void) {
 
        CRYPTO_set_locking_callback(NULL);
 
-       /* REQUIRE(CRYPTO_set_mem_functions(OPENSSL_malloc, OPENSSL_realloc,
-        * OPENSSL_free) == 1); */
-
        if (locks != NULL) {
-               INSIST(isc__tls_mctx != NULL);
                isc_mutexblock_destroy(locks, nlocks);
-               isc_mem_put(isc__tls_mctx, locks, nlocks * sizeof(locks[0]));
+               free(locks);
                locks = NULL;
        }
 #endif
-       isc_mem_detach(&isc__tls_mctx);
 
-       atomic_compare_exchange_strong(&shut_done, &(bool){ false }, true);
+       REQUIRE(atomic_compare_exchange_strong(&shut_done, &(bool){ false },
+                                              true));
 }
 
 void
-isc_tls_destroy(void) {
-       isc_result_t result = isc_once_do(&shut_once, isc__tls_destroy);
+isc__tls_shutdown(void) {
+       isc_result_t result = isc_once_do(&shut_once, tls_shutdown);
        REQUIRE(result == ISC_R_SUCCESS);
        REQUIRE(atomic_load(&shut_done));
 }
@@ -198,8 +173,6 @@ isc_tlsctx_createclient(isc_tlsctx_t **ctxp) {
 
        REQUIRE(ctxp != NULL && *ctxp == NULL);
 
-       isc_tls_initialize();
-
        method = TLS_client_method();
        if (method == NULL) {
                goto ssl_error;
@@ -248,8 +221,6 @@ isc_tlsctx_createserver(const char *keyfile, const char *certfile,
 
        REQUIRE(ctxp != NULL && *ctxp == NULL);
 
-       isc_tls_initialize();
-
        if (ephemeral) {
                INSIST(keyfile == NULL);
                INSIST(certfile == NULL);
@@ -365,6 +336,7 @@ ssl_error:
        isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_NETMGR,
                      ISC_LOG_ERROR, "Error initializing TLS context: %s",
                      errbuf);
+
        if (ctx != NULL) {
                SSL_CTX_free(ctx);
        }

diff --git a/lib/isc/tls_p.h b/lib/isc/tls_p.h
new file mode 100644 (file)
index 0000000..5d44c64
--- /dev/null
+++ b/lib/isc/tls_p.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+#pragma once
+
+void
+isc__tls_initialize(void);
+
+void
+isc__tls_shutdown(void);

diff --git a/lib/isc/win32/DLLMain.c b/lib/isc/win32/DLLMain.c
index 3acfee4f4679e9ba1e45776634865742a8992e02..c649effbc278f897b7e7bed05abdc6efccf93260 100644 (file)
--- a/lib/isc/win32/DLLMain.c
+++ b/lib/isc/win32/DLLMain.c
@@ -12,6 +12,13 @@
 #include <stdio.h>
 #include <windows.h>
 
+#include <isc/mem.h>
+#include <isc/tls.h>
+#include <isc/util.h>
+
+#include "mem_p.h"
+#include "tls_p.h"
+
 /*
  * Called when we enter the DLL
  */
@@ -27,6 +34,8 @@ __declspec(dllexport) BOOL WINAPI
                 * Disable DllMain() invocation on Thread creation/destruction
                 */
                DisableThreadLibraryCalls(hinstDLL);
+               isc__mem_initialize();
+               isc__tls_initialize();
                break;
 
        /*
@@ -34,6 +43,8 @@ __declspec(dllexport) BOOL WINAPI
         * termination or a call to FreeLibrary.
         */
        case DLL_PROCESS_DETACH:
+               isc__tls_shutdown();
+               isc__mem_shutdown();
                break;
 
        case DLL_THREAD_ATTACH:
@@ -43,6 +54,7 @@ __declspec(dllexport) BOOL WINAPI
                 * disabled.
                 */
                INSIST(0);
+               ISC_UNREACHABLE();
                break;
 
        default:

diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in
index b02fb22bbb2a7cbe9a51bd4c7f8319b8bf0f4135..5797632b87169d0d2ce56dded4b56450cb8fefc9 100644 (file)
--- a/lib/isc/win32/libisc.def.in
+++ b/lib/isc/win32/libisc.def.in
@@ -362,6 +362,9 @@ isc_md_get_block_size
 isc_md_type_get_size
 isc_md_type_get_block_size
 isc_md
+isc__mem_checkdestroyed
+isc__mem_initialize
+isc__mem_shutdown
 isc_mem_attach
 isc_mem_checkdestroyed
 isc_mem_create
@@ -707,8 +710,8 @@ isc_timermgr_create
 isc_timermgr_createinctx
 isc_timermgr_destroy
 isc_timermgr_poke
-isc_tls_initialize
-isc_tls_destroy
+isc__tls_initialize
+isc__tls_shutdown
 isc_tlsctx_createclient
 isc_tlsctx_createserver
 isc_tlsctx_free

diff --git a/util/copyrights b/util/copyrights
index 5c0d643f4b16fdc6a6373bca6032f556392abe22..ee79ea4c983fba24abb10bee10323bf4f1220d15 100644 (file)
--- a/util/copyrights
+++ b/util/copyrights
@@ -1994,6 +1994,7 @@
 ./lib/isc/tests/uv_wrap.h                      C       2020,2021
 ./lib/isc/timer.c                              C       1998,1999,2000,2001,2002,2004,2005,2007,2008,2009,2011,2012,2013,2014,2015,2016,2017,2018,2019,2020,2021
 ./lib/isc/tls.c                                        C       2021
+./lib/isc/tls_p.h                              C       2021
 ./lib/isc/tm.c                                 C       2014,2016,2018,2019,2020,2021
 ./lib/isc/unix/dir.c                           C       1999,2000,2001,2004,2005,2007,2008,2009,2011,2012,2016,2017,2018,2019,2020,2021
 ./lib/isc/unix/errno.c                         C       2016,2018,2019,2020,2021