Enable --security-revert to be used in NODE_OPTIONS environment variable

[singyantam]

What is the problem this feature will solve?

When Nodejs is used for AWS lambda, the only mechanism available to supply Nodejs options is via NODE_OPTIONS environment variable. The recent Feb 24 security release started blocking use of RSA_PKCS1_PADDING in crypto module's privateDecrypt method. For any major company that always keep up to date with security patches, this new release immediate block Nodejs lambda's ability to process legacy data that was encrypted with RSA_PKCS1_PADDING.

If the --security-revert option can be specified via NODE_OPTIONS, we have the mechanism to allow Nodejs Lambda's to continue processing legacy data while remediation is developed and implemented.

What is the feature you are proposing to solve the problem?

Just allow --security-revert to be specified and acknowledge using NODE_OPTIONS - and not restrict it to be on the command line.

What alternatives have you considered?

No other possibility when dealing with AWS Lambda Nodejs runtime.

[WilliamABradley]

I'm running into the same issue with AWS Lambda, and hit this with an API that is outside of my control.

As a workaround, we can use node-rsa:

const crypto = require("crypto");
const NodeRSA = require("node-rsa");

const keypair = crypto.generateKeyPairSync("rsa", {
  modulusLength: 2048,
});

const data = Buffer.from("Hello, World!");

const encrypted = crypto.publicEncrypt(
  {
    key: keypair.publicKey,
    padding: crypto.constants.RSA_PKCS1_PADDING,
  },
  data
);
console.log("encrypted: ", encrypted.toString("base64"));

const keyRSA = new NodeRSA(
  keypair.privateKey.export({ format: "pem", type: "pkcs1" }),
  "private",
  {
    encryptionScheme: "pkcs1",
  }
);

// By default it will use the node crypto library with the CVE
keyRSA.setOptions({ environment: "browser" });

const decrypted = keyRSA.decrypt(encrypted);
console.log("decrypted: ", decrypted.toString());

[singyantam]

Thanks for the suggestion, we wrapped the original privateDecrypt in try catch and use node-rsa as a back up. But we prefer to have the NODE_OPTIONS support since node-rsa is JavaScript based implementation (when you specify browser - otherwise it actually relegate the work back to crypto module) while crypto leverage native code.

…

On Sun, Mar 10, 2024 at 5:34 PM Will Bradley ***@***.***> wrote: I'm running into the same issue with AWS Lambda, and hit this with an API that is outside of my control. As a workaround, we can use node-rsa: const crypto = require("crypto");const NodeRSA = require("node-rsa"); const keypair = crypto.generateKeyPairSync("rsa", { modulusLength: 2048,}); const data = Buffer.from("Hello, World!"); const encrypted = crypto.publicEncrypt( { key: keypair.publicKey, padding: crypto.constants.RSA_PKCS1_PADDING, }, data);console.log("encrypted: ", encrypted.toString("base64")); const keyRSA = new NodeRSA( keypair.privateKey.export({ format: "pem", type: "pkcs1" }), "private", { encryptionScheme: "pkcs1", }); // By default it will use the node crypto library with the CVEkeyRSA.setOptions({ environment: "browser" }); const decrypted = keyRSA.decrypt(encrypted);console.log("decrypted: ", decrypted.toString()); — Reply to this email directly, view it on GitHub <#52017 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANJLIH4NWQ6WGFQ27SAGIPLYXTGWFAVCNFSM6AAAAABENKVUGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBXGM3DSNRRGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[WilliamABradley]

Yeah, I'm not a fan of the workaround (and adding another dependency), and would much rather be able to disable the CVE block with an environment variable or other method.

[fliodhais]

The workaround isn't ideal but at least it works in the case of AWS lambda environment where we can't just simply pass in the NODE_OPTIONS unfortunately this is a good patch solution.

[mhdawson]

I'm surprised that NODE_OPTIONS does not support the security revert command line options. We'll have to see if that was on purpose on an omission due to the way options are allowed in NODE_OPTIONS.

[mhdawson]

It seems like it was a specific decision at the time - #12028 (comment)

[mhdawson]

I've reached out to one of the other maintainers that was part of the original PR/discussion to see if we still believe the exclusion from NODE_OPTIONS is the right way to go.

[melihbirim]

Having node-rsa and environment attribute to 'browser' solved the problem on my end. But the problem is having a fix on node that does break all the code we have without clarifying the exact real solution.
Any system, without a change should work as it is. It just drops the trust on the node versions in which we are not responsible to update to the latest version like in AWS lambda.

[mhdawson]

Talked with one of the other maintainers re the earlier concerns about including --cve-revert in NODE_OPTIONS and those concerns still apply.

One idea we came up with was the option of adding and api along the lines of

process.cveRevert('cve...")

that could be used to turn on a revert programatically. That would require an update to the Node.js application and would not flow through to spawned processes (similar to command line options I think), but would give another option versus the command line.

@singyantam would that approach work in your situation?

[WilliamABradley]

I think that would work for us, I was wondering if I could set process.CVE_... to true manually (As that is true when the security revert is applied), but seeing as this logic is in native code and not userland, that would not do anything.

[singyantam]

Michael, This should work for us since we already have to make code changes to use node-rsa.

…

On Wed, Mar 13, 2024 at 5:01 PM Michael Dawson ***@***.***> wrote: Talked with one of the other maintainers re the earlier concerns about including --cve-revert in NODE_OPTIONS and those concerns still apply. One idea we came up with was the option of adding and api along the lines of process.cveRevert('cve...") that could be used to turn on a revert programatically. That would require an update to the Node.js application and would not flow through to spawned processes (similar to command line options I think), but would give another option versus the command line. @singyantam <https://github.com/singyantam> would that approach work in your situation? — Reply to this email directly, view it on GitHub <#52017 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANJLIH2UUGYYK4DC4DDYIELYYC5BHAVCNFSM6AAAAABENKVUGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJVG44TQMJQGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mhdawson]

Draft PR from looking at adding process.cveRevert - #52090

@jasnell could you take a llook and let me know if the approach looks ok to you. If so I'll add the documentation. The other question I have is if we might want to leave in a few test entries like I have so that we could have tests to validate the functionality. People could enable them, but of course they would have no effect so I'm thinking it might be ok as a tradeoff for being able to have some testing. If you agree that makes sense I could then add some tests as well.

[andclt]

No other possibility when dealing with AWS Lambda Nodejs runtime.

Not as handy as NODE_OPTIONS env var, but it is possible to set --security-revert command line option in Lambda by using a wrapper script: https://docs.aws.amazon.com/lambda/latest/dg/runtimes-modify.html#runtime-wrapper

[mhdawson]

@singyantam would the wrapper script suggestion in #52017 (comment) work for you ?

[singyantam]

Michael, The wrapper script is centrally controlled by our DevOps and injected as Lambda Layer, so not something we as developers can adjust. It is currently used to enable Dynatrace among other things. I am 100% they will reject adding --security-revert into it, as this have a much bigger impact than using NODE_OPTIONS I do not think this approach will get any traction in our organization. Thanks

…

On Wed, Mar 20, 2024 at 11:55 AM Michael Dawson ***@***.***> wrote: @singyantam <https://github.com/singyantam> would the wrapper script suggestion in #52017 (comment) <#52017 (comment)> work for you ? — Reply to this email directly, view it on GitHub <#52017 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ANJLIH4XEEHHOEHBAFOQ2JTYZGWQXAVCNFSM6AAAAABENKVUGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBZHEYTKOBUGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mhdawson]

@singyantam in that case I think you trying to explain your challenge more in #52090 might be useful. Right now it's not gaining traction.

[if-fi]

We are facing the same problem i Google/Firebase Cloud Functions since yesterday.
The NodeRsa workaround does work, but adding --security-revert in NODE_OPTIONS would be much cleaner solution.

[tniessen]

Is the main concern with NODE_OPTIONS that it might be inherited by child processes unintentionally?

[mhdawson]

@tniessen I believe that would be the concern.

[tniessen]

My main concern with process.cveRevert is that it can also be used with --require and thus effectively through NODE_OPTIONS, but I do see the advantage of not implicitly inheriting it unless someone intentionally uses NODE_OPTIONS that way. Still, regardless of the threat model, I think it might be preferable to only allow such security-critical configuration before any user code runs.

Does AWS allow setting other environment variables? Theoretically, we could also add a new environment variable NODE_SECURITY_REVERT that, if set, emits a warning, is applied just like --security-revert, and is then cleared unless the value ends with +sticky or so. (This is not elegant by any means, of course, I'm just trying to come up with workarounds.)

[WilliamABradley]

@tniessen AWS Lambdas can set any environment variable, it is just modifying node arguments which is difficult

[mhdawson]

@tniessen great to have other suggestions. The main different as I understand it is that the user could control the propagation, right?

[tniessen]

@mhdawson Yes, and no user code can change the setting at runtime (except for child processes, but users can always pass --security-revert to child processes as well). It would also work for hypothetical scenarios in which some configuration is only possible before running user code, e.g., if it's some V8 flag or libuv setup or so.

[tniessen]

Alternative PR: #52365

[tniessen]

@singyantam @WilliamABradley @if-fi Could you please check if #52365 would solve this issue for you?

[if-fi]

@singyantam @WilliamABradley @if-fi Could you please check if #52365 would solve this issue for you?

It looks like it would solve the Google Cloud Functions issue.

[extremeheat]

From a library standpoint, requiring users to use command line or environment variables to revert seems relatively high friction especially as it's code that has worked before and suddenly stops working on a new node version.

I do understand the security rationale for this change, but is there a reason an override couldn't be added to a call parameter and not relying on environment variables or runtime options (which we don't have much control over from lib standpoint)? We may have to switch to a pure JS implementation to handle this cleanly (thanks @WilliamABradley for the snippet).

+1 to mhdawson's proposal:

process.cveRevert('cve...")

in #52090 -- this would be great!

[github-actions]

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Mark issue so that it is never considered stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.