GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||
GeezerVison is showing github.com — Always available FIPS options by khardix · Pull Request #36341 · nodejs/node Conversation
Contributor
This is continuation of #35019, rebased on current master. I have taken it over from @voxik. Additional changes
Open problemsThere is still the issue of reporting errors in /* 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 Any pointers?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
khardix
marked this pull request as draft
addaleax
approved these changes
|
Uh oh!
There was an error while loading. Please reload this page.
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 $ ./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' } |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
|
@danbev Thanks, the Now I can get a proper look at the failing tests. |
All reactions
-
👍 1 reaction
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
danbev
reviewed
Dec 15, 2020
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could optionally be shortened to just be:
return ThrowCryptoError(env, err);
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Contributor
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sorry that is what I meant, just the last line.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
khardix
force-pushed
the
always-available-fips-options
branch
2 times, most recently
from
December 15, 2020 16:31
4f5a7de to
6725b14
Compare
Contributor
Author
|
Cleaned the history a bit and rebased on master. If the checks pass, this should be ready for review. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
khardix
marked this pull request as ready for review
December 15, 2020 16:32
voxik
suggested changes
Dec 16, 2020
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Collaborator
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Collaborator
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Collaborator
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Member
|
@khardix there still seem to be a few related tests that are failing: test.parallel/test-cli-node-print-help |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
|
Just noting this is still on my backlog, hopefully I can get back to this within a week again. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 nodejs#34903
Signed-off-by: Jan Staněk <[email protected]>
- The fipsMode constant (defined at compile time) was replaced by the new `TestFipsCrypto()`/`testFipsCrypto()` functions, which rely on the OpenSSL function `FIPS_selftest()`. This results in the FIPS mode being always checked on runtime and being informed purely by the OpenSSL implementation in use.
khardix
force-pushed
the
always-available-fips-options
branch
from
February 9, 2021 16:04
b183f96 to
e0380d9
Compare
Contributor
Author
|
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. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
Author
|
@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. |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
mhdawson
reviewed
Feb 10, 2021
Uh oh!
There was an error while loading. Please reload this page.
mhdawson
commented
Feb 25, 2021
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Member
|
Landed in f392ac0 |
All reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
targos
pushed a commit
that referenced
this pull request
Feb 28, 2021
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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
codebytere
added a commit
to electron/electron
that referenced
this pull request
May 20, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
May 20, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
May 31, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
May 31, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 8, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 9, 2021
codebytere
added a commit
to electron/electron
that referenced
this pull request
Jun 10, 2021
danbev
pushed a commit
to danbev/node
that referenced
this pull request
Sep 28, 2021
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 nodejs#34903 Backport-PR-URL: nodejs#40241 PR-URL: nodejs#36341 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
richardlau
pushed a commit
that referenced
this pull request
Nov 25, 2021
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 Backport-PR-URL: #40241 PR-URL: #36341 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
richardlau
pushed a commit
that referenced
this pull request
Nov 30, 2021
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 Backport-PR-URL: #40241 PR-URL: #36341 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
richardlau
added a commit
that referenced
this pull request
Jan 25, 2022
Notable changes: Corepack: Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, Corepack will let you use Yarn and pnpm without having to install them - just like what currently happens with npm, which is shipped in Node.js by default. Contributed by Maël Nison - #39608 ICU updated: ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - #40658 New option to disable loading of native addons: A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - #39977 Updated Root Certificates: Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - #40280 Other Notable Changes: crypto: * (SEMVER-MINOR) make FIPS related options always available (Vít Ondruch) #36341 lib: * (SEMVER-MINOR) add unsubscribe method to non-active DC channels (simon-id) #40433 * (SEMVER-MINOR) add return value for DC channel.unsubscribe (simon-id) #40433 module: * (SEMVER-MINOR) support pattern trailers (Guy Bedford) #39635 src: * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 PR-URL: #41696
Merged
richardlau
added a commit
that referenced
this pull request
Feb 1, 2022
Notable changes: Corepack: Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, Corepack will let you use Yarn and pnpm without having to install them - just like what currently happens with npm, which is shipped in Node.js by default. Contributed by Maël Nison - #39608 ICU updated: ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - #40658 New option to disable loading of native addons: A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - #39977 Updated Root Certificates: Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - #40280 Other Notable Changes: crypto: * (SEMVER-MINOR) make FIPS related options always available (Vít Ondruch) #36341 lib: * (SEMVER-MINOR) add unsubscribe method to non-active DC channels (simon-id) #40433 * (SEMVER-MINOR) add return value for DC channel.unsubscribe (simon-id) #40433 module: * (SEMVER-MINOR) support pattern trailers (Guy Bedford) #39635 src: * (SEMVER-MINOR) make napi_create_reference accept symbol (JckXia) #39926 PR-URL: #41696
mwalbeck
pushed a commit
to mwalbeck/docker-jellyfin-livestream
that referenced
this pull request
Mar 16, 2022
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [node](https://github.com/nodejs/node) | stage | minor | `14.18.3-bullseye-slim` -> `14.19.0-bullseye-slim` | --- ### Release Notes <details> <summary>nodejs/node</summary> ### [`v14.19.0`](https://github.com/nodejs/node/releases/v14.19.0) [Compare Source](nodejs/node@v14.18.3...v14.19.0) ##### Notable Changes ##### Corepack Node.js now includes Corepack, a script that acts as a bridge between Node.js projects and the package managers they are intended to be used with during development. In practical terms, **Corepack will let you use Yarn and pnpm without having to install them** - just like what currently happens with npm, which is shipped in Node.js by default. Please head over to the [Corepack documentation page](https://nodejs.org/dist/latest-v14.x/docs/api/corepack.html) for more information on how to use it. Contributed by Maël Nison - [#​39608](nodejs/node#39608) ##### ICU updated ICU has been updated to 70.1. This updates timezone database to 2021a3, including bringing forward the start for DST for Jordan from March to February. Contributed by Michaël Zasso - [#​40658](nodejs/node#40658) ##### New option to disable loading of native addons A new command line option `--no-addons` has been added to disallow loading of native addons. Contributed by Dominic Elm - [#​39977](nodejs/node#39977) ##### Updated Root Certificates Root certificates have been updated to those from Mozilla's Network Security Services 3.71. Contributed by Richard Lau - [#​40280](nodejs/node#40280) ##### Other Notable Changes - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#​36341](nodejs/node#36341) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#​40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#​40433](nodejs/node#40433) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#​39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#​39926](nodejs/node#39926) ##### Commits - \[[`0231ffa501`](nodejs/node@0231ffa501)] - **build**: add `--without-corepack` (Jonah Snider) [#​41060](nodejs/node#41060) - \[[`5389b8ab05`](nodejs/node@5389b8ab05)] - **crypto**: update root certificates (Richard Lau) [#​40280](nodejs/node#40280) - \[[`0d448eaab5`](nodejs/node@0d448eaab5)] - **(SEMVER-MINOR)** **crypto**: make FIPS related options always available (Vít Ondruch) [#​36341](nodejs/node#36341) - \[[`cd20ecc7cb`](nodejs/node@cd20ecc7cb)] - **deps**: upgrade Corepack to 0.10 (Maël Nison) [#​40374](nodejs/node#40374) - \[[`737df75e17`](nodejs/node@737df75e17)] - **(SEMVER-MINOR)** **deps**: add corepack (Maël Nison) [#​39608](nodejs/node#39608) - \[[`b85aa5a143`](nodejs/node@b85aa5a143)] - **deps**: upgrade npm to 6.14.16 (Ruy Adorno) [#​41603](nodejs/node#41603) - \[[`2755d391a5`](nodejs/node@2755d391a5)] - **deps**: update ICU to 70.1 (Michaël Zasso) [#​40658](nodejs/node#40658) - \[[`3089326d89`](nodejs/node@3089326d89)] - **deps**: update archs files for OpenSSL-1.1.1m (Richard Lau) [#​41173](nodejs/node#41173) - \[[`59da7c12aa`](nodejs/node@59da7c12aa)] - **deps**: upgrade openssl sources to 1.1.1m (Richard Lau) [#​41173](nodejs/node#41173) - \[[`cede1f26f6`](nodejs/node@cede1f26f6)] - **deps**: add -fno-strict-aliasing flag to libuv (Daniel Bevenius) [#​40631](nodejs/node#40631) - \[[`4477da858f`](nodejs/node@4477da858f)] - **doc**: fix corepack grammar for `--force` flag (Steven) [#​40762](nodejs/node#40762) - \[[`5971d58600`](nodejs/node@5971d58600)] - **doc**: add missing YAML tag in `esm.md` (Antoine du Hamel) [#​41516](nodejs/node#41516) - \[[`e903798ae1`](nodejs/node@e903798ae1)] - **doc**: add note regarding unfinished TLA (Antoine du Hamel) [#​41434](nodejs/node#41434) - \[[`a90defebcf`](nodejs/node@a90defebcf)] - **esm**: make `process.exit()` default to exit code 0 (Gang Chen) [#​41388](nodejs/node#41388) - \[[`fc328f1ab0`](nodejs/node@fc328f1ab0)] - **fs**: nullish coalescing to respect zero positional reads (Omar El-Mihilmy) [#​40716](nodejs/node#40716) - \[[`004eafbebf`](nodejs/node@004eafbebf)] - **(SEMVER-MINOR)** **lib**: add unsubscribe method to non-active DC channels (simon-id) [#​40433](nodejs/node#40433) - \[[`625be7585d`](nodejs/node@625be7585d)] - **(SEMVER-MINOR)** **lib**: add return value for DC channel.unsubscribe (simon-id) [#​40433](nodejs/node#40433) - \[[`2c365961d0`](nodejs/node@2c365961d0)] - **module**: support pattern trailers for imports field (Guy Bedford) [#​40041](nodejs/node#40041) - \[[`607bc74eae`](nodejs/node@607bc74eae)] - **(SEMVER-MINOR)** **module**: support pattern trailers (Guy Bedford) [#​39635](nodejs/node#39635) - \[[`f74fe2a59c`](nodejs/node@f74fe2a59c)] - **(SEMVER-MINOR)** **src**: make napi_create_reference accept symbol (JckXia) [#​39926](nodejs/node#39926) - \[[`b050c65885`](nodejs/node@b050c65885)] - **src**: add option to disable loading native addons (Dominic Elm) [#​39977](nodejs/node#39977) - \[[`c1695ac68a`](nodejs/node@c1695ac68a)] - **tools**: update certdata.txt (Richard Lau) [#​40280](nodejs/node#40280) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Reviewed-on: https://git.walbeck.it/mwalbeck/docker-jellyfin-livestream/pulls/97 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
+3 more reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
c++
Issues and PRs that require attention from people who are familiar with C++.
lib / src
Issues and PRs involving general changes in the lib/ or src/ directories.
semver-minor
PRs that contain new features and should be released in the next minor version.
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Drop#ifdef NODE_FIPS_MODE wherever possible
Uh oh!
There was an error while loading. Please reload this page.
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.