Skip to content

Commit

Permalink
crypto: fix openssl.cnf FIPS handling & testing
Browse files Browse the repository at this point in the history
* Add documentation for `--openssl-conf=file`.
* Fix openssl.cnf loading and OpenSSL init ordering
* Fix FIPS tests so `OPENSSL_CONF` is not longer usable but
  `--openssl-conf` is

PR-URL: nodejs-private/node-private#82
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
rvagg authored and jasnell committed Oct 19, 2016
1 parent c32be9a commit 3518372
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 14 deletions.
10 changes: 10 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ Force FIPS-compliant crypto on startup. (Cannot be disabled from script code.)
(Same requirements as `--enable-fips`)


### `--openssl-config=file`
<!-- YAML
added: v6.9.0
-->

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`.


### `--icu-data-dir=file`
<!-- YAML
added: v0.11.15
Expand Down
6 changes: 6 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ Enable FIPS-compliant crypto at startup. (Requires Node.js to be built with
Force FIPS-compliant crypto on startup. (Cannot be disabled from script code.)
(Same requirements as \fB\-\-enable\-fips\fR)

.TP
.BR \-\-openssl\-config =\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.

.TP
.BR \-\-icu\-data\-dir =\fIfile\fR
Specify ICU data load path. (overrides \fBNODE_ICU_DATA\fR)
Expand Down
7 changes: 2 additions & 5 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,10 @@ typedef intptr_t ssize_t;
namespace node {

NODE_EXTERN extern bool no_deprecation;
#if HAVE_OPENSSL
# if NODE_FIPS_MODE
#if HAVE_OPENSSL && NODE_FIPS_MODE
NODE_EXTERN extern bool enable_fips_crypto;
NODE_EXTERN extern bool force_fips_crypto;
# endif // NODE_FIPS_MODE
NODE_EXTERN extern const char* openssl_config;
#endif // HAVE_OPENSSL
#endif

NODE_EXTERN int Start(int argc, char *argv[]);
NODE_EXTERN void Init(int* argc,
Expand Down
9 changes: 7 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5767,14 +5767,20 @@ void TimingSafeEqual(const FunctionCallbackInfo<Value>& args) {
}

void InitCryptoOnce() {
SSL_load_error_strings();
OPENSSL_no_config();

// --openssl-config=...
if (openssl_config != nullptr) {
OPENSSL_load_builtin_modules();
#ifndef OPENSSL_NO_ENGINE
ENGINE_load_builtin_engines();
#endif
ERR_clear_error();
CONF_modules_load_file(
openssl_config,
nullptr,
CONF_MFLAGS_DEFAULT_SECTION | CONF_MFLAGS_IGNORE_MISSING_FILE);
CONF_MFLAGS_DEFAULT_SECTION);
int err = ERR_get_error();
if (0 != err) {
fprintf(stderr,
Expand All @@ -5786,7 +5792,6 @@ void InitCryptoOnce() {

SSL_library_init();
OpenSSL_add_all_algorithms();
SSL_load_error_strings();

crypto_lock_init();
CRYPTO_set_locking_callback(crypto_lock_cb);
Expand Down
4 changes: 4 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ struct sockaddr;

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;

// Set in node.cc by ParseArgs when --preserve-symlinks is used.
// Used in node_config.cc to set a constant on process.binding('config')
// that is used by lib/module.js
Expand Down
35 changes: 28 additions & 7 deletions test/parallel/test-crypto-fips.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,26 @@ testHelper(
// OpenSSL config file should be able to turn on FIPS mode
testHelper(
'stdout',
[],
[`--openssl-config=${CNF_FIPS_ON}`],
compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
'require("crypto").fips',
process.env);
// OPENSSL_CONF should _not_ be able to turn on FIPS mode
testHelper(
'stdout',
[],
FIPS_DISABLED,
'require("crypto").fips',
addToEnv('OPENSSL_CONF', CNF_FIPS_ON));

// --enable-fips should take precedence over OpenSSL config file
testHelper(
compiledWithFips() ? 'stdout' : 'stderr',
['--enable-fips', `--openssl-config=${CNF_FIPS_OFF}`],
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',
['--enable-fips'],
Expand All @@ -102,6 +116,13 @@ testHelper(
addToEnv('OPENSSL_CONF', CNF_FIPS_OFF));

// --force-fips should take precedence over OpenSSL config file
testHelper(
compiledWithFips() ? 'stdout' : 'stderr',
['--force-fips', `--openssl-config=${CNF_FIPS_OFF}`],
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',
['--force-fips'],
Expand All @@ -116,7 +137,7 @@ testHelper(
compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING,
'(require("crypto").fips = true,' +
'require("crypto").fips)',
addToEnv('OPENSSL_CONF', ''));
process.env);

// setFipsCrypto should be able to turn FIPS mode on and off
testHelper(
Expand All @@ -126,25 +147,25 @@ testHelper(
'(require("crypto").fips = true,' +
'require("crypto").fips = false,' +
'require("crypto").fips)',
addToEnv('OPENSSL_CONF', ''));
process.env);

// setFipsCrypto takes precedence over OpenSSL config file, FIPS on
testHelper(
compiledWithFips() ? 'stdout' : 'stderr',
[],
[`--openssl-config=${CNF_FIPS_OFF}`],
compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING,
'(require("crypto").fips = true,' +
'require("crypto").fips)',
addToEnv('OPENSSL_CONF', CNF_FIPS_OFF));
process.env);

// setFipsCrypto takes precedence over OpenSSL config file, FIPS off
testHelper(
compiledWithFips() ? 'stdout' : 'stderr',
[],
[`--openssl-config=${CNF_FIPS_ON}`],
compiledWithFips() ? FIPS_DISABLED : FIPS_ERROR_STRING,
'(require("crypto").fips = false,' +
'require("crypto").fips)',
addToEnv('OPENSSL_CONF', CNF_FIPS_ON));
process.env);

// --enable-fips does not prevent use of setFipsCrypto API
testHelper(
Expand Down

0 comments on commit 3518372

Please sign in to comment.