Rename OPENSSL_NO_THREADS, part 1.
BoringSSL depends on the platform's locking APIs to make internal global
state thread-safe, including the PRNG. On some single-threaded embedded
platforms, locking APIs may not exist, so this dependency may be disabled
with a build flag.
Doing so means the consumer promises the library will never be used in any
multi-threaded address space. It causes BoringSSL to be globally thread-unsafe.
Setting it inappropriately will subtly and unpredictably corrupt memory and
leak secret keys.
Unfortunately, folks sometimes misinterpreted OPENSSL_NO_THREADS as skipping an
internal thread pool or disabling an optionally extra-thread-safe mode. This is
not and has never been the case. Rename it to
OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED to clarify what
this option does.
Update-Note: As a first step, this CL makes both OPENSSL_NO_THREADS and
OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED work. A later CL
will remove the old name, so migrate callers after or at the same time as
picking up this CL.
Change-Id: Ibe4964ae43eb7a52f08fd966fccb330c0cc11a8c
Reviewed-on: https://boringssl-review.googlesource.com/32084
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/internal.h b/crypto/internal.h
index 0e8ae3a..6944d56 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -132,13 +132,13 @@
#endif
#endif
-#if !defined(OPENSSL_NO_THREADS) && \
+#if defined(OPENSSL_THREADS) && \
(!defined(OPENSSL_WINDOWS) || defined(__MINGW32__))
#include <pthread.h>
#define OPENSSL_PTHREADS
#endif
-#if !defined(OPENSSL_NO_THREADS) && !defined(OPENSSL_PTHREADS) && \
+#if defined(OPENSSL_THREADS) && !defined(OPENSSL_PTHREADS) && \
defined(OPENSSL_WINDOWS)
#define OPENSSL_WINDOWS_THREADS
OPENSSL_MSVC_PRAGMA(warning(push, 3))
@@ -367,7 +367,7 @@
// Thread-safe initialisation.
-#if defined(OPENSSL_NO_THREADS)
+#if !defined(OPENSSL_THREADS)
typedef uint32_t CRYPTO_once_t;
#define CRYPTO_ONCE_INIT 0
#elif defined(OPENSSL_WINDOWS_THREADS)
@@ -423,7 +423,7 @@
// thread.h as a structure large enough to fit the real type. The global lock is
// a different type so it may be initialized with platform initializer macros.
-#if defined(OPENSSL_NO_THREADS)
+#if !defined(OPENSSL_THREADS)
struct CRYPTO_STATIC_MUTEX {
char padding; // Empty structs have different sizes in C and C++.
};
diff --git a/crypto/pool/pool_test.cc b/crypto/pool/pool_test.cc
index 62fb17d..8f32fb6 100644
--- a/crypto/pool/pool_test.cc
+++ b/crypto/pool/pool_test.cc
@@ -18,7 +18,7 @@
#include "../test/test_util.h"
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
#include <chrono>
#include <thread>
#endif
@@ -61,7 +61,7 @@
EXPECT_EQ(buf.get(), buf2.get()) << "CRYPTO_BUFFER_POOL did not dedup data.";
}
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
TEST(PoolTest, Threads) {
bssl::UniquePtr<CRYPTO_BUFFER_POOL> pool(CRYPTO_BUFFER_POOL_new());
ASSERT_TRUE(pool);
diff --git a/crypto/rand_extra/rand_test.cc b/crypto/rand_extra/rand_test.cc
index bd2eb18..183c4f7 100644
--- a/crypto/rand_extra/rand_test.cc
+++ b/crypto/rand_extra/rand_test.cc
@@ -20,7 +20,7 @@
#include "../test/test_util.h"
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
#include <array>
#include <thread>
#include <vector>
@@ -146,7 +146,7 @@
}
#endif // !OPENSSL_WINDOWS && !BORINGSSL_UNSAFE_DETERMINISTIC_MODE
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
static void RunConcurrentRands(size_t num_threads) {
static const uint8_t kZeros[256] = {0};
diff --git a/crypto/refcount_test.cc b/crypto/refcount_test.cc
index 6ce0746..700863f 100644
--- a/crypto/refcount_test.cc
+++ b/crypto/refcount_test.cc
@@ -16,7 +16,7 @@
#include <gtest/gtest.h>
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
#include <thread>
#endif
@@ -43,7 +43,7 @@
EXPECT_EQ(1u, count);
}
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
// This test is primarily intended to run under ThreadSanitizer.
TEST(RefCountTest, Threads) {
CRYPTO_refcount_t count = 0;
diff --git a/crypto/rsa_extra/rsa_test.cc b/crypto/rsa_extra/rsa_test.cc
index b0a0b7e..9bd47b1 100644
--- a/crypto/rsa_extra/rsa_test.cc
+++ b/crypto/rsa_extra/rsa_test.cc
@@ -72,7 +72,7 @@
#include "../internal.h"
#include "../test/test_util.h"
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
#include <thread>
#include <vector>
#endif
@@ -1048,7 +1048,7 @@
}
#endif // !BORINGSSL_SHARED_LIBRARY
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
TEST(RSATest, Threads) {
bssl::UniquePtr<RSA> rsa_template(
RSA_private_key_from_bytes(kKey1, sizeof(kKey1) - 1));
diff --git a/crypto/thread_none.c b/crypto/thread_none.c
index 718d960..4f07b9d 100644
--- a/crypto/thread_none.c
+++ b/crypto/thread_none.c
@@ -14,7 +14,7 @@
#include "internal.h"
-#if defined(OPENSSL_NO_THREADS)
+#if !defined(OPENSSL_THREADS)
void CRYPTO_MUTEX_init(CRYPTO_MUTEX *lock) {}
@@ -56,4 +56,4 @@
return 1;
}
-#endif // OPENSSL_NO_THREADS
+#endif // !OPENSSL_THREADS
diff --git a/crypto/thread_test.cc b/crypto/thread_test.cc
index f89b22e..f9fad9b 100644
--- a/crypto/thread_test.cc
+++ b/crypto/thread_test.cc
@@ -25,7 +25,7 @@
#include "test/test_util.h"
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
static unsigned g_once_init_called = 0;
@@ -130,4 +130,4 @@
thread.join();
}
-#endif // !OPENSSL_NO_THREADS
+#endif // OPENSSL_THREADS
diff --git a/include/openssl/base.h b/include/openssl/base.h
index d134993..e287de7 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -136,14 +136,36 @@
#if defined(TRUSTY)
#define OPENSSL_TRUSTY
-#define OPENSSL_NO_THREADS
+#define OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED
#endif
#if defined(__ANDROID_API__)
#define OPENSSL_ANDROID
#endif
-#if !defined(OPENSSL_NO_THREADS)
+// OPENSSL_NO_THREADS has been deprecated in favor of this much longer and
+// louder name, to better reflect exactly what that option did.
+//
+// TODO(davidben): Remove this block when callers have migrated.
+#if defined(OPENSSL_NO_THREADS) && \
+ !defined(OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED)
+#define OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED
+#endif
+
+// BoringSSL requires platform's locking APIs to make internal global state
+// thread-safe, including the PRNG. On some single-threaded embedded platforms,
+// locking APIs may not exist, so this dependency may be disabled with the
+// following build flag.
+//
+// IMPORTANT: Doing so means the consumer promises the library will never be
+// used in any multi-threaded context. It causes BoringSSL to be globally
+// thread-unsafe. Setting it inappropriately will subtly and unpredictably
+// corrupt memory and leak secret keys.
+//
+// Do not set this flag on any platform where threads are possible. BoringSSL
+// maintainers will not provide support for any consumers that do so. Changes
+// which break such unsupported configurations will not be reverted.
+#if !defined(OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED)
#define OPENSSL_THREADS
#endif
diff --git a/include/openssl/thread.h b/include/openssl/thread.h
index 98073b0..91706fe 100644
--- a/include/openssl/thread.h
+++ b/include/openssl/thread.h
@@ -66,7 +66,7 @@
#endif
-#if defined(OPENSSL_NO_THREADS)
+#if !defined(OPENSSL_THREADS)
typedef struct crypto_mutex_st {
char padding; // Empty structs have different sizes in C and C++.
} CRYPTO_MUTEX;
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 894bb14..61a47d3 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -48,7 +48,7 @@
#include <sys/time.h>
#endif
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
#include <thread>
#endif
@@ -4298,7 +4298,7 @@
// These tests test multi-threaded behavior. They are intended to run with
// ThreadSanitizer.
-#if !defined(OPENSSL_NO_THREADS)
+#if defined(OPENSSL_THREADS)
TEST_P(SSLVersionTest, SessionCacheThreads) {
SSL_CTX_set_options(server_ctx_.get(), SSL_OP_NO_TICKET);
SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH);
