Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test test/parallel/test-crypto-oneshot-hash.js fails in OpenSSL 3.4.0 due to breaking changes in OpenSSL #56159

Open
jelly opened this issue Dec 6, 2024 · 1 comment

Comments

@jelly
Copy link

jelly commented Dec 6, 2024

Version

23.3.0 and main

Platform

Linux X 6.12.1-arch1-1 #1 SMP PREEMPT_DYNAMIC Fri, 22 Nov 2024 16:04:27 +0000 x86_64 GNU/Linux

Subsystem

crypto

What steps will reproduce the bug?

const crypto = require('crypto');
crypto.hash('shake128', "wefwwfe", "utf-8")

How often does it reproduce? Is there a required condition?

100%

What is the expected behavior? Why is that the expected behavior?

Hashing does not throw an Error

What do you see instead?

> crypto.hash('shake128', "wefwwfe", "utf-8")
Uncaught Error: error:00000000:lib(0)::reason(0)
    at Object.hash (node:internal/crypto/hash:216:10)
>

Additional information

This is due to a breaking change in OpenSSL 3.4.0 which makes providing an output length mandatory.

See also, how this is completely broken.

[jelle@natrium][~/projects/node]%openssl shake128 /etc/os-release
[jelle@natrium][~/projects/node]%

Further writeup available here.

@jelly jelly changed the title Running tests against OpenSSL 3.4 breaks crypto.hash('shake128', "wefwwfe", "utf-8") Test test/parallel/test-crypto-oneshot-hash.js fails in OpenSSL 3.4.0 due to breaking changes in OpenSSL Dec 6, 2024
@adrien-n
Copy link

I started looking at the issue too and AFAIU, crypto.hash() needs an API change to pass options and outputLength in particular in order to match the change in openssl (and I don't think the change in openssl is bad). It seems sensible to me to have options in hash just like there are options in createHash.

Also, code as what follows does not make the libraries give an error but the result is erroneous nonetheless:

crypto.createHash('shake128').update('foo').digest('base64')

Thus, the question is whether not passing options should be accepted. Not preventing progress after such code seems like a footgun to me but I'm not sure which part should throw an error (digest probably?).

BTW, if you want to read more about the change, the relevant documentation change in openssl is openssl/openssl@ad3f28c

adrien-n added a commit to adrien-n/node that referenced this issue Dec 17, 2024
OpenSSL 3.4 has intentionally broken EVP_DigestFinal for SHAKE128 and
SHAKE256 when OSSL_DIGEST_PARAM_XOFLEN is not set because a) the default
length used weakened them from their maximum strength and b) a static
length does not fully make sense for XOFs (which SHAKE* are).

Unfortunately, while crypto.createHash accepts an option argument that can
be something like `{ outputLength: 128 }`, crypto.hash doesn't offer a
similar API. Therefore there is little choice but to skip the test
completely for shake128 and shake256 on openssl >= 3.4.

Refs: nodejs#56159
Refs: openssl/openssl@b911fef
Refs: openssl/openssl@ad3f28c
adrien-n added a commit to adrien-n/node that referenced this issue Dec 17, 2024
OpenSSL 3.4 has intentionally broken EVP_DigestFinal for SHAKE128 and
SHAKE256 when OSSL_DIGEST_PARAM_XOFLEN is not set because a) the default
length used weakened them from their maximum strength and b) a static
length does not fully make sense for XOFs (which SHAKE* are).

Unfortunately, while crypto.createHash accepts an option argument that can
be something like `{ outputLength: 128 }`, crypto.hash doesn't offer a
similar API. Therefore there is little choice but to skip the test
completely for shake128 and shake256 on openssl >= 3.4.

Fixes: nodejs#56159
Refs: openssl/openssl@b911fef
Refs: openssl/openssl@ad3f28c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants