OERV-RUNTIME/boringssl
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make BoringSSL initialization-less

Browse Source 
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>
This commit is contained in:
David Benjamin
2024-06-18 01:13:09 -04:00
committed by Boringssl LUCI CQ
parent 6c98ebeb8c
commit 2fcdd11f6d
15 changed files with 39 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
PORTING.md
View File

@@ -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

2
crypto/cipher_extra/e_chacha20poly1305.c
View File

@@ -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;

68
crypto/crypto.c
View File

@@ -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;
}

2
crypto/fipsmodule/bcm.c
View File

@@ -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.

2
crypto/fipsmodule/ec/p256-nistz.c
View File

@@ -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;

22
crypto/internal.h
View File

@@ -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

2
crypto/test/gtest_main.h
View File

@@ -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);

14
crypto/thread_test.cc
View File

@@ -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;

17
include/openssl/crypto.h
View File

@@ -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

4
include/openssl/ssl.h
View File

@@ -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);

6
rust/bssl-sys/src/lib.rs
View File

@@ -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.
}

6
ssl/ssl_lib.cc
View File

@@ -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;
}

2
ssl/test/bssl_shim.cc
View File

@@ -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)) {

2
tool/tool.cc
View File

@@ -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)

2
util/fipstools/test_fips.c
View File

@@ -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);