Skip to content

Always available FIPS options#36341

Closed
khardix wants to merge 7 commits intonodejs:masterfrom
khardix:always-available-fips-options
Closed

Always available FIPS options#36341
khardix wants to merge 7 commits intonodejs:masterfrom
khardix:always-available-fips-options

Conversation

@khardix
Copy link
Copy Markdown
Contributor

@khardix khardix commented Dec 1, 2020

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.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop #ifdef NODE_FIPS_MODE wherever possible

9 participants