Is it Node policy that backwards incompatible security fixes are released in patch releases?

[adevine]

It looks like there were a number of fixes in Node 20.11.1 that were security fixes, where the implementation just disabled some code. For example, this commit, 7079c062bb, "disable PKCS#1 padding for privateDecrypt", broke some widely used code from OpenPGP (see openpgpjs/openpgpjs#1727).

I completely understand why this change was made, but why was it done in a patch release? This change is by definition not backwards compatible, so why wasn't it held off to a major release? Is there a general policy that backwards-incompatible security fixes are OK for patch releases? The Marvin attack described in the CVE, CVE-2023-46809, that this change is meant to fix was very low risk to us based on our usage of OpenPGP. Again, I of course understand that this may be critical risk to other users, but just generally want to understand why this change was made in a patch release.

[richardlau]

The policy allows backwards incompatible changes to land in existing release lines, but they should have caused the version to increment to the next semver-minor:

https://github.com/nodejs/Release/blob/42a1d7c5f3bce8d2efc8d96c7aeb147a173ed396/README.md?plain=1#L36-L38

Changes required for critical security and bug fixes may lead to semver-major
changes landing within a release stream, such situations will be rare and will
land as semver-minor. Although, those changes should have a revert option included.

Perhaps https://nodejs.org/en/blog/vulnerability/february-2024-security-releases/ could have explicitly mentioned the --security-revert=CVE-2023-46809 revert flag.

[adevine]

Just wanted to say thanks, greatly appreciate the detailed and quick response.

[adevine]

Following up on this, again thanks for your earlier response @richardlau. Upon further testing, just want to point out the the change in the recent security patch releases is going to break a ton of code everywhere. I updated the issue I filed on OpenPGP, openpgpjs/openpgpjs#1727, but this breaks any usage of OpenPGP that uses RSA keys. The "fix" in the patch release was simply to disable PKCS#1 in privateDecrypt, but note that PKCS#1 v1.5 is heavily used in a ton of places.

Basically, I understand the argument that RSA PKCS#1 has some fundamental issues, but I question if this is the right path forward.

[mhdawson]

The --security-revert=CVE-2023-46809 revert flag is in place to allow reverting to the old behavior. Does this not work in the use cases you are running into?

The other thing in play is that if a version of OpenSSL is used which has addressed the issue, Node.js will detect that and not throw an error.

[adevine]

Can't speak for others, but lots of times when running in hosted environments (i.e. Google's App Engine) you don't have access to set Node startup flags.

The OpenPGP library just released a fix that basically just bypasses the core node crypto libraries, so my code is working, but I still feel that just disabling the ability to handle PKCS#1 in privateDecrypt is the wrong approach. I mean, while I don't understand the full details of the Marvin attack, isn't this blocking it in the wrong place? That is, if somebody previously encrypted a whole bunch of data, isn't saying at the point of decryption "sorry, there is a security issue there, we can't decrypt it" already too late? I'm just concerned other people will have a bunch of encrypted files and then only discover their decryption has been disabled after they upgrade their node version and stuff stops working. That is, it seems wrong to me that decrypting was blocked but encrypting is fine.

Also, and again I fully accept that as a non-cryptographer I could be off base here, but it looks like some of the other libraries that were affected were able to implement mitigations without disabling it.

[tomato42]

The attack is happens on the decryption side: i.e. the side that has access to the private key. So it's the automated and unattended decryption that is vulnerable to the attack.

The correct way to handle data encrypted with RSAES-PKCS1-v1_5 is to stop accepting RSAES-PKCS1-v1_5 and accept only RSAES-OAEP. If the standard or implementation you're working with doesn't support RSAES-OAEP, a standard that is 26 years old at this point, maybe it's time to start using something a little bit better...

If you already have stuff encrypted with RSAES-PKCS1-v1_5, you should do a one-off, manual migration to RSAES-OAEP, or simply decrypting the data and storing it decrypted (using database encryption or filesystem encryption is also an option).

Also, looking at that fix... I'm pretty sure that https://github.com/openpgpjs/openpgpjs/ is vulnerable to the Marvin Attack too...

[tomato42]

Also, please check the commit that added this check: 54cd268, it triggers only if Node.js is running with OpenSSL that does not implement implicit rejection. Complain to your OS vendor if they haven't backported that option from OpenSSL 3.2.0.