From a3bf4a248661fd2f71d69c922e89672fcf315a46 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Wed, 25 Jan 2017 14:13:34 -0800 Subject: [PATCH] crypto: support OPENSSL_CONF again A side-effect of https://github.com/nodejs/node-private/pull/82 was to remove support for OPENSSL_CONF, as well as removing the default read of a configuration file on startup. Partly revert this, allowing OPENSSL_CONF to be used to specify a configuration file to read on startup, but do not read a file by default. If the --openssl-config command line option is provided, its value is used, not the OPENSSL_CONF environment variable. Fix: https://github.com/nodejs/node/issues/10938 PR-URL: https://github.com/nodejs/node/pull/11006 Reviewed-By: Michael Dawson Reviewed-By: Ben Noordhuis --- doc/api/cli.md | 13 +++++++++++++ doc/node.1 | 10 ++++++++++ src/node.cc | 14 ++++++++++---- src/node_crypto.cc | 4 ++-- src/node_internals.h | 2 +- test/parallel/test-crypto-fips.js | 25 ++++++++++++++++++++++--- 6 files changed, 58 insertions(+), 10 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 809bd326a6273c..099c4c993d113c 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -363,6 +363,18 @@ misformatted, but any errors are otherwise ignored. Note that neither the well known nor extra certificates are used when the `ca` options property is explicitly specified for a TLS or HTTPS client or server. +### `OPENSSL_CONF=file` + + +Load an OpenSSL configuration file on startup. Among other uses, this can be +used to enable FIPS-compliant crypto if Node.js is built with `./configure +\-\-openssl\-fips`. + +If the [`--openssl-config`][] command line option is used, the environment +variable is ignored. + ### `SSL_CERT_DIR=dir` If `--use-openssl-ca` is enabled, this overrides and sets OpenSSL's directory @@ -386,3 +398,4 @@ OpenSSL, it may cause them to trust the same CAs as node. [debugger]: debugger.html [REPL]: repl.html [SlowBuffer]: buffer.html#buffer_class_slowbuffer +[`--openssl-config`]: #cli_openssl_config_file diff --git a/doc/node.1 b/doc/node.1 index ffe72e1d022aa6..026485d3920923 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -243,6 +243,16 @@ asynchronous when outputting to a TTY on platforms which support async stdio. Setting this will void any guarantee that stdio will not be interleaved or dropped at program exit. \fBAvoid use.\fR +.TP +.BR OPENSSL_CONF = \fIfile\fR +Load an OpenSSL configuration file on startup. Among other uses, this can be +used to enable FIPS-compliant crypto if Node.js is built with +\fB./configure \-\-openssl\-fips\fR. + +If the +\fB\-\-openssl\-config\fR +command line option is used, the environment variable is ignored. + .TP .BR SSL_CERT_DIR = \fIdir\fR If \fB\-\-use\-openssl\-ca\fR is enabled, this overrides and sets OpenSSL's directory diff --git a/src/node.cc b/src/node.cc index 0e35fea72da40d..f19a2867f85f71 100644 --- a/src/node.cc +++ b/src/node.cc @@ -174,7 +174,7 @@ bool ssl_openssl_cert_store = bool enable_fips_crypto = false; bool force_fips_crypto = false; # endif // NODE_FIPS_MODE -const char* openssl_config = nullptr; +std::string openssl_config; // NOLINT(runtime/string) #endif // HAVE_OPENSSL // true if process warnings should be suppressed @@ -3519,8 +3519,9 @@ static void PrintHelp() { " --enable-fips enable FIPS crypto at startup\n" " --force-fips force FIPS crypto (cannot be disabled)\n" #endif /* NODE_FIPS_MODE */ - " --openssl-config=path load OpenSSL configuration file from\n" - " the specified path\n" + " --openssl-config=file load OpenSSL configuration from the\n" + " specified file (overrides\n" + " OPENSSL_CONF)\n" #endif /* HAVE_OPENSSL */ #if defined(NODE_HAVE_I18N_SUPPORT) " --icu-data-dir=dir set ICU data load path to dir\n" @@ -3553,6 +3554,8 @@ static void PrintHelp() { " prefixed to the module search path\n" "NODE_REPL_HISTORY path to the persistent REPL history\n" " file\n" + "OPENSSL_CONF load OpenSSL configuration from file\n" + "\n" "Documentation can be found at https://nodejs.org/\n"); } @@ -3690,7 +3693,7 @@ static void ParseArgs(int* argc, force_fips_crypto = true; #endif /* NODE_FIPS_MODE */ } else if (strncmp(arg, "--openssl-config=", 17) == 0) { - openssl_config = arg + 17; + openssl_config.assign(arg + 17); #endif /* HAVE_OPENSSL */ #if defined(NODE_HAVE_I18N_SUPPORT) } else if (strncmp(arg, "--icu-data-dir=", 15) == 0) { @@ -4187,6 +4190,9 @@ void Init(int* argc, SafeGetenv("NODE_PRESERVE_SYMLINKS", &text) && text[0] == '1'; } + if (openssl_config.empty()) + SafeGetenv("OPENSSL_CONF", &openssl_config); + // Parse a few arguments which are specific to Node. int v8_argc; const char** v8_argv; diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 072a48f40c58dc..90e54f6398d859 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5881,14 +5881,14 @@ void InitCryptoOnce() { OPENSSL_no_config(); // --openssl-config=... - if (openssl_config != nullptr) { + if (!openssl_config.empty()) { OPENSSL_load_builtin_modules(); #ifndef OPENSSL_NO_ENGINE ENGINE_load_builtin_engines(); #endif ERR_clear_error(); CONF_modules_load_file( - openssl_config, + openssl_config.c_str(), nullptr, CONF_MFLAGS_DEFAULT_SECTION); int err = ERR_get_error(); diff --git a/src/node_internals.h b/src/node_internals.h index adfb453895a650..f786adcdb3121c 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -36,7 +36,7 @@ namespace node { // Set in node.cc by ParseArgs with the value of --openssl-config. // Used in node_crypto.cc when initializing OpenSSL. -extern const char* openssl_config; +extern std::string openssl_config; // Set in node.cc by ParseArgs when --preserve-symlinks is used. // Used in node_config.cc to set a constant on process.binding('config') diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js index 6fd3352740c8c3..ae510741943ca8 100644 --- a/test/parallel/test-crypto-fips.js +++ b/test/parallel/test-crypto-fips.js @@ -37,8 +37,9 @@ function testHelper(stream, args, expectedOutput, cmd, env) { env: env }); - console.error('Spawned child [pid:' + child.pid + '] with cmd ' + - cmd + ' and args \'' + args + '\''); + console.error('Spawned child [pid:' + child.pid + '] with cmd \'' + + cmd + '\' expect %j with args \'' + args + '\'' + + ' OPENSSL_CONF=%j', expectedOutput, env.OPENSSL_CONF); function childOk(child) { console.error('Child #' + ++num_children_ok + @@ -92,10 +93,26 @@ testHelper( compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED, 'require("crypto").fips', process.env); -// OPENSSL_CONF should _not_ be able to turn on FIPS mode + +// OPENSSL_CONF should be able to turn on FIPS mode testHelper( 'stdout', [], + compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED, + 'require("crypto").fips', + addToEnv('OPENSSL_CONF', CNF_FIPS_ON)); + +// --openssl-config option should override OPENSSL_CONF +testHelper( + 'stdout', + [`--openssl-config=${CNF_FIPS_ON}`], + compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED, + 'require("crypto").fips', + addToEnv('OPENSSL_CONF', CNF_FIPS_OFF)); + +testHelper( + 'stdout', + [`--openssl-config=${CNF_FIPS_OFF}`], FIPS_DISABLED, 'require("crypto").fips', addToEnv('OPENSSL_CONF', CNF_FIPS_ON)); @@ -107,6 +124,7 @@ testHelper( compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING, 'require("crypto").fips', process.env); + // OPENSSL_CONF should _not_ make a difference to --enable-fips testHelper( compiledWithFips() ? 'stdout' : 'stderr', @@ -122,6 +140,7 @@ testHelper( compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING, 'require("crypto").fips', process.env); + // Using OPENSSL_CONF should not make a difference to --force-fips testHelper( compiledWithFips() ? 'stdout' : 'stderr',