Fix crash caused by initial patch for rhbz#1831086

This commit is contained in:
David Woodhouse 2020-05-13 19:46:16 +01:00
parent c2ee7fde90
commit 1e209fc79d
2 changed files with 22 additions and 49 deletions

View File

@ -1,60 +1,29 @@
diff --git a/src/lib/crypto/OSSLCryptoFactory.cpp b/src/lib/crypto/OSSLCryptoFactory.cpp
index 32daca2f..81d080a5 100644
index 32daca2..ace4bcb 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 @@ OSSLCryptoFactory::OSSLCryptoFactory()
@@ -226,31 +226,49 @@ err:
// Destructor
OSSLCryptoFactory::~OSSLCryptoFactory()
{
-#ifdef WITH_GOST
- // Finish the GOST engine
- if (eg != NULL)
+ // Don't do this if OPENSSL_cleanup() has already happened
+ bool ossl_shutdown = false;
+
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L && !defined(LIBRESSL_VERSION_NUMBER)
+ // OpenSSL 1.1.0+ will register an atexit() handler to run
+ // OPENSSL_cleanup(). If that has already happened we must
+ // not attempt to free any ENGINEs because they'll already
+ // have been destroyed and the use-after-free would cause
+ // a deadlock or crash.
+ //
+ // Detect that situation because reinitialisation will fail
+ // after OPENSSL_cleanup() has run.
+ (void)ERR_set_mark();
+ ossl_shutdown = !OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_RDRAND, NULL);
+ (void)ERR_pop_to_mark();
+#endif
+ if (!ossl_shutdown)
{
- ENGINE_finish(eg);

View File

@ -4,7 +4,7 @@
Summary: Software version of a PKCS#11 Hardware Security Module
Name: softhsm
Version: 2.6.1
Release: %{?prever:0.}2%{?prever:.%{prever}}%{?dist}
Release: %{?prever:0.}3%{?prever:.%{prever}}%{?dist}
License: BSD
Url: http://www.opendnssec.org/
Source: http://dist.opendnssec.org/source/%{?prever:testing/}%{name}-%{version}.tar.gz
@ -111,6 +111,10 @@ if [ -f /var/softhsm/slot0.db ]; then
fi
%changelog
* Wed May 13 2020 David Woodhouse <dwmw2@infradead.org> - 2.6.1-3
- Resolves: rhbz#1831086 softhsm use-after-free on process exit
Fix crash introduced by initial patch
* Tue May 12 2020 Paul Wouters <pwouters@redhat.com> - 2.6.1-2
- Resolves: rhbz#1831086 softhsm use-after-free on process exit