From 6c98ebeb8cf24c7be5d462ded7e60d88b2ceccec Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 15 Jun 2024 09:52:09 -0400 Subject: [PATCH] Call CRYPTO_library_init before ChaCha20 and P-256 assembly We really should remove the ia32cap references from those files, but now that we're down to two files, let's go ahead and remove the CRYPTO_library_init requirement from our callers and close out the initialization hole. Notably, use of bssl-crypto in Chromium is slightly shaky without this. Although I think, prior to this CL, we'd already gotten to benign races being all that are possible because these two remaining spots don't change any in-memory representations. (Unlike C/C++, benign races from assembly are actually well-defined and truly benign.) But no sense in relying on this when we can just fix it directly. This CL just adds some explicit CRYPTO_library_init calls. A subsequent one will update the docs and clean up all the remnants of our messy initialization story. Bug: 40644931 Change-Id: Ife288a4817b930473210f43a2680a60b040bf9a0 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69507 Commit-Queue: David Benjamin Commit-Queue: Adam Langley Auto-Submit: David Benjamin Reviewed-by: Adam Langley --- crypto/cipher_extra/e_chacha20poly1305.c | 7 +++++++ crypto/fipsmodule/ec/p256-nistz.c | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/crypto/cipher_extra/e_chacha20poly1305.c b/crypto/cipher_extra/e_chacha20poly1305.c index 6510ff48a..3ac2af82f 100644 --- a/crypto/cipher_extra/e_chacha20poly1305.c +++ b/crypto/cipher_extra/e_chacha20poly1305.c @@ -41,6 +41,13 @@ static_assert(alignof(union evp_aead_ctx_st_state) >= static int aead_chacha20_poly1305_init(EVP_AEAD_CTX *ctx, const uint8_t *key, size_t key_len, size_t tag_len) { + // TODO(crbug.com/42290548): The x86_64 assembly depends on initializing + // |OPENSSL_ia32cap_P|. Move the dispatch to C. While we're here, it may be + // 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(); + struct aead_chacha20_poly1305_ctx *c20_ctx = (struct aead_chacha20_poly1305_ctx *)&ctx->state; diff --git a/crypto/fipsmodule/ec/p256-nistz.c b/crypto/fipsmodule/ec/p256-nistz.c index cf0996382..277382046 100644 --- a/crypto/fipsmodule/ec/p256-nistz.c +++ b/crypto/fipsmodule/ec/p256-nistz.c @@ -612,6 +612,11 @@ static int ecp_nistz256_cmp_x_coordinate(const EC_GROUP *group, } 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(); + out->point_get_affine_coordinates = ecp_nistz256_get_affine; out->add = ecp_nistz256_add; out->dbl = ecp_nistz256_dbl;