Skip to content

Commit 6699c75

Browse files
panvaaduh95
authored andcommitted
crypto: fix SHAKE128/256 breaking change introduced with OpenSSL 3.4
Reverts: #56160Fixes: #56159Fixes: #58913 Refs: #58121 PR-URL: #58942 Backport-PR-URL: #58960 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent 26f3711 commit 6699c75

7 files changed

Lines changed: 94 additions & 23 deletions

File tree

‎doc/api/deprecations.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3850,6 +3850,21 @@ To make [`child_process.exec`][] invoke the default shell, either omit the
38503850
`shell` option, or set it to a nullish value. If the intention is not to invoke
38513851
a shell, use [`child_process.execFile`][] instead.
38523852

3853+
<!-- md-lint skip-deprecation DEP0197 -->
3854+
3855+
### DEP0198: Creating SHAKE-128 and SHAKE-256 digests without an explicit `options.outputLength`
3856+
3857+
<!-- YAML
3858+
changes:
3859+
- version: REPLACEME
3860+
pr-url: https://github.com/nodejs/node/pull/58942
3861+
description: Documentation-only deprecation with support for `--pending-deprecation`.
3862+
-->
3863+
3864+
Type: Documentation-only (supports [`--pending-deprecation`][])
3865+
3866+
Creating SHAKE-128 and SHAKE-256 digests without an explicit `options.outputLength` is deprecated.
3867+
38533868
[DEP0142]: #dep0142-repl_builtinlibs
38543869
[NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
38553870
[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
constkState=Symbol('kState');
6467
constkFinalized=Symbol('kFinalized');
6568

69+
/**
70+
* @param {string} name
71+
*/
72+
functionnormalizeAlgorithm(name){
73+
returnStringPrototypeReplace(StringPrototypeToLowerCase(name),'-','');
74+
}
75+
76+
constmaybeEmitDeprecationWarning=getDeprecationWarningEmitter(
77+
'DEP0198',
78+
'Creating SHAKE128/256 digests without an explicit options.outputLength is deprecated.',
79+
undefined,
80+
false,
81+
(algorithm)=>{
82+
if(!isPendingDeprecation())returnfalse;
83+
constnormalized=normalizeAlgorithm(algorithm);
84+
returnnormalized==='shake128'||normalized==='shake256';
85+
},
86+
);
87+
6688
functionHash(algorithm,options){
6789
if(!new.target)
6890
returnnewHash(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+
constnormalizedAlgorithm=normalizeAlgorithm(algorithm);
244+
if(normalizedAlgorithm==='shake128'||normalizedAlgorithm==='shake256'){
245+
returnnewHash(algorithm).update(input).digest(normalized);
246+
}
216247
returnoneShotDigest(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
letwarned=false;
113-
returnfunction(){
114-
if(!warned&&shouldEmitWarning()){
113+
returnfunction(arg){
114+
if(!warned&&shouldEmitWarning(arg)){
115115
warned=true;
116116
if(code==='ExperimentalWarning'){
117117
process.emitWarning(msg,code,deprecated);
@@ -998,4 +998,6 @@ module.exports = {
998998
setOwnProperty,
999999
pendingDeprecate,
10001000
WeakReference,
1001+
isPendingDeprecation,
1002+
getDeprecationWarningEmitter,
10011003
};

‎src/crypto/crypto_hash.cc‎

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

347347
md_len_ = EVP_MD_size(md);
348+
bool is_xof = (EVP_MD_flags(md) & EVP_MD_FLAG_XOF) != 0;
349+
if (is_xof && !xof_md_len.IsJust() && md_len_ == 0) {
350+
constchar* name = OBJ_nid2sn(EVP_MD_type(md));
351+
if (name != nullptr) {
352+
if (strcmp(name, "SHAKE128") == 0) {
353+
md_len_ = 16;
354+
} elseif (strcmp(name, "SHAKE256") == 0) {
355+
md_len_ = 32;
356+
}
357+
}
358+
}
359+
348360
if (xof_md_len.IsJust() && xof_md_len.FromJust() != md_len_) {
349361
// This is a little hack to cause createHash to fail when an incorrect
350362
// hashSize option was passed for a non-XOF hash function.
351-
if ((EVP_MD_flags(md) & EVP_MD_FLAG_XOF) == 0) {
363+
if (!is_xof) {
352364
EVPerr(EVP_F_EVP_DIGESTFINALXOF, EVP_R_NOT_XOF_OR_INVALID_LENGTH);
353365
returnfalse;
354366
}
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+
constcommon=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
constcrypto=require('crypto');
99
constfs=require('fs');
1010

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

1413
letcryptoType;
@@ -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
constassert=require('assert');
99
constcrypto=require('crypto');
1010
constfixtures=require('../common/fixtures');
11-
const{ hasOpenSSL }=require('../common/crypto');
1211
constfs=require('fs');
1312

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

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

0 commit comments

Comments
 (0)