From a89c6bce5252d85ff585f1bf2ebf7e4e5fc7ee7e 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 | 10 +++++++--- src/node_crypto.cc | 4 ++-- src/node_internals.h | 2 +- test/parallel/test-crypto-fips.js | 25 ++++++++++++++++++++++--- 6 files changed, 55 insertions(+), 9 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 94e4f250c94..74210a88eea 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -328,8 +328,21 @@ malformed, 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. + [emit_warning]: process.html#process_process_emitwarning_warning_name_ctor [Buffer]: buffer.html#buffer_buffer [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 7b39ef05ce9..684eec03294 100644 --- a/doc/node.1 +++ b/doc/node.1 @@ -215,6 +215,16 @@ 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. + .SH BUGS Bugs are tracked in GitHub Issues: diff --git a/src/node.cc b/src/node.cc index 84a84d48747..aee6fe93d13 100644 --- a/src/node.cc +++ b/src/node.cc @@ -179,7 +179,7 @@ bool no_deprecation = false; 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 @@ -3709,7 +3709,7 @@ static void PrintHelp() { " --force-fips force FIPS crypto (cannot be disabled)\n" #endif /* NODE_FIPS_MODE */ " --openssl-config=path load OpenSSL configuration file from the\n" - " specified path\n" + " specified file (overrides OPENSSL_CONF)\n" #endif /* HAVE_OPENSSL */ #if defined(NODE_HAVE_I18N_SUPPORT) " --icu-data-dir=dir set ICU data load path to dir\n" @@ -3744,6 +3744,7 @@ static void PrintHelp() { #endif " prefixed to the module search path\n" "NODE_REPL_HISTORY path to the persistent REPL history file\n" + "OPENSSL_CONF load OpenSSL configuration from file\n" "\n" "Documentation can be found at https://nodejs.org/\n"); } @@ -3878,7 +3879,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) { @@ -4373,6 +4374,9 @@ void Init(int* argc, V8::SetFlagsFromString(NODE_V8_OPTIONS, sizeof(NODE_V8_OPTIONS) - 1); #endif + 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 ab2fc0cb387..c30e78e68f1 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -5904,14 +5904,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 72d303b8662..cb71720e820 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 147ccb44381..0b5c0a04584 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',