Skip to content

Commit eb4ff0b

Browse files
khardixkhardix
committed
Adjust tests for always-available FIPS options
- 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.
1 parent 7aefc26 commit eb4ff0b

File tree

4 files changed

+40
-70
lines changed

4 files changed

+40
-70
lines changed

‎doc/api/cli.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ code from strings throw an exception instead. This does not affect the Node.js
186186
added: v6.0.0
187187
-->
188188

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

192192
### `--enable-source-maps`
193193
<!-- YAML
@@ -606,8 +606,8 @@ added: v6.9.0
606606
-->
607607

608608
Load an OpenSSL configuration file on startup. Among other uses, this can be
609-
used to enable FIPS-compliant crypto if Node.js is built with
610-
`./configure --openssl-fips`.
609+
used to enable FIPS-compliant crypto if Node.js is built
610+
with against FIPS-enabled OpenSSL.
611611

612612
### `--pending-deprecation`
613613
<!-- YAML

‎lib/crypto.js

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,10 @@ assertCrypto();
3737

3838
const {
3939
ERR_CRYPTO_FIPS_FORCED,
40-
ERR_CRYPTO_FIPS_UNAVAILABLE
4140
} = require('internal/errors').codes;
4241
const constants = internalBinding('constants').crypto;
4342
const { getOptionValue } = require('internal/options');
4443
const pendingDeprecation = getOptionValue('--pending-deprecation');
45-
const { fipsMode } = internalBinding('config');
4644
const fipsForced = getOptionValue('--force-fips');
4745
const {
4846
getFipsCrypto,
@@ -218,10 +216,8 @@ module.exports = {
218216
sign: signOneShot,
219217
setEngine,
220218
timingSafeEqual,
221-
getFips: !fipsMode ? getFipsDisabled :
222-
fipsForced ? getFipsForced : getFipsCrypto,
223-
setFips: !fipsMode ? setFipsDisabled :
224-
fipsForced ? setFipsForced : setFipsCrypto,
219+
getFips: fipsForced ? getFipsForced : getFipsCrypto,
220+
setFips: fipsForced ? setFipsForced : setFipsCrypto,
225221
verify: verifyOneShot,
226222

227223
// Classes
@@ -242,19 +238,11 @@ module.exports = {
242238
secureHeapUsed,
243239
};
244240

245-
function setFipsDisabled() {
246-
throw new ERR_CRYPTO_FIPS_UNAVAILABLE();
247-
}
248-
249241
function setFipsForced(val) {
250242
if (val) return;
251243
throw new ERR_CRYPTO_FIPS_FORCED();
252244
}
253245

254-
function getFipsDisabled() {
255-
return 0;
256-
}
257-
258246
function getFipsForced() {
259247
return 1;
260248
}
@@ -276,10 +264,8 @@ ObjectDefineProperties(module.exports, {
276264
},
277265
// crypto.fips is deprecated. DEP0093. Use crypto.getFips()/crypto.setFips()
278266
fips: {
279-
get: !fipsMode ? getFipsDisabled :
280-
fipsForced ? getFipsForced : getFipsCrypto,
281-
set: !fipsMode ? setFipsDisabled :
282-
fipsForced ? setFipsForced : setFipsCrypto
267+
get: fipsForced ? getFipsForced : getFipsCrypto,
268+
set: fipsForced ? setFipsForced : setFipsCrypto
283269
},
284270
DEFAULT_ENCODING: {
285271
enumerable: false,

‎test/parallel/test-crypto-fips.js

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,27 +9,20 @@ const spawnSync = require('child_process').spawnSync;
99
const path = require('path');
1010
const fixtures = require('../common/fixtures');
1111
const { internalBinding } = require('internal/test/binding');
12-
const { fipsMode } = internalBinding('config');
12+
const { testFipsCrypto } = internalBinding('crypto');
1313

1414
const FIPS_ENABLED = 1;
1515
const FIPS_DISABLED = 0;
16-
const FIPS_ERROR_STRING =
17-
'Error [ERR_CRYPTO_FIPS_UNAVAILABLE]: Cannot set FIPS mode in a ' +
18-
'non-FIPS build.';
1916
const FIPS_ERROR_STRING2 =
2017
'Error [ERR_CRYPTO_FIPS_FORCED]: Cannot set FIPS mode, it was forced with ' +
2118
'--force-fips at startup.';
22-
const OPTION_ERROR_STRING = 'bad option';
19+
const FIPS_UNSUPPORTED_ERROR_STRING = 'fips mode not supported';
2320

2421
const CNF_FIPS_ON = fixtures.path('openssl_fips_enabled.cnf');
2522
const CNF_FIPS_OFF = fixtures.path('openssl_fips_disabled.cnf');
2623

2724
let num_children_ok = 0;
2825

29-
function compiledWithFips() {
30-
return fipsMode ? true : false;
31-
}
32-
3326
function sharedOpenSSL() {
3427
return process.config.variables.node_shared_openssl;
3528
}
@@ -75,17 +68,17 @@ testHelper(
7568

7669
// --enable-fips should turn FIPS mode on
7770
testHelper(
78-
compiledWithFips() ? 'stdout' : 'stderr',
71+
testFipsCrypto() ? 'stdout' : 'stderr',
7972
['--enable-fips'],
80-
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
73+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
8174
'require("crypto").getFips()',
8275
process.env);
8376

8477
// --force-fips should turn FIPS mode on
8578
testHelper(
86-
compiledWithFips() ? 'stdout' : 'stderr',
79+
testFipsCrypto() ? 'stdout' : 'stderr',
8780
['--force-fips'],
88-
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
81+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
8982
'require("crypto").getFips()',
9083
process.env);
9184

@@ -106,23 +99,23 @@ if (!sharedOpenSSL()) {
10699
testHelper(
107100
'stdout',
108101
[`--openssl-config=${CNF_FIPS_ON}`],
109-
compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
102+
testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
110103
'require("crypto").getFips()',
111104
process.env);
112105

113106
// OPENSSL_CONF should be able to turn on FIPS mode
114107
testHelper(
115108
'stdout',
116109
[],
117-
compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
110+
testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
118111
'require("crypto").getFips()',
119112
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_ON }));
120113

121114
// --openssl-config option should override OPENSSL_CONF
122115
testHelper(
123116
'stdout',
124117
[`--openssl-config=${CNF_FIPS_ON}`],
125-
compiledWithFips() ? FIPS_ENABLED : FIPS_DISABLED,
118+
testFipsCrypto() ? FIPS_ENABLED : FIPS_DISABLED,
126119
'require("crypto").getFips()',
127120
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
128121
}
@@ -136,78 +129,78 @@ testHelper(
136129

137130
// --enable-fips should take precedence over OpenSSL config file
138131
testHelper(
139-
compiledWithFips() ? 'stdout' : 'stderr',
132+
testFipsCrypto() ? 'stdout' : 'stderr',
140133
['--enable-fips', `--openssl-config=${CNF_FIPS_OFF}`],
141-
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
134+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
142135
'require("crypto").getFips()',
143136
process.env);
144137

145138
// OPENSSL_CONF should _not_ make a difference to --enable-fips
146139
testHelper(
147-
compiledWithFips() ? 'stdout' : 'stderr',
140+
testFipsCrypto() ? 'stdout' : 'stderr',
148141
['--enable-fips'],
149-
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
142+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
150143
'require("crypto").getFips()',
151144
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
152145

153146
// --force-fips should take precedence over OpenSSL config file
154147
testHelper(
155-
compiledWithFips() ? 'stdout' : 'stderr',
148+
testFipsCrypto() ? 'stdout' : 'stderr',
156149
['--force-fips', `--openssl-config=${CNF_FIPS_OFF}`],
157-
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
150+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
158151
'require("crypto").getFips()',
159152
process.env);
160153

161154
// Using OPENSSL_CONF should not make a difference to --force-fips
162155
testHelper(
163-
compiledWithFips() ? 'stdout' : 'stderr',
156+
testFipsCrypto() ? 'stdout' : 'stderr',
164157
['--force-fips'],
165-
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
158+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
166159
'require("crypto").getFips()',
167160
Object.assign({}, process.env, { 'OPENSSL_CONF': CNF_FIPS_OFF }));
168161

169162
// setFipsCrypto should be able to turn FIPS mode on
170163
testHelper(
171-
compiledWithFips() ? 'stdout' : 'stderr',
164+
testFipsCrypto() ? 'stdout' : 'stderr',
172165
[],
173-
compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING,
166+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
174167
'(require("crypto").setFips(true),' +
175168
'require("crypto").getFips())',
176169
process.env);
177170

178171
// setFipsCrypto should be able to turn FIPS mode on and off
179172
testHelper(
180-
compiledWithFips() ? 'stdout' : 'stderr',
173+
testFipsCrypto() ? 'stdout' : 'stderr',
181174
[],
182-
compiledWithFips() ? FIPS_DISABLED : FIPS_ERROR_STRING,
175+
testFipsCrypto() ? FIPS_DISABLED : FIPS_UNSUPPORTED_ERROR_STRING,
183176
'(require("crypto").setFips(true),' +
184177
'require("crypto").setFips(false),' +
185178
'require("crypto").getFips())',
186179
process.env);
187180

188181
// setFipsCrypto takes precedence over OpenSSL config file, FIPS on
189182
testHelper(
190-
compiledWithFips() ? 'stdout' : 'stderr',
183+
testFipsCrypto() ? 'stdout' : 'stderr',
191184
[`--openssl-config=${CNF_FIPS_OFF}`],
192-
compiledWithFips() ? FIPS_ENABLED : FIPS_ERROR_STRING,
185+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
193186
'(require("crypto").setFips(true),' +
194187
'require("crypto").getFips())',
195188
process.env);
196189

197190
// setFipsCrypto takes precedence over OpenSSL config file, FIPS off
198191
testHelper(
199-
compiledWithFips() ? 'stdout' : 'stderr',
192+
'stdout',
200193
[`--openssl-config=${CNF_FIPS_ON}`],
201-
compiledWithFips() ? FIPS_DISABLED : FIPS_ERROR_STRING,
194+
FIPS_DISABLED,
202195
'(require("crypto").setFips(false),' +
203196
'require("crypto").getFips())',
204197
process.env);
205198

206199
// --enable-fips does not prevent use of setFipsCrypto API
207200
testHelper(
208-
compiledWithFips() ? 'stdout' : 'stderr',
201+
testFipsCrypto() ? 'stdout' : 'stderr',
209202
['--enable-fips'],
210-
compiledWithFips() ? FIPS_DISABLED : OPTION_ERROR_STRING,
203+
testFipsCrypto() ? FIPS_DISABLED : FIPS_UNSUPPORTED_ERROR_STRING,
211204
'(require("crypto").setFips(false),' +
212205
'require("crypto").getFips())',
213206
process.env);
@@ -216,15 +209,15 @@ testHelper(
216209
testHelper(
217210
'stderr',
218211
['--force-fips'],
219-
compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
212+
testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
220213
'require("crypto").setFips(false)',
221214
process.env);
222215

223216
// --force-fips makes setFipsCrypto enable a no-op (FIPS stays on)
224217
testHelper(
225-
compiledWithFips() ? 'stdout' : 'stderr',
218+
testFipsCrypto() ? 'stdout' : 'stderr',
226219
['--force-fips'],
227-
compiledWithFips() ? FIPS_ENABLED : OPTION_ERROR_STRING,
220+
testFipsCrypto() ? FIPS_ENABLED : FIPS_UNSUPPORTED_ERROR_STRING,
228221
'(require("crypto").setFips(true),' +
229222
'require("crypto").getFips())',
230223
process.env);
@@ -233,14 +226,14 @@ testHelper(
233226
testHelper(
234227
'stderr',
235228
['--force-fips', '--enable-fips'],
236-
compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
229+
testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
237230
'require("crypto").setFips(false)',
238231
process.env);
239232

240233
// --enable-fips and --force-fips order does not matter
241234
testHelper(
242235
'stderr',
243236
['--enable-fips', '--force-fips'],
244-
compiledWithFips() ? FIPS_ERROR_STRING2 : OPTION_ERROR_STRING,
237+
testFipsCrypto() ? FIPS_ERROR_STRING2 : FIPS_UNSUPPORTED_ERROR_STRING,
245238
'require("crypto").setFips(false)',
246239
process.env);

‎test/parallel/test-process-env-allowed-flags-are-documented.js

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,6 @@ const conditionalOpts = [
5353
'--secure-heap-min',
5454
].includes(opt);
5555
}
56-
}, {
57-
// We are using openssl_is_fips from the configuration because it could be
58-
// the case that OpenSSL is FIPS compatible but fips has not been enabled
59-
// (starting node with --enable-fips). If we use common.hasFipsCrypto
60-
// that would only tells us if fips has been enabled, but in this case we
61-
// want to check options which will be available regardless of whether fips
62-
// is enabled at runtime or not.
63-
include: process.config.variables.openssl_is_fips,
64-
filter: (opt) => opt.includes('-fips')
6556
}, {
6657
include: common.hasIntl,
6758
filter: (opt) => opt === '--icu-data-dir'

0 commit comments

Comments
 (0)