From fd330d0e7f334fd05fb6d44c0eced84a5243bcfa Mon Sep 17 00:00:00 2001 From: Lindsay Stewart Date: Fri, 5 May 2023 14:48:19 -0700 Subject: [PATCH] [ci]: Use custom library context for rc4 instead of global default context (#3980) --- bindings/rust/s2n-tls-sys/tests/s2n_init.rs | 7 +- crypto/s2n_cipher.h | 3 + crypto/s2n_libcrypto.c | 40 +------- crypto/s2n_libcrypto.h | 2 - crypto/s2n_stream_cipher_rc4.c | 105 ++++++++++++-------- tests/unit/s2n_rc4_test.c | 17 +++- tls/s2n_cipher_suites.c | 6 ++ utils/s2n_init.c | 2 - 8 files changed, 86 insertions(+), 96 deletions(-) diff --git a/bindings/rust/s2n-tls-sys/tests/s2n_init.rs b/bindings/rust/s2n-tls-sys/tests/s2n_init.rs index cc42c693299..52b4e8b3f0c 100644 --- a/bindings/rust/s2n-tls-sys/tests/s2n_init.rs +++ b/bindings/rust/s2n-tls-sys/tests/s2n_init.rs @@ -10,11 +10,8 @@ fn s2n_init_test() { std::env::set_var("S2N_DONT_MLOCK", "1"); // try to initialize the library - s2n_init(); - - // make sure it was successful - let error = *s2n_errno_location(); - if error != 0 { + if s2n_init() != 0 { + let error = *s2n_errno_location(); let msg = s2n_strerror_name(error); let msg = std::ffi::CStr::from_ptr(msg); panic!("s2n did not initialize correctly: {:?}", msg); diff --git a/crypto/s2n_cipher.h b/crypto/s2n_cipher.h index a58163025f8..18ac7cdccf7 100644 --- a/crypto/s2n_cipher.h +++ b/crypto/s2n_cipher.h @@ -107,3 +107,6 @@ extern const struct s2n_cipher s2n_chacha20_poly1305; extern const struct s2n_cipher s2n_tls13_aes128_gcm; extern const struct s2n_cipher s2n_tls13_aes256_gcm; + +S2N_RESULT s2n_rc4_init(); +S2N_RESULT s2n_rc4_cleanup(); diff --git a/crypto/s2n_libcrypto.c b/crypto/s2n_libcrypto.c index 0ad7d4706b5..d92da10a087 100644 --- a/crypto/s2n_libcrypto.c +++ b/crypto/s2n_libcrypto.c @@ -17,17 +17,13 @@ #include #include +#include #include "crypto/s2n_crypto.h" #include "crypto/s2n_fips.h" #include "crypto/s2n_openssl.h" #include "utils/s2n_safety.h" #include "utils/s2n_safety_macros.h" -#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0) - #include -#endif - -#include /* Note: OpenSSL 1.0.2 -> 1.1.0 implemented a new API to get the version number * and version name. We have to handle that by using old functions @@ -149,40 +145,6 @@ bool s2n_libcrypto_is_libressl() #endif } -S2N_RESULT s2n_libcrypto_init(void) -{ -#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0) - RESULT_ENSURE(OSSL_PROVIDER_load(NULL, "default") != NULL, S2N_ERR_OSSL_PROVIDER); - #ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_RC4 - /* needed to support RC4 algorithm - * https://www.openssl.org/docs/man3.0/man7/OSSL_PROVIDER-legacy.html - */ - RESULT_ENSURE(OSSL_PROVIDER_load(NULL, "legacy") != NULL, S2N_ERR_OSSL_PROVIDER); - #endif -#endif - - return S2N_RESULT_OK; -} - -#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0) -int s2n_libcrypto_cleanup_cb(OSSL_PROVIDER *provider, void *cbdata) -{ - return OSSL_PROVIDER_unload(provider); -} - -S2N_RESULT s2n_libcrypto_cleanup(void) -{ - RESULT_GUARD_OSSL(OSSL_PROVIDER_do_all(NULL, *s2n_libcrypto_cleanup_cb, NULL), S2N_ERR_ATEXIT); - - return S2N_RESULT_OK; -} -#else -S2N_RESULT s2n_libcrypto_cleanup(void) -{ - return S2N_RESULT_OK; -} -#endif - /* Performs various checks to validate that the libcrypto used at compile-time * is the same libcrypto being used at run-time. */ diff --git a/crypto/s2n_libcrypto.h b/crypto/s2n_libcrypto.h index c3b933562f1..34d9ef2aacc 100644 --- a/crypto/s2n_libcrypto.h +++ b/crypto/s2n_libcrypto.h @@ -17,6 +17,4 @@ #include "utils/s2n_result.h" -S2N_RESULT s2n_libcrypto_init(void); S2N_RESULT s2n_libcrypto_validate_runtime(void); -S2N_RESULT s2n_libcrypto_cleanup(void); diff --git a/crypto/s2n_stream_cipher_rc4.c b/crypto/s2n_stream_cipher_rc4.c index 85bf5bca90a..8514f41b332 100644 --- a/crypto/s2n_stream_cipher_rc4.c +++ b/crypto/s2n_stream_cipher_rc4.c @@ -21,20 +21,62 @@ #include "utils/s2n_blob.h" #include "utils/s2n_safety.h" -static uint8_t s2n_stream_cipher_rc4_available() +#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0) + #include "openssl/provider.h" +DEFINE_POINTER_CLEANUP_FUNC(OSSL_LIB_CTX *, OSSL_LIB_CTX_free); +#endif + +static EVP_CIPHER *s2n_rc4_cipher = NULL; + +S2N_RESULT s2n_rc4_init() +{ + /* In Openssl-3.0, RC4 is only available from the "legacy" provider, + * which is not loaded in the default library context. + */ +#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_RC4) && S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0) + DEFER_CLEANUP(OSSL_LIB_CTX *lib_ctx = OSSL_LIB_CTX_new(), OSSL_LIB_CTX_free_pointer); + RESULT_ENSURE_REF(lib_ctx); + RESULT_ENSURE_REF(OSSL_PROVIDER_load(lib_ctx, "legacy")); + s2n_rc4_cipher = EVP_CIPHER_fetch(lib_ctx, "rc4", "provider=legacy"); + RESULT_ENSURE_REF(s2n_rc4_cipher); +#endif + return S2N_RESULT_OK; +} + +S2N_RESULT s2n_rc4_cleanup() { -#ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_RC4 +#if S2N_OPENSSL_VERSION_AT_LEAST(3, 0, 0) + EVP_CIPHER_free(s2n_rc4_cipher); +#endif + return S2N_RESULT_OK; +} + +static S2N_RESULT s2n_get_rc4_cipher(const EVP_CIPHER **cipher) +{ + RESULT_ENSURE_REF(cipher); + *cipher = NULL; if (s2n_is_in_fips_mode()) { - return 0; + *cipher = NULL; + } else if (s2n_rc4_cipher) { + *cipher = s2n_rc4_cipher; +#if S2N_LIBCRYPTO_SUPPORTS_EVP_RC4 } else { - return (EVP_rc4() ? 1 : 0); + *cipher = EVP_rc4(); +#endif + } + RESULT_ENSURE(*cipher, S2N_ERR_UNIMPLEMENTED); + return S2N_RESULT_OK; +} + +static uint8_t s2n_stream_cipher_rc4_available() +{ + const EVP_CIPHER *cipher = NULL; + if (s2n_result_is_ok(s2n_get_rc4_cipher(&cipher)) && cipher) { + return 1; } -#else return 0; -#endif /* S2N_LIBCRYPTO_SUPPORTS_EVP_RC4 */ } -#ifdef S2N_LIBCRYPTO_SUPPORTS_EVP_RC4 static int s2n_stream_cipher_rc4_encrypt(struct s2n_session_key *key, struct s2n_blob *in, struct s2n_blob *out) { POSIX_ENSURE_GTE(out->size, in->size); @@ -64,17 +106,25 @@ static int s2n_stream_cipher_rc4_decrypt(struct s2n_session_key *key, struct s2n static int s2n_stream_cipher_rc4_set_encryption_key(struct s2n_session_key *key, struct s2n_blob *in) { POSIX_ENSURE_EQ(in->size, 16); - POSIX_GUARD_OSSL(EVP_EncryptInit_ex(key->evp_cipher_ctx, EVP_rc4(), NULL, in->data, NULL), S2N_ERR_KEY_INIT); - return 0; + const EVP_CIPHER *evp_rc4 = NULL; + POSIX_GUARD_RESULT(s2n_get_rc4_cipher(&evp_rc4)); + + POSIX_GUARD_OSSL(EVP_EncryptInit_ex(key->evp_cipher_ctx, evp_rc4, NULL, in->data, NULL), S2N_ERR_KEY_INIT); + + return S2N_SUCCESS; } static int s2n_stream_cipher_rc4_set_decryption_key(struct s2n_session_key *key, struct s2n_blob *in) { POSIX_ENSURE_EQ(in->size, 16); - POSIX_GUARD_OSSL(EVP_DecryptInit_ex(key->evp_cipher_ctx, EVP_rc4(), NULL, in->data, NULL), S2N_ERR_KEY_INIT); - return 0; + const EVP_CIPHER *evp_rc4 = NULL; + POSIX_GUARD_RESULT(s2n_get_rc4_cipher(&evp_rc4)); + + POSIX_GUARD_OSSL(EVP_DecryptInit_ex(key->evp_cipher_ctx, evp_rc4, NULL, in->data, NULL), S2N_ERR_KEY_INIT); + + return S2N_SUCCESS; } static int s2n_stream_cipher_rc4_init(struct s2n_session_key *key) @@ -90,39 +140,6 @@ static int s2n_stream_cipher_rc4_destroy_key(struct s2n_session_key *key) return 0; } -#else - -static int s2n_stream_cipher_rc4_encrypt(struct s2n_session_key *key, struct s2n_blob *in, struct s2n_blob *out) -{ - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); -} - -static int s2n_stream_cipher_rc4_decrypt(struct s2n_session_key *key, struct s2n_blob *in, struct s2n_blob *out) -{ - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); -} - -static int s2n_stream_cipher_rc4_set_encryption_key(struct s2n_session_key *key, struct s2n_blob *in) -{ - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); -} - -static int s2n_stream_cipher_rc4_set_decryption_key(struct s2n_session_key *key, struct s2n_blob *in) -{ - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); -} - -static int s2n_stream_cipher_rc4_init(struct s2n_session_key *key) -{ - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); -} - -static int s2n_stream_cipher_rc4_destroy_key(struct s2n_session_key *key) -{ - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); -} - -#endif /* S2N_LIBCRYPTO_SUPPORTS_EVP_RC4 */ const struct s2n_cipher s2n_rc4 = { .type = S2N_STREAM, diff --git a/tests/unit/s2n_rc4_test.c b/tests/unit/s2n_rc4_test.c index a66b2d79721..b65de079e26 100644 --- a/tests/unit/s2n_rc4_test.c +++ b/tests/unit/s2n_rc4_test.c @@ -55,11 +55,22 @@ int main(int argc, char **argv) conn->server = conn->secure; conn->client = conn->secure; + /* Make sure that RC4 is available when expected */ +#if defined(S2N_LIBCRYPTO_SUPPORTS_EVP_RC4) + if (s2n_is_in_fips_mode()) { + EXPECT_FALSE(s2n_rc4.is_available()); + } else { + EXPECT_TRUE(s2n_rc4.is_available()); + } +#else + EXPECT_FALSE(s2n_rc4.is_available()); +#endif + /* test the RC4 cipher with a SHA1 hash */ conn->secure->cipher_suite->record_alg = &s2n_record_alg_rc4_sha; + EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->server_key)); + EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->client_key)); if (conn->secure->cipher_suite->record_alg->cipher->is_available()) { - EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->server_key)); - EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->client_key)); EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->set_decryption_key(&conn->secure->client_key, &key_iv)); EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->set_encryption_key(&conn->secure->server_key, &key_iv)); EXPECT_SUCCESS(s2n_hmac_init(&conn->secure->client_record_mac, S2N_HMAC_SHA1, mac_key, sizeof(mac_key))); @@ -119,8 +130,6 @@ int main(int argc, char **argv) EXPECT_SUCCESS(conn->secure->cipher_suite->record_alg->cipher->destroy_key(&conn->secure->client_key)); EXPECT_SUCCESS(s2n_connection_free(conn)); } else { - EXPECT_FAILURE_WITH_ERRNO(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->server_key), S2N_ERR_UNIMPLEMENTED); - EXPECT_FAILURE_WITH_ERRNO(conn->secure->cipher_suite->record_alg->cipher->init(&conn->secure->client_key), S2N_ERR_UNIMPLEMENTED); EXPECT_FAILURE_WITH_ERRNO(conn->secure->cipher_suite->record_alg->cipher->set_decryption_key(&conn->secure->client_key, &key_iv), S2N_ERR_UNIMPLEMENTED); EXPECT_FAILURE_WITH_ERRNO(conn->secure->cipher_suite->record_alg->cipher->set_encryption_key(&conn->secure->server_key, &key_iv), S2N_ERR_UNIMPLEMENTED); } diff --git a/tls/s2n_cipher_suites.c b/tls/s2n_cipher_suites.c index e9c38dc0a6c..d20325270b3 100644 --- a/tls/s2n_cipher_suites.c +++ b/tls/s2n_cipher_suites.c @@ -971,6 +971,9 @@ int s2n_crypto_disable_init(void) /* Determines cipher suite availability and selects record algorithms */ int s2n_cipher_suites_init(void) { + /* RC4 requires some extra setup */ + POSIX_GUARD_RESULT(s2n_rc4_init()); + const int num_cipher_suites = s2n_array_len(s2n_all_cipher_suites); for (int i = 0; i < num_cipher_suites; i++) { struct s2n_cipher_suite *cur_suite = s2n_all_cipher_suites[i]; @@ -1052,6 +1055,9 @@ S2N_RESULT s2n_cipher_suites_cleanup(void) #endif } + /* RC4 requires some extra cleanup */ + RESULT_GUARD(s2n_rc4_cleanup()); + return S2N_RESULT_OK; } diff --git a/utils/s2n_init.c b/utils/s2n_init.c index 8f1b66e0f94..420ebf0e883 100644 --- a/utils/s2n_init.c +++ b/utils/s2n_init.c @@ -66,7 +66,6 @@ int s2n_init(void) POSIX_GUARD(s2n_mem_init()); /* Must run before any init method that calls libcrypto methods. */ POSIX_GUARD_RESULT(s2n_locking_init()); - POSIX_GUARD_RESULT(s2n_libcrypto_init()); POSIX_GUARD(s2n_fips_init()); POSIX_GUARD_RESULT(s2n_rand_init()); POSIX_GUARD(s2n_cipher_suites_init()); @@ -100,7 +99,6 @@ static bool s2n_cleanup_atexit_impl(void) bool cleaned_up = s2n_result_is_ok(s2n_cipher_suites_cleanup()) && s2n_result_is_ok(s2n_rand_cleanup_thread()) && s2n_result_is_ok(s2n_rand_cleanup()) - && s2n_result_is_ok(s2n_libcrypto_cleanup()) && s2n_result_is_ok(s2n_locking_cleanup()) && (s2n_mem_cleanup() == S2N_SUCCESS);