nodejs / security-wg

Node.js Ecosystem Security Working Group
MIT License
501 stars 122 forks source link

Abort when vulnerable flag #852

Open RafaelGSS opened 1 year ago

RafaelGSS commented 1 year ago

As previously discussed in #846. I'm creating this issue to discuss the possibility of including a flag (--abort-when-vulnerable) that will abort the node.js process if the version contains a known vulnerability.

The idea is pretty simple: perform a remote call and check against our database: https://github.com/nodejs/security-wg/tree/main/vuln/core if the version in use is vulnerable. We will need to include more metadata such as the operating system, but that's easy to do.

A similar discussion is in progress on https://github.com/nodejs/node/issues/44942 and one of the concerns raised was the necessity to perform a remote call. However, since this feature is also opt-in, I don't see why it could be a problem.

Just to summarize:

While it could just as easily be a module, I don't think it would be as reliable as having it as an option in Node.js. But, of course, I'm open to discussing it.

I'm tagging @nodejs/tsc to get more visibility on it before any work.

cc: @nodejs/security-wg

tniessen commented 1 year ago

While it could just as easily be a module, I don't think it would be as reliable as having it as an option in Node.js.

What's the reasoning behind this? If node --abort-when-vulnerable exits either way, how is it better than npx -y is-my-node-vulnerable? (Sure, node might have fewer vulnerabilities itself than npx or is-my-node-vulnerable, but if there is a TLS vulnerability in node, the first command is hazardous as well.)

RafaelGSS commented 1 year ago

While it could just as easily be a module, I don't think it would be as reliable as having it as an option in Node.js.

What's the reasoning behind this? If node --abort-when-vulnerable exits either way, how is it better than npx -y is-my-node-vulnerable? (Sure, node might have fewer vulnerabilities itself than npx or is-my-node-vulnerable, but if there is a TLS vulnerability in node, the first command is hazardous as well.)

I mean, I'm fine with a separate module, I just don't think people are going to use it. That's pretty much the same analogy for --check-updates.

Neither do I think we should include that flag based just in my guess. We're going to discuss it on Security-WG. I'll elaborate on it soon.

RafaelGSS commented 1 year ago

Discussed it on Security WG 05/01 - I'll proceed with a separate module and see how it goes.

RafaelGSS commented 1 year ago

So, I built it https://github.com/RafaelGSS/is-my-node-vulnerable. Let's see how the community adopts it.

RafaelGSS commented 1 year ago

Related topic https://twitter.com/threepointone/status/1627270416615055360

RafaelGSS commented 2 months ago

Hey,

I've been reconsidering and even after releasing https://github.com/RafaelGSS/is-my-node-vulnerable, I still believe it would be a valuable addition to the Node.js core. It will likely gain more adoption, especially since I don't have a large public audience as a regular user. is-my-node-vulnerable got very good feedback and it's pretty stable right now.

That said, I'd love to discuss it again if possible

@nodejs/tsc @nodejs/security-wg

ljharb commented 2 months ago

Would this flag just immediately abort when a node version went out of support, even if there weren't any vulns in it?

RafaelGSS commented 2 months ago

Would this flag just immediately abort when a node version went out of support, even if there weren't any vulns in it?

It will behave similarly to 'is-my-node-vulnerable'. If a user is using an EOL version and no security release happened after it became EOL, it won't abort but will warn.

richardlau commented 2 months ago

FTR Since this would have to "phone home" to get an updated list of vulnerabilities I would be against any form of this being on by default (which isn't being proposed here), but will not block it being opt-in.

marco-ippolito commented 2 months ago

What if the vuln list is not available ? (network issues, infrastructure issues)

RafaelGSS commented 1 month ago

FTR Since this would have to "phone home" to get an updated list of vulnerabilities I would be against any form of this being on by default (which isn't being proposed here), but will not block it being opt-in.

Yeah, I agree that it shouldn't ever be a thing that will be on by default. Currently, is-my-node-vulnerable stores a local cache of https://github.com/nodejs/security-wg/blob/main/vuln/core/index.json and checks if ETag header has changed and if so, it updates the local cache before listing vulnerabilities. For this specific scenario, I don't see a better way than a remote call. Shipping it with the binary will obviously cause inconsistencies.

My idea is to have a --abort-if-vulnerable flag that will throw if process.version is vulnerable to a public Node.js CVE or warn if: EOL or the vulnerability list isn't available.

RafaelGSS commented 1 month ago

Echoing the message I sent to Node.js TSC.

I was sleeping on it a bit and I think adding it to the core might not be worth it as I thought it would be. It would need to cover a bunch of edge cases that will make this feature untrustworthy for early adopters. However, if we (Node.js account) promote a bit the package itself it might reach a good audience. What do you think about it? if needed I can transfer the package to the nodejs org

cc: @nodejs/security-wg

UPDATE from last security meeting: