From dc866b4992c3734db64e18d7483740dc8517dd99 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Sat, 14 May 2022 10:04:48 +0200 Subject: [PATCH] src,deps,build,test: add OpenSSL config appname This commit adds the setting of an appname (configuration section name), 'nodejs_conf', to be used when reading OpenSSL configuration files. The motivation for this is that currently the default OpenSSL configuration, 'openssl_conf', element will be used which may be undesirable as it might configure OpenSSL in unwanted ways. With this commit it is still possible to use a default openssl.cnf file but the only section that Node.js will read from is a section named 'nodejs_conf'. PR-URL: https://github.com/nodejs/node/pull/43124 Refs: https://github.com/nodejs/node/issues/40366 Reviewed-By: James M Snell Reviewed-By: Rich Trott Reviewed-By: Rafael Gonzaga Reviewed-By: Beth Griggs --- BUILDING.md | 17 +++++++++ configure.py | 8 ++++ deps/openssl/nodejs-openssl.cnf | 30 +++++++++++++++ node.gyp | 10 ++--- src/node.cc | 51 ++++++++++++++++--------- test/fixtures/openssl_fips_disabled.cnf | 2 +- test/fixtures/openssl_fips_enabled.cnf | 2 +- test/parallel/test-crypto-fips.js | 2 +- 8 files changed, 96 insertions(+), 26 deletions(-) create mode 100644 deps/openssl/nodejs-openssl.cnf diff --git a/BUILDING.md b/BUILDING.md index 1e36403f53e454..10ef85eb88ad05 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -52,6 +52,7 @@ file a new issue. * [Build with a specific ICU](#build-with-a-specific-icu) * [Unix/macOS](#unixmacos-3) * [Windows](#windows-4) +* [Configuring OpenSSL config appname](#configure-openssl-appname) * [Building Node.js with FIPS-compliant OpenSSL](#building-nodejs-with-fips-compliant-openssl) * [Building Node.js with external core modules](#building-nodejs-with-external-core-modules) * [Unix/macOS](#unixmacos-4) @@ -768,6 +769,19 @@ as `deps/icu` (You'll have: `deps/icu/source/...`) > .\vcbuild full-icu ``` +### Configure OpenSSL appname + +Node.js can use an OpenSSL configuration file by specifying the environment +variable `OPENSSL_CONF`, or using the command line option `--openssl-conf`, and +if none of those are specified will default to reading the default OpenSSL +configuration file `openssl.cnf`. Node.js will only read a section that is by +default named `nodejs_conf`, but this name can be overridden using the following +configure option: + +```console +$ ./configure --openssl-conf-name= +``` + ## Building Node.js with FIPS-compliant OpenSSL The current version of Node.js supports FIPS when statically and @@ -819,6 +833,9 @@ $ ls out/Release/obj.target/deps/openssl/lib/openssl-modules/ fips.so ``` +Running `configure` without `--openssl-is-fips` flag and rebuilding will reset +the FIPS configuration. + ### FIPS support when dynamically linking OpenSSL For quictls/openssl 3.0 it is possible to enable FIPS when dynamically linking. diff --git a/configure.py b/configure.py index 187c381660b369..e53a83f332ffd6 100755 --- a/configure.py +++ b/configure.py @@ -181,6 +181,12 @@ "e.g. /root/x/y.js will be referenced via require('root/x/y'). " "Can be used multiple times") +parser.add_argument("--openssl-conf-name", + action="store", + dest="openssl_conf_name", + default='nodejs_conf', + help="The OpenSSL config appname (config section name) used by Node.js") + parser.add_argument('--openssl-default-cipher-list', action='store', dest='openssl_default_cipher_list', @@ -1488,6 +1494,8 @@ def configure_openssl(o): if options.openssl_no_asm: variables['openssl_no_asm'] = 1 + o['defines'] += ['NODE_OPENSSL_CONF_NAME=' + options.openssl_conf_name] + if options.without_ssl: def without_ssl_error(option): error('--without-ssl is incompatible with %s' % option) diff --git a/deps/openssl/nodejs-openssl.cnf b/deps/openssl/nodejs-openssl.cnf new file mode 100644 index 00000000000000..a4c2ff54a8c68e --- /dev/null +++ b/deps/openssl/nodejs-openssl.cnf @@ -0,0 +1,30 @@ + # Use this in order to automatically load providers. +nodejs_conf = openssl_init + +# Optionally include a file that is generated by the OpenSSL fipsinstall +# application. This file contains configuration data required by the OpenSSL +# fips provider. It contains a named section e.g. [fips_sect] which is +# referenced from the [provider_sect] below. +# Refer to the OpenSSL security policy for more information. +# .include fipsmodule.cnf + +[openssl_init] +providers = provider_sect + +# List of providers to load +[provider_sect] +default = default_sect +# The fips section name should match the section name inside the +# included fipsmodule.cnf. +# fips = fips_sect + +# If no providers are activated explicitly, the default one is activated implicitly. +# See man 7 OSSL_PROVIDER-default for more details. +# +# If you add a section explicitly activating any other provider(s), you most +# probably need to explicitly activate the default provider, otherwise it +# becomes unavailable in openssl. As a consequence applications depending on +# OpenSSL may not work correctly which could lead to significant system +# problems including inability to remotely access the system. +[default_sect] +# activate = 1 diff --git a/node.gyp b/node.gyp index e36617232dd7cc..d063a6408072ac 100644 --- a/node.gyp +++ b/node.gyp @@ -364,7 +364,7 @@ 'variables': { 'openssl-cli': '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)openssl-cli<(EXECUTABLE_SUFFIX)', 'provider_name': 'libopenssl-fipsmodule', - 'opensslconfig': './deps/openssl/openssl/apps/openssl.cnf', + 'opensslconfig': './deps/openssl/nodejs-openssl.cnf', 'conditions': [ ['GENERATOR == "ninja"', { 'fipsmodule_internal': '<(PRODUCT_DIR)/lib/<(provider_name).so', @@ -374,7 +374,7 @@ }, { 'fipsmodule_internal': '<(PRODUCT_DIR)/obj.target/deps/openssl/<(provider_name).so', 'fipsmodule': '<(PRODUCT_DIR)/obj.target/deps/openssl/lib/openssl-modules/fips.so', - 'fipsconfig': '<(PRODUCT_DIR)/obj/deps/openssl/fipsmodule.cnf', + 'fipsconfig': '<(PRODUCT_DIR)/obj.target/deps/openssl/fipsmodule.cnf', 'opensslconfig_internal': '<(PRODUCT_DIR)/obj.target/deps/openssl/openssl.cnf', }], ], @@ -426,7 +426,7 @@ }, { 'variables': { 'opensslconfig_internal': '<(obj_dir)/deps/openssl/openssl.cnf', - 'opensslconfig': './deps/openssl/openssl/apps/openssl.cnf', + 'opensslconfig': './deps/openssl/nodejs-openssl.cnf', }, 'actions': [ { @@ -435,8 +435,8 @@ 'outputs': [ '<(opensslconfig_internal)', ], 'action': [ 'python', 'tools/copyfile.py', - './deps/openssl/openssl/apps/openssl.cnf', - '<(obj_dir)/deps/openssl/openssl.cnf', + '<(opensslconfig)', + '<(opensslconfig_internal)', ], }, ], diff --git a/src/node.cc b/src/node.cc index 9919ddb5f1b37b..4fcad0112bef5b 100644 --- a/src/node.cc +++ b/src/node.cc @@ -162,6 +162,9 @@ PVOID old_vectored_exception_handler; struct V8Platform v8_platform; } // namespace per_process +// The section in the OpenSSL configuration file to be loaded. +const char* conf_section_name = STRINGIFY(NODE_OPENSSL_CONF_NAME); + #ifdef __POSIX__ void SignalExit(int signo, siginfo_t* info, void* ucontext) { ResetStdio(); @@ -1084,27 +1087,39 @@ InitializationResult InitializeOncePerProcess( // 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. + + // Passing NULL as the config file will allow the default openssl.cnf file + // to be loaded, but the default section in that file will not be used, + // instead only the section that matches the value of conf_section_name + // will be read from the default configuration file. + // fprintf(stderr, "appanme: %s\n", conf_section_name); + const char* conf_file = nullptr; + // Use OPENSSL_CONF environment variable is set. std::string env_openssl_conf; credentials::SafeGetenv("OPENSSL_CONF", &env_openssl_conf); + if (!env_openssl_conf.empty()) { + conf_file = env_openssl_conf.c_str(); + } + // Use --openssl-conf command line option if specified. + if (!per_process::cli_options->openssl_config.empty()) { + conf_file = per_process::cli_options->openssl_config.c_str(); + } - bool has_cli_conf = !per_process::cli_options->openssl_config.empty(); - if (has_cli_conf || !env_openssl_conf.empty()) { - OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new(); - 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); - - 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; - } + OPENSSL_INIT_SETTINGS* settings = OPENSSL_INIT_new(); + OPENSSL_INIT_set_config_filename(settings, conf_file); + OPENSSL_INIT_set_config_appname(settings, conf_section_name); + OPENSSL_INIT_set_config_file_flags(settings, + CONF_MFLAGS_IGNORE_MISSING_FILE); + + 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 // OPENSSL_VERSION_MAJOR < 3 if (FIPS_mode()) { diff --git a/test/fixtures/openssl_fips_disabled.cnf b/test/fixtures/openssl_fips_disabled.cnf index 8668370fac52f7..253c6906e3f3a3 100644 --- a/test/fixtures/openssl_fips_disabled.cnf +++ b/test/fixtures/openssl_fips_disabled.cnf @@ -1,6 +1,6 @@ # Skeleton openssl.cnf for testing with FIPS -openssl_conf = openssl_conf_section +nodejs_conf = openssl_conf_section authorityKeyIdentifier=keyid:always,issuer:always [openssl_conf_section] diff --git a/test/fixtures/openssl_fips_enabled.cnf b/test/fixtures/openssl_fips_enabled.cnf index 9c1a90f5087727..79733c657a96d0 100644 --- a/test/fixtures/openssl_fips_enabled.cnf +++ b/test/fixtures/openssl_fips_enabled.cnf @@ -1,6 +1,6 @@ # Skeleton openssl.cnf for testing with FIPS -openssl_conf = openssl_conf_section +nodejs_conf = openssl_conf_section authorityKeyIdentifier=keyid:always,issuer:always [openssl_conf_section] diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js index ba8a1ba653ec55..cc3e4eed79d80f 100644 --- a/test/parallel/test-crypto-fips.js +++ b/test/parallel/test-crypto-fips.js @@ -83,7 +83,7 @@ testHelper( [], FIPS_DISABLED, 'require("crypto").getFips()', - { ...process.env, 'OPENSSL_CONF': '' }); + { ...process.env, 'OPENSSL_CONF': ' ' }); // This should succeed for both FIPS and non-FIPS builds in combination with // OpenSSL 1.1.1 or OpenSSL 3.0