Skip to content

Commit 068c439

Browse files
panvamarco-ippolito
authored andcommitted
crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
Reverts: #56160 Fixes: #56159 Fixes: #58913 Refs: #58121 PR-URL: #58942 Backport-PR-URL: #58961 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 fad773c commit 068c439

File tree

7 files changed

+112
-22
lines changed

7 files changed

+112
-22
lines changed

doc/api/deprecations.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3589,6 +3589,39 @@ Type: Documentation-only
35893589
Passing non-supported argument types is deprecated and, instead of returning `false`,
35903590
will throw an error in a future version.
35913591

3592+
<!-- md-lint skip-deprecation DEP0188 -->
3593+
3594+
<!-- md-lint skip-deprecation DEP0189 -->
3595+
3596+
<!-- md-lint skip-deprecation DEP0190 -->
3597+
3598+
<!-- md-lint skip-deprecation DEP0191 -->
3599+
3600+
<!-- md-lint skip-deprecation DEP0192 -->
3601+
3602+
<!-- md-lint skip-deprecation DEP0193 -->
3603+
3604+
<!-- md-lint skip-deprecation DEP0194 -->
3605+
3606+
<!-- md-lint skip-deprecation DEP0195 -->
3607+
3608+
<!-- md-lint skip-deprecation DEP0196 -->
3609+
3610+
<!-- md-lint skip-deprecation DEP0197 -->
3611+
3612+
### DEP0198: Creating SHAKE-128 and SHAKE-256 digests without an explicit `options.outputLength`
3613+
3614+
<!-- YAML
3615+
changes:
3616+
- version: REPLACEME
3617+
pr-url: https://github.com/nodejs/node/pull/58942
3618+
description: Documentation-only deprecation with support for `--pending-deprecation`.
3619+
-->
3620+
3621+
Type: Documentation-only (supports [`--pending-deprecation`][])
3622+
3623+
Creating SHAKE-128 and SHAKE-256 digests without an explicit `options.outputLength` is deprecated.
3624+
35923625
[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
35933626
[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
35943627
[RFC 8247 Section 2.4]: https://www.rfc-editor.org/rfc/rfc8247#section-2.4

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
@@ -104,8 +104,8 @@ function getDeprecationWarningEmitter(
104104
shouldEmitWarning = () => true,
105105
) {
106106
let warned = false;
107-
return function() {
108-
if (!warned && shouldEmitWarning()) {
107+
return function(arg) {
108+
if (!warned && shouldEmitWarning(arg)) {
109109
warned = true;
110110
if (code !== undefined) {
111111
if (!codesWarned.has(code)) {
@@ -994,4 +994,6 @@ module.exports = {
994994
setOwnProperty,
995995
pendingDeprecate,
996996
WeakReference,
997+
isPendingDeprecation,
998+
getDeprecationWarningEmitter,
997999
};

src/crypto/crypto_hash.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,10 +341,22 @@ bool Hash::HashInit(const EVP_MD* md, Maybe<unsigned int> xof_md_len) {
341341
}
342342

343343
md_len_ = EVP_MD_size(md);
344+
bool is_xof = (EVP_MD_flags(md) & EVP_MD_FLAG_XOF) != 0;
345+
if (is_xof && !xof_md_len.IsJust() && md_len_ == 0) {
346+
const char* name = OBJ_nid2sn(EVP_MD_type(md));
347+
if (name != nullptr) {
348+
if (strcmp(name, "SHAKE128") == 0) {
349+
md_len_ = 16;
350+
} else if (strcmp(name, "SHAKE256") == 0) {
351+
md_len_ = 32;
352+
}
353+
}
354+
}
355+
344356
if (xof_md_len.IsJust() && xof_md_len.FromJust() != md_len_) {
345357
// This is a little hack to cause createHash to fail when an incorrect
346358
// hashSize option was passed for a non-XOF hash function.
347-
if ((EVP_MD_flags(md) & EVP_MD_FLAG_XOF) == 0) {
359+
if (!is_xof) {
348360
EVPerr(EVP_F_EVP_DIGESTFINALXOF, EVP_R_NOT_XOF_OR_INVALID_LENGTH);
349361
return false;
350362
}
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
@@ -7,7 +7,6 @@ const assert = require('assert');
77
const crypto = require('crypto');
88
const fs = require('fs');
99

10-
const { hasOpenSSL } = common;
1110
const fixtures = require('../common/fixtures');
1211

1312
let cryptoType;
@@ -183,21 +182,19 @@ assert.throws(
183182

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

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

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,6 @@ const methods = crypto.getHashes();
3131
const input = fs.readFileSync(fixtures.path('utf8_test_text.txt'));
3232

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

0 commit comments

Comments
 (0)