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>

15 files changed, 39 insertions, 120 deletions

diff --git a/PORTING.md b/PORTING.md
index eb951f5..3faf757 100644
--- a/PORTING.md
+++ b/PORTING.md
@@ -206,12 +206,8 @@ errors.
 
 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 @@ static int aead_chacha20_poly1305_init(EVP_AEAD_CTX *ctx, const uint8_t *key,
   // 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 uint8_t BORINGSSL_function_hit[7] = {0};
 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 =
 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 @@ int CRYPTO_has_asm(void) {
 
 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 @@ int ENGINE_register_all_complete(void) { return 1; }
 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 BORINGSSL_maybe_set_module_text_permissions(int permission) {}
 
 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 @@ DEFINE_METHOD_FUNCTION(EC_METHOD, EC_GFp_nistz256_method) {
   // 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 @@ extern "C" {
 #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 @@ class TestEventListener : public testing::EmptyTestEventListener {
 // 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 @@ TEST(ThreadTest, RandState) {
   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 @@ extern "C" {
 #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 @@ OPENSSL_EXPORT void OPENSSL_load_builtin_modules(void);
 #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 FIPS_query_algorithm_status(const char *algorithm);
 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 @@ OPENSSL_EXPORT void SSL_set_check_ecdsa_curve(SSL *ssl, int enable);
 
 // 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 @@ OPENSSL_EXPORT SSL_SESSION *SSL_get1_session(SSL *ssl);
 #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 @@ pub use { ERR_GET_LIB_RUST as ERR_GET_LIB,
           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 @@ BSSL_NAMESPACE_END
 
 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 @@ int main(int argc, char **argv) {
   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 @@ int main(int argc, char **argv) {
   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 @@ static void hexdump(const void *a, size_t len) {
 }
 
 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);