Skip to content

Commit

Permalink
crypto: make FIPS related options always awailable
Browse files Browse the repository at this point in the history
There is no reason to hide FIPS functionality behind build flags.
OpenSSL always provide the information about FIPS availability via
`FIPS_mode()` function.

This makes the user experience more consistent, because the OpenSSL
library is always queried and the `crypto.getFips()` always returns
OpenSSL settings.

Fixes #34903

PR-URL: #36341
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
  • Loading branch information
voxik authored and mhdawson committed Feb 25, 2021
1 parent 148bc33 commit f392ac0
Show file tree
Hide file tree
Showing 15 changed files with 76 additions and 107 deletions.
8 changes: 4 additions & 4 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ code from strings throw an exception instead. This does not affect the Node.js
added: v6.0.0
-->

Enable FIPS-compliant crypto at startup. (Requires Node.js to be built with
`./configure --openssl-fips`.)
Enable FIPS-compliant crypto at startup. (Requires Node.js to be built
against FIPS-compatible OpenSSL.)

### `--enable-source-maps`
<!-- YAML
Expand Down Expand Up @@ -613,8 +613,8 @@ 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`.
used to enable FIPS-compliant crypto if Node.js is built
against FIPS-enabled OpenSSL.

### `--pending-deprecation`
<!-- YAML
Expand Down
22 changes: 4 additions & 18 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,10 @@ assertCrypto();

const {
ERR_CRYPTO_FIPS_FORCED,
ERR_CRYPTO_FIPS_UNAVAILABLE
} = require('internal/errors').codes;
const constants = internalBinding('constants').crypto;
const { getOptionValue } = require('internal/options');
const pendingDeprecation = getOptionValue('--pending-deprecation');
const { fipsMode } = internalBinding('config');
const fipsForced = getOptionValue('--force-fips');
const {
getFipsCrypto,
Expand Down Expand Up @@ -218,10 +216,8 @@ module.exports = {
sign: signOneShot,
setEngine,
timingSafeEqual,
getFips: !fipsMode ? getFipsDisabled :
fipsForced ? getFipsForced : getFipsCrypto,
setFips: !fipsMode ? setFipsDisabled :
fipsForced ? setFipsForced : setFipsCrypto,
getFips: fipsForced ? getFipsForced : getFipsCrypto,
setFips: fipsForced ? setFipsForced : setFipsCrypto,
verify: verifyOneShot,

// Classes
Expand All @@ -242,19 +238,11 @@ module.exports = {
secureHeapUsed,
};

function setFipsDisabled() {
throw new ERR_CRYPTO_FIPS_UNAVAILABLE();
}

function setFipsForced(val) {
if (val) return;
throw new ERR_CRYPTO_FIPS_FORCED();
}

function getFipsDisabled() {
return 0;
}

function getFipsForced() {
return 1;
}
Expand All @@ -276,10 +264,8 @@ ObjectDefineProperties(module.exports, {
},
// crypto.fips is deprecated. DEP0093. Use crypto.getFips()/crypto.setFips()
fips: {
get: !fipsMode ? getFipsDisabled :
fipsForced ? getFipsForced : getFipsCrypto,
set: !fipsMode ? setFipsDisabled :
fipsForced ? setFipsForced : setFipsCrypto
get: fipsForced ? getFipsForced : getFipsCrypto,
set: fipsForced ? setFipsForced : setFipsCrypto
},
DEFAULT_ENCODING: {
enumerable: false,
Expand Down
3 changes: 0 additions & 3 deletions node.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,6 @@
[ 'node_use_openssl=="true"', {
'defines': [ 'HAVE_OPENSSL=1' ],
'conditions': [
['openssl_fips != "" or openssl_is_fips=="true"', {
'defines': [ 'NODE_FIPS_MODE' ],
}],
[ 'node_shared_openssl=="false"', {
'dependencies': [
'./deps/openssl/openssl.gyp:openssl',
Expand Down
4 changes: 0 additions & 4 deletions src/crypto/crypto_cipher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,10 @@ void CipherBase::Init(const char* cipher_type,
HandleScope scope(env()->isolate());
MarkPopErrorOnReturn mark_pop_error_on_return;

#ifdef NODE_FIPS_MODE
if (FIPS_mode()) {
return THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION(env(),
"crypto.createCipher() is not supported in FIPS mode.");
}
#endif // NODE_FIPS_MODE

const EVP_CIPHER* const cipher = EVP_get_cipherbyname(cipher_type);
if (cipher == nullptr)
Expand Down Expand Up @@ -528,14 +526,12 @@ bool CipherBase::InitAuthenticated(
return false;
}

#ifdef NODE_FIPS_MODE
// TODO(tniessen) Support CCM decryption in FIPS mode
if (mode == EVP_CIPH_CCM_MODE && kind_ == kDecipher && FIPS_mode()) {
THROW_ERR_CRYPTO_UNSUPPORTED_OPERATION(env(),
"CCM encryption not supported in FIPS mode");
return false;
}
#endif

// Tell OpenSSL about the desired length.
if (!EVP_CIPHER_CTX_ctrl(ctx_.get(), EVP_CTRL_AEAD_SET_TAG, auth_tag_len,
Expand Down
2 changes: 0 additions & 2 deletions src/crypto/crypto_sig.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ using v8::Value;
namespace crypto {
namespace {
bool ValidateDSAParameters(EVP_PKEY* key) {
#ifdef NODE_FIPS_MODE
/* Validate DSA2 parameters from FIPS 186-4 */
if (FIPS_mode() && EVP_PKEY_DSA == EVP_PKEY_base_id(key)) {
DSA* dsa = EVP_PKEY_get0_DSA(key);
Expand All @@ -43,7 +42,6 @@ bool ValidateDSAParameters(EVP_PKEY* key) {
(L == 2048 && N == 256) ||
(L == 3072 && N == 256);
}
#endif // NODE_FIPS_MODE

return true;
}
Expand Down
24 changes: 14 additions & 10 deletions src/crypto/crypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ void InitCryptoOnce() {
}
#endif

#ifdef NODE_FIPS_MODE
/* Override FIPS settings in cnf file, if needed. */
unsigned long err = 0; // NOLINT(runtime/int)
if (per_process::cli_options->enable_fips_crypto ||
Expand All @@ -143,12 +142,10 @@ void InitCryptoOnce() {
}
}
if (0 != err) {
fprintf(stderr,
"openssl fips failed: %s\n",
ERR_error_string(err, nullptr));
UNREACHABLE();
auto* isolate = Isolate::GetCurrent();
auto* env = Environment::GetCurrent(isolate);
return ThrowCryptoError(env, err);
}
#endif // NODE_FIPS_MODE

// Turn off compression. Saves memory and protects against CRIME attacks.
// No-op with OPENSSL_NO_COMP builds of OpenSSL.
Expand All @@ -162,7 +159,6 @@ void InitCryptoOnce() {
NodeBIO::GetMethod();
}

#ifdef NODE_FIPS_MODE
void GetFipsCrypto(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(FIPS_mode() ? 1 : 0);
}
Expand All @@ -180,7 +176,16 @@ void SetFipsCrypto(const FunctionCallbackInfo<Value>& args) {
return ThrowCryptoError(env, err);
}
}
#endif /* NODE_FIPS_MODE */

void TestFipsCrypto(const v8::FunctionCallbackInfo<v8::Value>& args) {
#ifdef OPENSSL_FIPS
const auto enabled = FIPS_selftest() ? 1 : 0;
#else // OPENSSL_FIPS
const auto enabled = 0;
#endif // OPENSSL_FIPS

args.GetReturnValue().Set(enabled);
}

void CryptoErrorVector::Capture() {
clear();
Expand Down Expand Up @@ -652,10 +657,9 @@ void Initialize(Environment* env, Local<Object> target) {
env->SetMethod(target, "setEngine", SetEngine);
#endif // !OPENSSL_NO_ENGINE

#ifdef NODE_FIPS_MODE
env->SetMethodNoSideEffect(target, "getFipsCrypto", GetFipsCrypto);
env->SetMethod(target, "setFipsCrypto", SetFipsCrypto);
#endif
env->SetMethodNoSideEffect(target, "testFipsCrypto", TestFipsCrypto);

NODE_DEFINE_CONSTANT(target, kCryptoJobAsync);
NODE_DEFINE_CONSTANT(target, kCryptoJobSync);
Expand Down
9 changes: 7 additions & 2 deletions src/crypto/crypto_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
#ifndef OPENSSL_NO_ENGINE
# include <openssl/engine.h>
#endif // !OPENSSL_NO_ENGINE
// The FIPS-related functions are only available
// when the OpenSSL itself was compiled with FIPS support.
#ifdef OPENSSL_FIPS
# include <openssl/fips.h>
#endif // OPENSSL_FIPS

#include <algorithm>
#include <memory>
Expand Down Expand Up @@ -510,11 +515,11 @@ bool SetEngine(
void SetEngine(const v8::FunctionCallbackInfo<v8::Value>& args);
#endif // !OPENSSL_NO_ENGINE

#ifdef NODE_FIPS_MODE
void GetFipsCrypto(const v8::FunctionCallbackInfo<v8::Value>& args);

void SetFipsCrypto(const v8::FunctionCallbackInfo<v8::Value>& args);
#endif /* NODE_FIPS_MODE */

void TestFipsCrypto(const v8::FunctionCallbackInfo<v8::Value>& args);

class CipherPushContext {
public:
Expand Down
6 changes: 3 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1013,11 +1013,11 @@ InitializationResult InitializeOncePerProcess(int argc, char** argv) {
if (credentials::SafeGetenv("NODE_EXTRA_CA_CERTS", &extra_ca_certs))
crypto::UseExtraCaCerts(extra_ca_certs);
}
#ifdef NODE_FIPS_MODE
// In the case of FIPS builds we should make sure
// the random source is properly initialized first.
OPENSSL_init();
#endif // NODE_FIPS_MODE
if (FIPS_mode()) {
OPENSSL_init();
}
// V8 on Windows doesn't have a good source of entropy. Seed it from
// OpenSSL's pool.
V8::SetEntropySource(crypto::EntropySource);
Expand Down
2 changes: 0 additions & 2 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,7 @@ static void Initialize(Local<Object> target,
READONLY_FALSE_PROPERTY(target, "hasOpenSSL");
#endif // HAVE_OPENSSL

#ifdef NODE_FIPS_MODE
READONLY_TRUE_PROPERTY(target, "fipsMode");
#endif

#ifdef NODE_HAVE_I18N_SUPPORT

Expand Down
10 changes: 8 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ namespace node {
using v8::Context;
using v8::Local;
using v8::Object;
using v8::TryCatch;
using v8::Value;

namespace crypto {
Expand All @@ -39,10 +40,15 @@ void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);

static uv_once_t init_once = UV_ONCE_INIT;
TryCatch try_catch{env->isolate()};
uv_once(&init_once, InitCryptoOnce);

Environment* env = Environment::GetCurrent(context);
if (try_catch.HasCaught() && !try_catch.HasTerminated()) {
try_catch.ReThrow();
return;
}

AES::Initialize(env, target);
CipherBase::Initialize(env, target);
Expand Down
2 changes: 0 additions & 2 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,6 @@ PerProcessOptionsParser::PerProcessOptionsParser(
&PerProcessOptions::ssl_openssl_cert_store);
Implies("--use-openssl-ca", "[ssl_openssl_cert_store]");
ImpliesNot("--use-bundled-ca", "[ssl_openssl_cert_store]");
#if NODE_FIPS_MODE
AddOption("--enable-fips",
"enable FIPS crypto at startup",
&PerProcessOptions::enable_fips_crypto,
Expand All @@ -775,7 +774,6 @@ PerProcessOptionsParser::PerProcessOptionsParser(
"force FIPS crypto (cannot be disabled)",
&PerProcessOptions::force_fips_crypto,
kAllowedInEnvironment);
#endif
AddOption("--secure-heap",
"total size of the OpenSSL secure heap",
&PerProcessOptions::secure_heap,
Expand Down
2 changes: 0 additions & 2 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,8 @@ class PerProcessOptions : public Options {
#endif
bool use_openssl_ca = false;
bool use_bundled_ca = false;
#if NODE_FIPS_MODE
bool enable_fips_crypto = false;
bool force_fips_crypto = false;
#endif
#endif

// Per-process because reports can be triggered outside a known V8 context.
Expand Down
7 changes: 2 additions & 5 deletions test/parallel/test-cli-node-print-help.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ const common = require('../common');

const assert = require('assert');
const { exec } = require('child_process');
const { internalBinding } = require('internal/test/binding');
const { fipsMode } = internalBinding('config');
let stdOut;


Expand All @@ -28,9 +26,8 @@ function validateNodePrintHelp() {
const cliHelpOptions = [
{ compileConstant: HAVE_OPENSSL,
flags: [ '--openssl-config=...', '--tls-cipher-list=...',
'--use-bundled-ca', '--use-openssl-ca' ] },
{ compileConstant: fipsMode,
flags: [ '--enable-fips', '--force-fips' ] },
'--use-bundled-ca', '--use-openssl-ca',
'--enable-fips', '--force-fips' ] },
{ compileConstant: NODE_HAVE_I18N_SUPPORT,
flags: [ '--icu-data-dir=...', 'NODE_ICU_DATA' ] },
{ compileConstant: HAVE_INSPECTOR,
Expand Down
Loading

0 comments on commit f392ac0

Please sign in to comment.