Skip to content

Commit

Permalink
crypto: support OPENSSL_CONF again
Browse files Browse the repository at this point in the history
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: nodejs/node#10938
PR-URL: nodejs/node#11006
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
sam-github authored and andrew749 committed Jul 19, 2017
1 parent 403ace3 commit a89c6bc
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 9 deletions.
13 changes: 13 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
<!-- YAML
added: REPLACEME
-->

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
10 changes: 10 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 7 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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");
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
25 changes: 22 additions & 3 deletions test/parallel/test-crypto-fips.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 +
Expand Down Expand Up @@ -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));
Expand All @@ -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',
Expand All @@ -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',
Expand Down

0 comments on commit a89c6bc

Please sign in to comment.