Always available FIPS options

[khardix]

This is continuation of #35019, rebased on current master. I have taken it over from @voxik.

Additional changes

fipsMode constant was replaced by (hopefully) internal binding to FIPS_selftest() OpenSSL function.
The binding is called testFipsCrypto() and it simply returns 1 or 0 based on the FIPS status reported by OpenSSL.
The relevant tests were adjusted to rely on this in place of the original constant.

Open problems

There is still the issue of reporting errors in InitCryptoOnce():

/* Override FIPS settings in cnf file, if needed. */
unsigned long err = 0;  // NOLINT(runtime/int)
if (per_process::cli_options->enable_fips_crypto ||
    per_process::cli_options->force_fips_crypto) {
  if (0 == FIPS_mode() && !FIPS_mode_set(1)) {
    err = ERR_get_error();
  }
}
if (0 != err) {
  fprintf(stderr,
          "openssl fips failed: %s\n",
          ERR_error_string(err, nullptr));
  UNREACHABLE();
}

The UNREACHABLE() section is not so unreachable anymore 😉.
Unfortunately, I was not able to figure out better way to report an error – anything similar to return ThrowCryptoError(env, err) requires a reference to the environment, which AFAIK is not available in the InitCryptoOnce().

Any pointers?

---

Fixes #34903; obsoletes/closes #35019.

[danbev]

Any pointers?

Would something like this work perhaps?

diff --git a/src/crypto/crypto_util.cc b/src/crypto/crypto_util.cc
index a238b84e7e..759e829000 100644
--- a/src/crypto/crypto_util.cc
+++ b/src/crypto/crypto_util.cc
@@ -120,10 +120,9 @@ void InitCryptoOnce() {
     }
   }
   if (0 != err) {
-    fprintf(stderr,
-            "openssl fips failed: %s\n",
-            ERR_error_string(err, nullptr));
-    UNREACHABLE();
+    Isolate* isolate = Isolate::GetCurrent();
+    Environment* env = Environment::GetCurrent(isolate);
+    return ThrowCryptoError(env, err, ERR_error_string(err, nullptr));
   }
 
   // Turn off compression. Saves memory and protects against CRIME attacks.
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
index d25387f142..2ce5f211fb 100644
--- a/src/node_crypto.cc
+++ b/src/node_crypto.cc
@@ -31,6 +31,7 @@ namespace node {
 using v8::Context;
 using v8::Local;
 using v8::Object;
+using v8::TryCatch;
 using v8::Value;
 
 namespace crypto {
@@ -39,10 +40,14 @@ void Initialize(Local<Object> target,
                 Local<Value> unused,
                 Local<Context> context,
                 void* priv) {
+  Environment* env = Environment::GetCurrent(context);
+  TryCatch try_catch{env->isolate()};
   static uv_once_t init_once = UV_ONCE_INIT;
   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);

With this patch and using --enable-fips when there is no fips support available would result in:

$ ./out/Debug/node --enable-fips -p 'crypto.constants.OPENSSL_VERSION_NUMBER'
node:internal/bootstrap/loaders:140
      mod = bindingObj[module] = getInternalBinding(module);
                                 ^

Error: error:0F06D065:common libcrypto routines:FIPS_mode_set:fips mode not supported
    at internalBinding (node:internal/bootstrap/loaders:140:34)
    at node:crypto:50:5
    at NativeModule.compileForInternalLoader (node:internal/bootstrap/loaders:283:7)
    at nativeModuleRequire (node:internal/bootstrap/loaders:312:14)
    at get (node:internal/modules/cjs/helpers:158:21)
    at [eval]:1:1
    at Script.runInThisContext (node:vm:133:18)
    at Object.runInThisContext (node:vm:310:38)
    at node:internal/process/execution:77:19
    at [eval]-wrapper:6:22 {
  library: 'common libcrypto routines',
  function: 'FIPS_mode_set',
  reason: 'fips mode not supported',
  code: 'ERR_OSSL_CRYPTO_FIPS_MODE_NOT_SUPPORTED'
}

[khardix]

@danbev Thanks, the Isolate::GetCurrent(); Environment::GetCurrent(isolate); calls were exactly what I was looking for 🙂

Now I can get a proper look at the failing tests.

[danbev]

This could optionally be shortened to just be:

return ThrowCryptoError(env, err); 

[khardix]

I assume you mean the return line can be shortened… otherwise we are back at the start with not having an environment pointer – or do I miss something?

[danbev]

Yeah, sorry that is what I meant, just the last line.

[khardix]

Cleaned the history a bit and rebased on master. If the checks pass, this should be ready for review.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/35059/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/35107/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/35353/

[mhdawson]

@khardix there still seem to be a few related tests that are failing: test.parallel/test-cli-node-print-help
test.parallel/test-process-env-allowed-flags-are-documented

in https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1804_sharedlibs_debug_x64/24501/

[khardix]

Just noting this is still on my backlog, hopefully I can get back to this within a week again.

[khardix]

I'm currently unable to reproduce the test failures locally. I have rebased the changes on current master; let's see what the CI thinks.

[khardix]

@mhdawson Well, now it seems to pass; only macOS ran out of memory. Unless any changes are requested, I'm considering this ready to be merged.

[mhdawson]

Landed in f392ac0