From 0d943f4339fabd0afc167d5edf1711340a01a195 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 2 Jul 2019 09:46:43 +0200 Subject: [PATCH 1/4] test: use openssl_is_fips instead of hasFipsCrypto Currently, when dynamically linking against a FIPS enabled OpenSSL library test-process-env-allowed-flags-are-documented will fail with the following error: assert.js:89 throw new AssertionError(obj); ^ AssertionError [ERR_ASSERTION]: The following options are not documented as allowed in NODE_OPTIONS in /root/node/doc/api/cli.md: --enable-fips --force-fips at Object. (/test/parallel/test-process-env-allowed-flags-are-documented.js:82:8) at Module._compile (internal/modules/cjs/loader.js:779:30) at Object.Module._extensions..js (internal/modules/cjs/loader.js:790:10) at Module.load (internal/modules/cjs/loader.js:642:32) at Function.Module._load (internal/modules/cjs/loader.js:555:12) at Function.Module.runMain (internal/modules/cjs/loader.js:842:10) at internal/main/run_main_module.js:17:11 { generatedMessage: false, code: 'ERR_ASSERTION', actual: 2, expected: 0, operator: 'strictEqual' } This commit updates the test to use process.config.variables.openssl_is_fips instead of common.hasFipsCrypto as hasFipsCrypto only returns true if the OpenSSL library that is shipped with node was configured with FIPS enabled. --- test/parallel/test-process-env-allowed-flags-are-documented.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js index bb203d94d370d8..7d8a1b5f51dbe9 100644 --- a/test/parallel/test-process-env-allowed-flags-are-documented.js +++ b/test/parallel/test-process-env-allowed-flags-are-documented.js @@ -46,7 +46,7 @@ const conditionalOpts = [ return ['--openssl-config', '--tls-cipher-list', '--use-bundled-ca', '--use-openssl-ca' ].includes(opt); } }, - { include: common.hasFipsCrypto, + { include: process.config.variables.openssl_is_fips, filter: (opt) => opt.includes('-fips') }, { include: common.hasIntl, filter: (opt) => opt === '--icu-data-dir' }, From f1e2dc692bcd2b866df67ce7caf670cc920bad11 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 3 Jul 2019 13:03:09 +0200 Subject: [PATCH 2/4] squash!: add comment about using config property --- .../test-process-env-allowed-flags-are-documented.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js index 7d8a1b5f51dbe9..79c9abe65f96d8 100644 --- a/test/parallel/test-process-env-allowed-flags-are-documented.js +++ b/test/parallel/test-process-env-allowed-flags-are-documented.js @@ -46,7 +46,14 @@ const conditionalOpts = [ return ['--openssl-config', '--tls-cipher-list', '--use-bundled-ca', '--use-openssl-ca' ].includes(opt); } }, - { include: process.config.variables.openssl_is_fips, + { + // We are using openssl_is_fips from the configuration because it could be + // the case that OpenSSL is FIPS compatible but fips has not been enabled + // (starting node with --enable-fips). If we use common.hasFipsCrypto + // that would only tells us if fips has been enabled, but in this case we + // want to check options which will be available regardless of whether fips + // is enabled at runtime or not. + include: process.config.variables.openssl_is_fips, filter: (opt) => opt.includes('-fips') }, { include: common.hasIntl, filter: (opt) => opt === '--icu-data-dir' }, From 1d6935e25b6121b61a0b8202ac1203952b9d7fcd Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Fri, 5 Jul 2019 13:42:14 +0200 Subject: [PATCH 3/4] test: update hasFipsCrypto in test/common/README --- test/common/README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/common/README.md b/test/common/README.md index 2b8f852a3b74dd..131967f73be16a 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -194,7 +194,12 @@ Indicates whether OpenSSL is available. ### hasFipsCrypto * [<boolean>] -Indicates `hasCrypto` and `crypto` with fips. +Indicates that Node.js has been linked with a FIPS compatible OpenSSL library, +and that FIPS as been enabled using `--enable-fips`. + +To only detect if the OpenSSL library is FIPS compatible, regardless if it has +been enabled or not, then `process.config.variables.openssl_is_fips` can be +used to determine that situation. ### hasIntl * [<boolean>] From 7a1fac41ff4887dc3e5ab4b6bcc24fbe5633a78a Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Fri, 5 Jul 2019 13:42:14 +0200 Subject: [PATCH 4/4] squash!: test: update hasFipsCrypto in test/common/README --- test/common/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/README.md b/test/common/README.md index 131967f73be16a..1db6d15a0d9cee 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -194,7 +194,7 @@ Indicates whether OpenSSL is available. ### hasFipsCrypto * [<boolean>] -Indicates that Node.js has been linked with a FIPS compatible OpenSSL library, +Indicates that Node.js has been linked with a FIPS compatible OpenSSL library, and that FIPS as been enabled using `--enable-fips`. To only detect if the OpenSSL library is FIPS compatible, regardless if it has