1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
|
From: David Woodhouse <dwmw2@infradead.org>
Date: Mon, 4 May 2020 17:36:22 +0100
Subject: Issue #548: Don't clean up engines after OpenSSL has already shut
down
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
As of 1.1.0, OpenSSL registers its own atexit() handler to call
OPENSSL_cleanup(). If our own code subsequently tries to, for example,
unreference an ENGINE, then it'll crash or deadlock with a use after
free.
Fix it by registering a callback with OPENSSL_atexit() to be called when
OPENSSL_cleanup() is called. It sets a flag which prevents any further
touching of OpenSSL objects — which would otherwise happen fairly much
immediately thereafter when our own OSSLCryptoFactory destructor gets
called by the C++ runtime's own atexit() handler.
Fixes: #548
---
src/lib/crypto/OSSLCryptoFactory.cpp | 64 ++++++++++++++++++++++++++----------
1 file changed, 46 insertions(+), 18 deletions(-)
diff --git a/src/lib/crypto/OSSLCryptoFactory.cpp b/src/lib/crypto/OSSLCryptoFactory.cpp
index 32daca2..81d080a 100644
--- a/src/lib/crypto/OSSLCryptoFactory.cpp
+++ b/src/lib/crypto/OSSLCryptoFactory.cpp
@@ -77,6 +77,7 @@ bool OSSLCryptoFactory::FipsSelfTestStatus = false;
static unsigned nlocks;
static Mutex** locks;
+static bool ossl_shutdown;
// Mutex callback
void lock_callback(int mode, int n, const char* file, int line)
@@ -101,6 +102,26 @@ void lock_callback(int mode, int n, const char* file, int line)
}
}
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L
+void ossl_factory_shutdown(void)
+{
+ /*
+ * As of 1.1.0, OpenSSL registers its own atexit() handler
+ * to call OPENSSL_cleanup(). If our own atexit() handler
+ * subsequently tries to, for example, unreference an
+ * ENGINE, then it'll crash or deadlock with a use-after-free.
+ *
+ * This hook into the OpenSSL_atexit() handlers will get called
+ * when OPENSSL_cleanup() is called, and sets a flag which
+ * prevents any further touching of OpenSSL objects — which
+ * would otherwise happen fairly much immediately thereafter
+ * when our own OSSLCryptoFactory destructor gets called by
+ * the C++ runtime's own atexit() handler.
+ */
+ ossl_shutdown = true;
+}
+#endif
+
// Constructor
OSSLCryptoFactory::OSSLCryptoFactory()
{
@@ -119,6 +140,9 @@ OSSLCryptoFactory::OSSLCryptoFactory()
CRYPTO_set_locking_callback(lock_callback);
setLockingCallback = true;
}
+#else
+ // Mustn't dereference engines after OpenSSL itself has shut down
+ OPENSSL_atexit(ossl_factory_shutdown);
#endif
#ifdef WITH_FIPS
@@ -226,31 +250,35 @@ err:
// Destructor
OSSLCryptoFactory::~OSSLCryptoFactory()
{
-#ifdef WITH_GOST
- // Finish the GOST engine
- if (eg != NULL)
+ // Don't do this if OPENSSL_cleanup() has already happened
+ if (!ossl_shutdown)
{
- ENGINE_finish(eg);
- ENGINE_free(eg);
- eg = NULL;
- }
+#ifdef WITH_GOST
+ // Finish the GOST engine
+ if (eg != NULL)
+ {
+ ENGINE_finish(eg);
+ ENGINE_free(eg);
+ eg = NULL;
+ }
#endif
- // Finish the rd_rand engine
- ENGINE_finish(rdrand_engine);
- ENGINE_free(rdrand_engine);
- rdrand_engine = NULL;
+ // Finish the rd_rand engine
+ ENGINE_finish(rdrand_engine);
+ ENGINE_free(rdrand_engine);
+ rdrand_engine = NULL;
+ // Recycle locks
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
+ if (setLockingCallback)
+ {
+ CRYPTO_set_locking_callback(NULL);
+ }
+#endif
+ }
// Destroy the one-and-only RNG
delete rng;
- // Recycle locks
-#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined(LIBRESSL_VERSION_NUMBER)
- if (setLockingCallback)
- {
- CRYPTO_set_locking_callback(NULL);
- }
-#endif
for (unsigned i = 0; i < nlocks; i++)
{
MutexFactory::i()->recycleMutex(locks[i]);
|
