Skip to content

Commit f5da8f8

Browse files
authored
crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
Reverts: #56160 Fixes: #56159 Fixes: #58913 Refs: #58121 PR-URL: #58942 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
1 parent 0534672 commit f5da8f8

File tree

7 files changed

+91
-22
lines changed

7 files changed

+91
-22
lines changed

doc/api/deprecations.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4061,6 +4061,19 @@ Type: Documentation-only
40614061

40624062
The [`util.types.isNativeError`][] API is deprecated. Please use [`Error.isError`][] instead.
40634063

4064+
### DEP0198: Creating SHAKE-128 and SHAKE-256 digests without an explicit `options.outputLength`
4065+
4066+
<!-- YAML
4067+
changes:
4068+
- version: REPLACEME
4069+
pr-url: https://github.com/nodejs/node/pull/58942
4070+
description: Documentation-only deprecation with support for `--pending-deprecation`.
4071+
-->
4072+
4073+
Type: Documentation-only (supports [`--pending-deprecation`][])
4074+
4075+
Creating SHAKE-128 and SHAKE-256 digests without an explicit `options.outputLength` is deprecated.
4076+
40644077
[DEP0142]: #dep0142-repl_builtinlibs
40654078
[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
40664079
[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3

lib/internal/crypto/hash.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
const {
44
ObjectSetPrototypeOf,
55
ReflectApply,
6+
StringPrototypeReplace,
67
StringPrototypeToLowerCase,
78
Symbol,
89
} = primordials;
@@ -33,6 +34,8 @@ const {
3334
lazyDOMException,
3435
normalizeEncoding,
3536
encodingsMap,
37+
isPendingDeprecation,
38+
getDeprecationWarningEmitter,
3639
} = require('internal/util');
3740

3841
const {
@@ -63,6 +66,25 @@ const LazyTransform = require('internal/streams/lazy_transform');
6366
const kState = Symbol('kState');
6467
const kFinalized = Symbol('kFinalized');
6568

69+
/**
70+
* @param {string} name
71+
*/
72+
function normalizeAlgorithm(name) {
73+
return StringPrototypeReplace(StringPrototypeToLowerCase(name), '-', '');
74+
}
75+
76+
const maybeEmitDeprecationWarning = getDeprecationWarningEmitter(
77+
'DEP0198',
78+
'Creating SHAKE128/256 digests without an explicit options.outputLength is deprecated.',
79+
undefined,
80+
false,
81+
(algorithm) => {
82+
if (!isPendingDeprecation()) return false;
83+
const normalized = normalizeAlgorithm(algorithm);
84+
return normalized === 'shake128' || normalized === 'shake256';
85+
},
86+
);
87+
6688
function Hash(algorithm, options) {
6789
if (!new.target)
6890
return new Hash(algorithm, options);
@@ -80,6 +102,9 @@ function Hash(algorithm, options) {
80102
this[kState] = {
81103
[kFinalized]: false,
82104
};
105+
if (!isCopy && xofLen === undefined) {
106+
maybeEmitDeprecationWarning(algorithm);
107+
}
83108
ReflectApply(LazyTransform, this, [options]);
84109
}
85110

@@ -213,6 +238,12 @@ function hash(algorithm, input, outputEncoding = 'hex') {
213238
}
214239
}
215240
}
241+
// TODO: ideally we have to ship https://github.com/nodejs/node/pull/58121 so
242+
// that a proper DEP0198 deprecation can be done here as well.
243+
const normalizedAlgorithm = normalizeAlgorithm(algorithm);
244+
if (normalizedAlgorithm === 'shake128' || normalizedAlgorithm === 'shake256') {
245+
return new Hash(algorithm).update(input).digest(normalized);
246+
}
216247
return oneShotDigest(algorithm, getCachedHashId(algorithm), getHashCache(),
217248
input, normalized, encodingsMap[normalized]);
218249
}

lib/internal/util.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ function getDeprecationWarningEmitter(
110110
shouldEmitWarning = () => true,
111111
) {
112112
let warned = false;
113-
return function() {
114-
if (!warned && shouldEmitWarning()) {
113+
return function(arg) {
114+
if (!warned && shouldEmitWarning(arg)) {
115115
warned = true;
116116
if (code === 'ExperimentalWarning') {
117117
process.emitWarning(msg, code, deprecated);
@@ -1011,4 +1011,6 @@ module.exports = {
10111011
setOwnProperty,
10121012
pendingDeprecate,
10131013
WeakReference,
1014+
isPendingDeprecation,
1015+
getDeprecationWarningEmitter,
10141016
};

src/crypto/crypto_hash.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,18 @@ bool Hash::HashInit(const EVP_MD* md, Maybe<unsigned int> xof_md_len) {
331331
}
332332

333333
md_len_ = mdctx_.getDigestSize();
334+
// TODO(@panva): remove this behaviour when DEP0198 is End-Of-Life
335+
if (mdctx_.hasXofFlag() && !xof_md_len.IsJust() && md_len_ == 0) {
336+
const char* name = OBJ_nid2sn(EVP_MD_type(md));
337+
if (name != nullptr) {
338+
if (strcmp(name, "SHAKE128") == 0) {
339+
md_len_ = 16;
340+
} else if (strcmp(name, "SHAKE256") == 0) {
341+
md_len_ = 32;
342+
}
343+
}
344+
}
345+
334346
if (xof_md_len.IsJust() && xof_md_len.FromJust() != md_len_) {
335347
// This is a little hack to cause createHash to fail when an incorrect
336348
// hashSize option was passed for a non-XOF hash function.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Flags: --pending-deprecation
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
8+
const { createHash } = require('crypto');
9+
10+
common.expectWarning({
11+
DeprecationWarning: {
12+
DEP0198: 'Creating SHAKE128/256 digests without an explicit options.outputLength is deprecated.',
13+
}
14+
});
15+
16+
{
17+
createHash('shake128').update('test').digest();
18+
}

test/parallel/test-crypto-hash.js

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ const assert = require('assert');
88
const crypto = require('crypto');
99
const fs = require('fs');
1010

11-
const { hasOpenSSL } = require('../common/crypto');
1211
const fixtures = require('../common/fixtures');
1312

1413
let cryptoType;
@@ -184,21 +183,19 @@ assert.throws(
184183

185184
// Test XOF hash functions and the outputLength option.
186185
{
187-
// Default outputLengths. Since OpenSSL 3.4 an outputLength is mandatory
188-
if (!hasOpenSSL(3, 4)) {
189-
assert.strictEqual(crypto.createHash('shake128').digest('hex'),
190-
'7f9c2ba4e88f827d616045507605853e');
191-
assert.strictEqual(crypto.createHash('shake128', null).digest('hex'),
192-
'7f9c2ba4e88f827d616045507605853e');
193-
assert.strictEqual(crypto.createHash('shake256').digest('hex'),
194-
'46b9dd2b0ba88d13233b3feb743eeb24' +
195-
'3fcd52ea62b81b82b50c27646ed5762f');
196-
assert.strictEqual(crypto.createHash('shake256', { outputLength: 0 })
197-
.copy() // Default outputLength.
198-
.digest('hex'),
199-
'46b9dd2b0ba88d13233b3feb743eeb24' +
200-
'3fcd52ea62b81b82b50c27646ed5762f');
201-
}
186+
// Default outputLengths.
187+
assert.strictEqual(crypto.createHash('shake128').digest('hex'),
188+
'7f9c2ba4e88f827d616045507605853e');
189+
assert.strictEqual(crypto.createHash('shake128', null).digest('hex'),
190+
'7f9c2ba4e88f827d616045507605853e');
191+
assert.strictEqual(crypto.createHash('shake256').digest('hex'),
192+
'46b9dd2b0ba88d13233b3feb743eeb24' +
193+
'3fcd52ea62b81b82b50c27646ed5762f');
194+
assert.strictEqual(crypto.createHash('shake256', { outputLength: 0 })
195+
.copy() // Default outputLength.
196+
.digest('hex'),
197+
'46b9dd2b0ba88d13233b3feb743eeb24' +
198+
'3fcd52ea62b81b82b50c27646ed5762f');
202199

203200
// Short outputLengths.
204201
assert.strictEqual(crypto.createHash('shake128', { outputLength: 0 })

test/parallel/test-crypto-oneshot-hash.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ if (!common.hasCrypto)
88
const assert = require('assert');
99
const crypto = require('crypto');
1010
const fixtures = require('../common/fixtures');
11-
const { hasOpenSSL } = require('../common/crypto');
1211
const fs = require('fs');
1312

1413
// Test errors for invalid arguments.
@@ -32,9 +31,6 @@ const methods = crypto.getHashes();
3231
const input = fs.readFileSync(fixtures.path('utf8_test_text.txt'));
3332

3433
for (const method of methods) {
35-
// Skip failing tests on OpenSSL 3.4.0
36-
if (method.startsWith('shake') && hasOpenSSL(3, 4))
37-
continue;
3834
for (const outputEncoding of ['buffer', 'hex', 'base64', undefined]) {
3935
const oldDigest = crypto.createHash(method).update(input).digest(outputEncoding || 'hex');
4036
const digestFromBuffer = crypto.hash(method, input, outputEncoding);

0 commit comments

Comments
 (0)