From 86fa07ce83e08dc0dd9f4745cf15a5527fc2104d Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 18 May 2021 13:45:05 +0200 Subject: [PATCH 1/3] src: set CONF_MFLAGS_DEFAULT_SECTION for OpenSSL 3 This commit adds a call to OPENSSL_init_crypto to initialize OPENSSL_INIT_LOAD_CONFIG to avoid the default behavior where errors raised during the parsing of the OpenSSL configuration file are not propagated and cannot be detected. The motivation for this is that if FIPS is configured the OpenSSL configuration file will have an .include pointing to the fipsmodule.cnf file generated by the openssl fipsinstall command. If the path to this file is incorrect no error will be reported. For Node.js this will mean that EntropySource will be called by V8 as part of its initalization process, and EntropySource will in turn call CheckEntropy. CheckEntropy will call RAND_status which will now always return 0 leading to an endless loop and the node process will appear to hang/freeze. I'll continue investigating the cause of this and see if this is expected behavior or not, but in the mean time it would be good to be able to workaround this issue with this commit. Refs: https://github.com/nodejs/node/pull/38633#pullrequestreview-658811317 --- src/crypto/crypto_util.cc | 2 +- src/node.cc | 40 ++++++++++++++++++++++++-- test/parallel/test-cli-node-options.js | 3 +- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 0d533ce42531d1..54bce2344e6916 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -106,7 +106,7 @@ int NoPasswordCallback(char* buf, int size, int rwflag, void* u) { } void InitCryptoOnce() { -#ifndef OPENSSL_IS_BORINGSSL +#if !defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_MAJOR < 3 OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new(); // --openssl-config=... diff --git a/src/node.cc b/src/node.cc index bf041fb682a2c2..c1a4570fbbc723 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1024,12 +1024,48 @@ InitializationResult InitializeOncePerProcess(int argc, char** argv) { // In the case of FIPS builds we should make sure // the random source is properly initialized first. #if OPENSSL_VERSION_MAJOR >= 3 - if (EVP_default_properties_is_fips_enabled(nullptr)) { + // Call OPENSSL_init_crypto to initialize OPENSSL_INIT_LOAD_CONFIG to + // avoid the default behavior where errors raised during the parsing of the + // OpenSSL configuration file are not propagated and cannot be detected. + // + // If FIPS is configured the OpenSSL configuration file will have an .include + // pointing to the fipsmodule.cnf file generated by the openssl fipsinstall + // command. If the path to this file is incorrect no error will be reported. + // + // For Node.js this will mean that EntropySource will be called by V8 as part + // of its initalization process, and EntropySource will in turn call + // CheckEntropy. CheckEntropy will call RAND_status which will now always + // return 0, leading to an endless loop and the node process will appear to + // hang/freeze. + std::string env_openssl_conf; + credentials::SafeGetenv("OPENSSL_CONF", &env_openssl_conf); + + bool has_cli_conf = !per_process::cli_options->openssl_config.empty(); + bool has_env_conf = !env_openssl_conf.empty(); + + if (has_cli_conf || has_env_conf) { + const char* conf = has_cli_conf ? + per_process::cli_options->openssl_config.c_str() : + env_openssl_conf.c_str(); + OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new(); + OPENSSL_INIT_set_config_filename(settings, conf); + OPENSSL_INIT_set_config_file_flags(settings, CONF_MFLAGS_DEFAULT_SECTION); + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, settings); + OPENSSL_INIT_free(settings); + + if (ERR_peek_error() != 0) { + result.exit_code = ERR_GET_REASON(ERR_peek_error()); + result.early_return = true; + fprintf(stderr, "OpenSSL configuration error:\n"); + ERR_print_errors_fp(stderr); + return result; + } + } #else if (FIPS_mode()) { OPENSSL_init(); -#endif } +#endif // V8 on Windows doesn't have a good source of entropy. Seed it from // OpenSSL's pool. V8::SetEntropySource(crypto::EntropySource); diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js index 8fb15d3ba505c1..0e824fe073dc94 100644 --- a/test/parallel/test-cli-node-options.js +++ b/test/parallel/test-cli-node-options.js @@ -61,7 +61,8 @@ if (common.isLinux) { if (common.hasCrypto) { expectNoWorker('--use-openssl-ca', 'B\n'); expectNoWorker('--use-bundled-ca', 'B\n'); - expectNoWorker('--openssl-config=_ossl_cfg', 'B\n'); + if (!common.hasOpenSSL3) + expectNoWorker('--openssl-config=_ossl_cfg', 'B\n'); } // V8 options From 7a8579f3af96d05cff192fe9bfe95cb797de745e Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 20 May 2021 05:37:44 +0200 Subject: [PATCH 2/3] squash! src: set CONF_MFLAGS_DEFAULT_SECTION for OpenSSL 3 Remove setting of config_filename when OPENSSL_CONF environment variable is specified (only set it for --openssl-config Node.js option. --- src/node.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/node.cc b/src/node.cc index c1a4570fbbc723..b7dd3e10cdb1c9 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1041,15 +1041,13 @@ InitializationResult InitializeOncePerProcess(int argc, char** argv) { credentials::SafeGetenv("OPENSSL_CONF", &env_openssl_conf); bool has_cli_conf = !per_process::cli_options->openssl_config.empty(); - bool has_env_conf = !env_openssl_conf.empty(); - - if (has_cli_conf || has_env_conf) { - const char* conf = has_cli_conf ? - per_process::cli_options->openssl_config.c_str() : - env_openssl_conf.c_str(); + if (has_cli_conf || !env_openssl_conf.empty()) { OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new(); - OPENSSL_INIT_set_config_filename(settings, conf); OPENSSL_INIT_set_config_file_flags(settings, CONF_MFLAGS_DEFAULT_SECTION); + if (has_cli_conf) { + const char* conf = per_process::cli_options->openssl_config.c_str(); + OPENSSL_INIT_set_config_filename(settings, conf); + } OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, settings); OPENSSL_INIT_free(settings); From ae154901f3b8eba80f3336e8520198c80377b05a Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 20 May 2021 05:50:49 +0200 Subject: [PATCH 3/3] squash! src: set CONF_MFLAGS_DEFAULT_SECTION for OpenSSL 3 Only skip the settting of the config_filename for OpenSSL 3 and not the OPENSSL_init_ssl. --- src/crypto/crypto_util.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc index 54bce2344e6916..47945389b4068d 100644 --- a/src/crypto/crypto_util.cc +++ b/src/crypto/crypto_util.cc @@ -106,14 +106,16 @@ int NoPasswordCallback(char* buf, int size, int rwflag, void* u) { } void InitCryptoOnce() { -#if !defined(OPENSSL_IS_BORINGSSL) && OPENSSL_VERSION_MAJOR < 3 +#ifndef OPENSSL_IS_BORINGSSL OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new(); +#if OPENSSL_VERSION_MAJOR < 3 // --openssl-config=... if (!per_process::cli_options->openssl_config.empty()) { const char* conf = per_process::cli_options->openssl_config.c_str(); OPENSSL_INIT_set_config_filename(settings, conf); } +#endif OPENSSL_init_ssl(0, settings); OPENSSL_INIT_free(settings);