Make BoringSSL initialization-less
Now that we don't depend on external CRYPTO_library_init calls or the
static initializer to initialize CPU capabilities, we can drop a ton of
code.
This makes CRYPTO_library_init, and all its wrappers, into no-ops and
drops the (non-FIPS) static initializer. I've added an internal
OPENSSL_init_cpuid function for the places where the library actually
needs to initialize the CPU vector.
Note this slightly changes the default, previously
static-initializer-full build: previously, CRYPTO_library_init was a
no-op and we relied on the static initializer. Now we uniformly use
CRYPTO_once. This should be an atomic read in the steady state and
essentially free. We can restore the static initializer by default if
this ends up being a problem, but having only one mode is more
straightforward. This also avoids problems if an application calls into
BoringSSL during its own static initializer. Static initializers are not
coherently ordered.
Update-Note: The BORINGSSL_NO_STATIC_INITIALIZER build option and
CRYPTO_library_init are now unnecessary. Once updating past this
revision, those options can now be cleaned up from downstream projects.
Fixed: 40644931
Change-Id: Idc2e6ea7a73d6352e0360fd886c46d88dba3568c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69508
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/PORTING.md b/PORTING.md
index eb951f5..3faf757 100644
--- a/PORTING.md
+++ b/PORTING.md
@@ -206,12 +206,8 @@
OpenSSL has a number of different initialization functions for setting up error
strings and loading algorithms, etc. All of these functions still exist in
-BoringSSL for convenience, but they do nothing and are not necessary.
-
-The one exception is `CRYPTO_library_init`. In `BORINGSSL_NO_STATIC_INITIALIZER`
-builds, it must be called to query CPU capabilities before the rest of the
-library. In the default configuration, this is done with a static initializer
-and is also unnecessary.
+BoringSSL for convenience, but they do nothing and are not necessary. BoringSSL
+internally initializes itself as needed.
### Threading
diff --git a/crypto/cipher_extra/e_chacha20poly1305.c b/crypto/cipher_extra/e_chacha20poly1305.c
index 3ac2af8..d200465 100644
--- a/crypto/cipher_extra/e_chacha20poly1305.c
+++ b/crypto/cipher_extra/e_chacha20poly1305.c
@@ -46,7 +46,7 @@
// worth adjusting the assembly calling convention. The assembly functions do
// too much work right now. For now, explicitly initialize |OPENSSL_ia32cap_P|
// first.
- CRYPTO_library_init();
+ OPENSSL_init_cpuid();
struct aead_chacha20_poly1305_ctx *c20_ctx =
(struct aead_chacha20_poly1305_ctx *)&ctx->state;
diff --git a/crypto/crypto.c b/crypto/crypto.c
index 0bca1e2..724a774 100644
--- a/crypto/crypto.c
+++ b/crypto/crypto.c
@@ -24,23 +24,6 @@
static_assert(sizeof(ossl_ssize_t) == sizeof(size_t),
"ossl_ssize_t should be the same size as size_t");
-#if !defined(OPENSSL_NO_ASM) && !defined(OPENSSL_STATIC_ARMCAP) && \
- (defined(OPENSSL_X86) || defined(OPENSSL_X86_64) || \
- defined(OPENSSL_ARM) || defined(OPENSSL_AARCH64))
-// x86, x86_64, and the ARMs need to record the result of a cpuid/getauxval call
-// for the asm to work correctly, unless compiled without asm code.
-#define NEED_CPUID
-
-#else
-
-// Otherwise, don't emit a static initialiser.
-
-#if !defined(BORINGSSL_NO_STATIC_INITIALIZER)
-#define BORINGSSL_NO_STATIC_INITIALIZER
-#endif
-
-#endif // !NO_ASM && !STATIC_ARMCAP && (X86 || X86_64 || ARM || AARCH64)
-
// Our assembly does not use the GOT to reference symbols, which means
// references to visible symbols will often require a TEXTREL. This is
@@ -79,7 +62,7 @@
HIDDEN uint32_t OPENSSL_ia32cap_P[4] = {0};
uint32_t OPENSSL_get_ia32cap(int idx) {
- CRYPTO_library_init();
+ OPENSSL_init_cpuid();
return OPENSSL_ia32cap_P[idx];
}
@@ -121,60 +104,24 @@
HIDDEN uint32_t OPENSSL_armcap_P = 0;
uint32_t *OPENSSL_get_armcap_pointer_for_test(void) {
- CRYPTO_library_init();
+ OPENSSL_init_cpuid();
return &OPENSSL_armcap_P;
}
#endif
uint32_t OPENSSL_get_armcap(void) {
- CRYPTO_library_init();
+ OPENSSL_init_cpuid();
return OPENSSL_armcap_P;
}
#endif
-#if defined(BORINGSSL_FIPS)
-// In FIPS mode, the power-on self-test function calls |CRYPTO_library_init|
-// because we have to ensure that CPUID detection occurs first.
-#define BORINGSSL_NO_STATIC_INITIALIZER
-#endif
-
-#if defined(OPENSSL_WINDOWS) && !defined(BORINGSSL_NO_STATIC_INITIALIZER)
-#define OPENSSL_CDECL __cdecl
-#else
-#define OPENSSL_CDECL
-#endif
-
-#if defined(BORINGSSL_NO_STATIC_INITIALIZER)
-static CRYPTO_once_t once = CRYPTO_ONCE_INIT;
-#elif defined(_MSC_VER)
-#pragma section(".CRT$XCU", read)
-static void __cdecl do_library_init(void);
-__declspec(allocate(".CRT$XCU")) void(*library_init_constructor)(void) =
- do_library_init;
-#else
-static void do_library_init(void) __attribute__ ((constructor));
-#endif
-
-// do_library_init is the actual initialization function. If
-// BORINGSSL_NO_STATIC_INITIALIZER isn't defined, this is set as a static
-// initializer. Otherwise, it is called by CRYPTO_library_init.
-static void OPENSSL_CDECL do_library_init(void) {
- // WARNING: this function may only configure the capability variables. See the
- // note above about the linker bug.
#if defined(NEED_CPUID)
- OPENSSL_cpuid_setup();
+static CRYPTO_once_t once = CRYPTO_ONCE_INIT;
+void OPENSSL_init_cpuid(void) { CRYPTO_once(&once, OPENSSL_cpuid_setup); }
#endif
-}
-void CRYPTO_library_init(void) {
- // TODO(davidben): It would be tidier if this build knob could be replaced
- // with an internal lazy-init mechanism that would handle things correctly
- // in-library. https://crbug.com/542879
-#if defined(BORINGSSL_NO_STATIC_INITIALIZER)
- CRYPTO_once(&once, do_library_init);
-#endif
-}
+void CRYPTO_library_init(void) {}
int CRYPTO_is_confidential_build(void) {
#if defined(BORINGSSL_CONFIDENTIAL)
@@ -194,7 +141,7 @@
void CRYPTO_pre_sandbox_init(void) {
// Read from /proc/cpuinfo if needed.
- CRYPTO_library_init();
+ OPENSSL_init_cpuid();
// Open /dev/urandom if needed.
CRYPTO_init_sysrand();
// Set up MADV_WIPEONFORK state if needed.
@@ -235,7 +182,6 @@
void OPENSSL_load_builtin_modules(void) {}
int OPENSSL_init_crypto(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings) {
- CRYPTO_library_init();
return 1;
}
diff --git a/crypto/fipsmodule/bcm.c b/crypto/fipsmodule/bcm.c
index 4b9b64f..c356730 100644
--- a/crypto/fipsmodule/bcm.c
+++ b/crypto/fipsmodule/bcm.c
@@ -168,8 +168,6 @@
static void __attribute__((constructor))
BORINGSSL_bcm_power_on_self_test(void) {
- CRYPTO_library_init();
-
#if !defined(OPENSSL_ASAN)
// Integrity tests cannot run under ASAN because it involves reading the full
// .text section, which triggers the global-buffer overflow detection.
diff --git a/crypto/fipsmodule/ec/p256-nistz.c b/crypto/fipsmodule/ec/p256-nistz.c
index 2773820..7d3e2fb 100644
--- a/crypto/fipsmodule/ec/p256-nistz.c
+++ b/crypto/fipsmodule/ec/p256-nistz.c
@@ -615,7 +615,7 @@
// TODO(crbug.com/42290548): The x86_64 assembly depends on initializing
// |OPENSSL_ia32cap_P|. Move the dispatch to C. For now, explicitly initialize
// things.
- CRYPTO_library_init();
+ OPENSSL_init_cpuid();
out->point_get_affine_coordinates = ecp_nistz256_get_affine;
out->add = ecp_nistz256_add;
diff --git a/crypto/internal.h b/crypto/internal.h
index 15facb1..f93c2e5 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -180,17 +180,29 @@
#endif
-#if defined(OPENSSL_X86) || defined(OPENSSL_X86_64) || defined(OPENSSL_ARM) || \
- defined(OPENSSL_AARCH64)
-// OPENSSL_cpuid_setup initializes the platform-specific feature cache.
+#if !defined(OPENSSL_NO_ASM) && !defined(OPENSSL_STATIC_ARMCAP) && \
+ (defined(OPENSSL_X86) || defined(OPENSSL_X86_64) || \
+ defined(OPENSSL_ARM) || defined(OPENSSL_AARCH64))
+// x86, x86_64, and the ARMs need to record the result of a cpuid/getauxval call
+// for the asm to work correctly, unless compiled without asm code.
+#define NEED_CPUID
+
+// OPENSSL_cpuid_setup initializes the platform-specific feature cache. This
+// function should not be called directly. Call |OPENSSL_init_cpuid| instead.
void OPENSSL_cpuid_setup(void);
+
+// OPENSSL_init_cpuid initializes the platform-specific feature cache, if
+// needed. This function is idempotent and may be called concurrently.
+void OPENSSL_init_cpuid(void);
+#else
+OPENSSL_INLINE void OPENSSL_init_cpuid(void) {}
#endif
#if (defined(OPENSSL_ARM) || defined(OPENSSL_AARCH64)) && \
!defined(OPENSSL_STATIC_ARMCAP)
// OPENSSL_get_armcap_pointer_for_test returns a pointer to |OPENSSL_armcap_P|
-// for unit tests. Any modifications to the value must be made after
-// |CRYPTO_library_init| but before any other function call in BoringSSL.
+// for unit tests. Any modifications to the value must be made before any other
+// function call in BoringSSL.
OPENSSL_EXPORT uint32_t *OPENSSL_get_armcap_pointer_for_test(void);
#endif
diff --git a/crypto/test/gtest_main.h b/crypto/test/gtest_main.h
index 05d468e..495b847 100644
--- a/crypto/test/gtest_main.h
+++ b/crypto/test/gtest_main.h
@@ -63,8 +63,6 @@
// SetupGoogleTest should be called by the test runner after
// testing::InitGoogleTest has been called and before RUN_ALL_TESTS.
inline void SetupGoogleTest() {
- CRYPTO_library_init();
-
#if defined(OPENSSL_WINDOWS)
// Initialize Winsock.
WORD wsa_version = MAKEWORD(2, 2);
diff --git a/crypto/thread_test.cc b/crypto/thread_test.cc
index 161c063..ea59c52 100644
--- a/crypto/thread_test.cc
+++ b/crypto/thread_test.cc
@@ -131,20 +131,6 @@
thread.join();
}
-TEST(ThreadTest, InitThreads) {
- constexpr size_t kNumThreads = 10;
-
- // |CRYPTO_library_init| is safe to call across threads.
- std::vector<std::thread> threads;
- threads.reserve(kNumThreads);
- for (size_t i = 0; i < kNumThreads; i++) {
- threads.emplace_back(&CRYPTO_library_init);
- }
- for (auto &thread : threads) {
- thread.join();
- }
-}
-
TEST(ThreadTest, PreSandboxInitThreads) {
constexpr size_t kNumThreads = 10;
diff --git a/include/openssl/crypto.h b/include/openssl/crypto.h
index 2981e68..63dbcb2 100644
--- a/include/openssl/crypto.h
+++ b/include/openssl/crypto.h
@@ -32,18 +32,9 @@
#endif
-// crypto.h contains functions for initializing the crypto library.
+// crypto.h contains functions for library-wide initialization and properties.
-// CRYPTO_library_init initializes the crypto library. It must be called if the
-// library is built with BORINGSSL_NO_STATIC_INITIALIZER. Otherwise, it does
-// nothing and a static initializer is used instead. It is safe to call this
-// function multiple times and concurrently from multiple threads.
-//
-// On some ARM configurations, this function may require filesystem access and
-// should be called before entering a sandbox.
-OPENSSL_EXPORT void CRYPTO_library_init(void);
-
// CRYPTO_is_confidential_build returns one if the linked version of BoringSSL
// has been built with the BORINGSSL_CONFIDENTIAL define and zero otherwise.
//
@@ -164,7 +155,7 @@
#define OPENSSL_INIT_NO_LOAD_CONFIG 0
#define OPENSSL_INIT_NO_ATEXIT 0
-// OPENSSL_init_crypto calls |CRYPTO_library_init| and returns one.
+// OPENSSL_init_crypto returns one.
OPENSSL_EXPORT int OPENSSL_init_crypto(uint64_t opts,
const OPENSSL_INIT_SETTINGS *settings);
@@ -199,6 +190,10 @@
OPENSSL_EXPORT int CRYPTO_has_broken_NEON(void);
#endif
+// CRYPTO_library_init does nothing. Historically, it was needed in some build
+// configurations to initialization the library. This is no longer necessary.
+OPENSSL_EXPORT void CRYPTO_library_init(void);
+
#if defined(__cplusplus)
} // extern C
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 9d7abe8..9f95a06 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -4864,7 +4864,7 @@
// Deprecated functions.
-// SSL_library_init calls |CRYPTO_library_init| and returns one.
+// SSL_library_init returns one.
OPENSSL_EXPORT int SSL_library_init(void);
// SSL_CIPHER_description writes a description of |cipher| into |buf| and
@@ -5427,7 +5427,7 @@
#define OPENSSL_INIT_LOAD_SSL_STRINGS 0
#define OPENSSL_INIT_SSL_DEFAULT 0
-// OPENSSL_init_ssl calls |CRYPTO_library_init| and returns one.
+// OPENSSL_init_ssl returns one.
OPENSSL_EXPORT int OPENSSL_init_ssl(uint64_t opts,
const OPENSSL_INIT_SETTINGS *settings);
diff --git a/rust/bssl-sys/src/lib.rs b/rust/bssl-sys/src/lib.rs
index 1d43e14..2d313af 100644
--- a/rust/bssl-sys/src/lib.rs
+++ b/rust/bssl-sys/src/lib.rs
@@ -55,8 +55,6 @@
CBS_len_RUST as CBS_len };
pub fn init() {
- // Safety: `CRYPTO_library_init` may be called multiple times and concurrently.
- unsafe {
- CRYPTO_library_init();
- }
+ // This function does nothing.
+ // TODO(davidben): Remove rust-openssl's dependency on this and remove this.
}
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 278c7a9..206a016 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -505,13 +505,9 @@
using namespace bssl;
-int SSL_library_init(void) {
- CRYPTO_library_init();
- return 1;
-}
+int SSL_library_init(void) { return 1; }
int OPENSSL_init_ssl(uint64_t opts, const OPENSSL_INIT_SETTINGS *settings) {
- CRYPTO_library_init();
return 1;
}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 5bf9af4..72793f2 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1374,8 +1374,6 @@
signal(SIGPIPE, SIG_IGN);
#endif
- CRYPTO_library_init();
-
TestConfig initial_config, resume_config, retry_config;
if (!ParseConfig(argc - 1, argv + 1, /*is_shim=*/true, &initial_config,
&resume_config, &retry_config)) {
diff --git a/tool/tool.cc b/tool/tool.cc
index 96bde33..7adfdb3 100644
--- a/tool/tool.cc
+++ b/tool/tool.cc
@@ -111,8 +111,6 @@
signal(SIGPIPE, SIG_IGN);
#endif
- CRYPTO_library_init();
-
int starting_arg = 1;
tool_func_t tool = nullptr;
#if !defined(OPENSSL_WINDOWS)
diff --git a/util/fipstools/test_fips.c b/util/fipstools/test_fips.c
index c779180..bb36853 100644
--- a/util/fipstools/test_fips.c
+++ b/util/fipstools/test_fips.c
@@ -48,8 +48,6 @@
}
int main(int argc, char **argv) {
- CRYPTO_library_init();
-
// Ensure that the output is line-buffered rather than fully buffered. When
// some of the tests fail, some of the output can otherwise be lost.
setvbuf(stdout, NULL, _IOLBF, 0);
