RFC: audit assertions

[bnb]

There's been an interest expressed in the ecosystem of having some form of counterclaim for advisories surfaced by npm audit. There's been some discussion of a potential counterclaim mechanism for some time, but I've not seen a solution proposed.

The intent of this RFC is to do that - propose a solution. I do not expect that this solution will go through unanimously and unchanged, but I'd like to get something up that can be talked about and addressed both by the ecosystem and by those thinking about Security in the registry and CLI. If the answer to this is "no" with some set of reasons, that's a perfectly reasonable outcome.

I'm particularly interested in feedback from security researchers on why this is / is not a good solution, and what alternatives might be implemented or what tweaks might be made to make it a better solution. I appreciate feedback from developers who care, but also want folks to keep in mind that this has the potential for massively negative impact should a vulnerability get through that should not get through.

[wesleytodd]

A couple of initial thoughts, but I will want to think on this approach a bit more from a holistic sense as well.

1. impactful=true|false might not be enough granularity
2. we need a range specifier for sure (either a part of --module or a separate flag)
3. we need a reason/explanation, otherwise we will loose valuable context. This should be required.
4. we should differentiate between usage as a prod/dev/peer dep as well since in some cases that will matter
5. on the audit/consumer side, we need a mechanism for defining trust.

I think num 5 is my largest concern, all the rest are really implementation details. My first thought on 5 is that these opinions need to be published as structured packages on the registry. Maybe the default one is provided by npm, and is what these claims are registered against, but I know for a fact we will need to override this trust. I think this will be really important to iron out before this lands.

[thescientist13]

Apologies if I'm speaking out of turn here, but in regards this proposal at a high level, but I suppose specifically around this flag in particular (i think this is what @wesleytodd was getting at here maybe? - https://github.com/npm/rfcs/pull/422#issuecomment-888513432)

• --impactful=<boolean> this should just be true or false. If it's true, then an assertion is made that the advisory is impactful to the module passed to --module. If it's false, then an assertion is made that the advisory is not impactful to the module passed to --module.

Isn't the nuance in Dan's post that it really all comes down to the runtime context of the vulnerability in question that matters the most? For example, if there is a critical vulnerability in lodash that is based on malicious user input; if Gatsby is using it for its dev server, then user input likely never enters the equation. But if I am using it in my client side and say supplying user input because i'm using it for a filter tab in my UI from user data, then the vulnerability would apply.

So I guess, who is the intended user / audience of this command? Is it even limited to a certain type of user?

1. Package authors
2. Direct package consumers
3. Transitive package consumers

I would argue it's ~~only~~ likely users 2 + 3 that are in the best context to determine the scope of the impact, but it seems that this command is intended to be run by users 1, but aren't they limited by the lack of their awareness to the direct proximity of the actual target site, if it even applies to them in that case?

I guess I'm not sure if this RFC aims to reconcile that at all, or it is literally just for exposing a way to counter a claim, with the scope of who should run it, is not intended here. Or is the intent that this would be more of a crowd sourcing mechanism, so based on real user input, the severity of a vulnerability could be challenged? Or that the CVE itself would be updated for more context based on the incoming reports?

Apologies for the questions, or if this isn't the right forum, happy to post this somewhere else. 👍

[ljharb]

Package authors absolutely can know - just not always.

[bnb]

What would be most ideal, combined with this, is a way for maintainers to get disclosures that these advisories impact their packages before muggles do, so that there’s a window in which the assertion can be made before any noise has been unnecessarily spread.

@ljharb I am extremely against a system like this. It creates a weird caste system that's incredibly easy to game. There's a company that's done a similar thing, and it could not have been more end-user hostile and easy to get around to get early access to information.

If security researchers disagree with me, I'm more than happy to concede to their recommendation.

[asciimike]

So I guess, who is the intended user / audience of this command? Is it even limited to a certain type of user?

1. Package authors
2. Direct package consumers
3. Transitive package consumers

I would argue it's ~only~ likely users 2 + 3 that are in the best context to determine the scope of the impact, but it seems that this command is intended to be run by users 1, but aren't they limited by the lack of their awareness to the direct proximity of the actual target site, if it even applies to them in that case?

IMO the point of this command is to filter out the noise of "not-actionable" vulnerabilities for package consumers, so I view it as: 1 is the group best able to set the impactful flag (e.g. they will know best if the vulnerability is going to impact everyone, only appears in their dev server, doesn't appear at all, etc.), while 2 and 3 are in the best position to use it (combined with 4: tools that help resolve vulnerabilities [disclosure: I work on Dependabot]).

[thescientist13]

1 is the group best able to set the impactful flag (e.g. they will know best if the vulnerability is going to impact everyone, only appears in their dev server, doesn't appear at all, etc.)

But how would they know all that and / or communicate that without knowing the usage context? (is what I'm not understanding here atm it appears.) Unless there is more nuance in this reporting structure / mechanism that I am missing?

[asciimike]

What would be most ideal, combined with this, is a way for maintainers to get disclosures that these advisories impact their packages before muggles do, so that there’s a window in which the assertion can be made before any noise has been unnecessarily spread.

@ljharb I am extremely against a system like this. It creates a weird caste system that's incredibly easy to game. There's a company that's done a similar thing, and it could not have been more end-user hostile and easy to get around to get early access to information.

If security researchers disagree with me, I'm more than happy to concede to their recommendation.

I totally agree that we don't want to create a "these people are special and get to see vulnerabilities first", but I definitely want to see if we can get maintainers (ideally first of direct dependencies) to signal if the dependency impacts them ASAP.

I view the current noise problem as: $DEEP_DEPENDENCY_IN_POPULAR_PACKAGE is potentially affected by some vulnerability in $DEEPER_DEPENDENCY and today we notify everyone in $POPULAR_PACKAGE about things that often don't affect them. IMO, it's better to get the maintainer of $DEEP_DEPENDENCY to say "this does/doesn't affect my consumers" than to immediately send it to $POPULAR_PACKAGE's maintainers to try and sort out.

[asciimike]

1 is the group best able to set the impactful flag (e.g. they will know best if the vulnerability is going to impact everyone, only appears in their dev server, doesn't appear at all, etc.)

But how would they know all that and / or communicate that without knowing the usage context? (is what I'm not understanding here atm it appears.) Unless there is more nuance in this reporting structure / mechanism that I am missing?

Ah, I think I may be thinking of maintainer/direct/indirect differently, so I'll provide an example. Let's imagine that I maintain $PACKAGE (I think this is "direct"?) and you maintain $DEPENDENCY (I think this is "maintainer"), which I depend on. Let's also imagine that $POPULAR_PACKAGE (which is "indirect") depends on my $PACKAGE.

If someone reports a vulnerability on #DEPENDENCY, you are definitionally impacted by this, so impactful=true by default (though this probably brings up the "is a boolean the right choice" question), thus you won't really be using this tool at all (which I think is what you mentioned). It's now up to me, the maintainer of $PACKAGE to determine if it's impactful in my use case (which is what I meant by package maintainers of direct dependencies, apologies). So maintainers of directs would be the ones who set the flag, and consumers of dependencies where impactful=true would be the ones consuming it.

[bnb]

Just for clarification:

The goal is to get maintainers (in a previous comment, group 1) to run this command to provide a signal to their users about whether or not a (transitive) dependency is impactful to a given module.

[thescientist13]

Sorry, yeah, those labels weren't super explicit but I think we're mostly still talking about the same thing: some package (1) <- some meta framework (2) <- user of said meta framework (3)

But I think @bnb 's comment helps some I think

The goal is to get maintainers (in a previous comment, group 1) to run this command to provide a signal to their users about whether or not a (transitive) dependency is impactful to a given module.

Would be curious to know how this command gets scoped so that only that particular maintainer (user #2 in my fictional scenario) is indeed them and they are in the "right" to signal this claim.

[wesleytodd]

I am extremely against a system like this. It creates a weird caste system that's incredibly easy to game.

I think this already exists and is made up of the module authors already. This is also the same group @ljharb proposes to get the "early access" so I am not sure it this holds up. I totally understand where you are coming from and agree that a caste system is not what we want.

Maybe a middle ground would be a grace period after the cve is registered in the database where it is only reported on if a flag is added? Even a 1 day grace here could do a lot to avoid the storm that can happen when a cve report goes live.

[wesleytodd]

After thinking about this more and reading all of these comments I think that the npm RFC portion of this work should a client built on top of some other systems. That said, if we were to choose a shorter term approach of proving out a method we plan to up level to a more industry applicable solution that would be fine to. I still think that we would want to design the short term solution in a way to actually prove out the larger concerns though, and in that regard this RFC is not yet all we need.

So, to satisfy that I think we need a proposal for an agnostic api for a "source of trust" provider. This api should be simple to implement and scalable (think like how the plain documents work to form the registry api's we use). Additionally it should specify at least a starter schema for what a counter claim looks like. Once we have that, I think that an npm specific implementation were the source of trust could be a published package would be awesome.

Additionally, a command in the cli would act as a client for this api. For the filing of a counter claim we would want the interface to provide all the necessary schema parts, but other than that I imagine we could leave the handling of that up to the implementers. In this way, we would get npm native control over how it manages it's own default trust but also enable third parties and enterprise use cases to have a clear path to implement their own counter claim ingestion model.

---

A deep dive would be awesome to sort this all out, unfortunately I think I will have a difficult time making next week. I am on vacation and will be in Yellowstone where from what I hear any connectivity is hard to come by. If that timeline is what folks want, I can write up my thoughts as best I can, but I was hoping we could have a few discussions in the collab space before going too far on solutions.

[bnb]

Would be curious to know how this command gets scoped so that only that particular maintainer (user #2 in my fictional scenario) is indeed them and they are in the "right" to signal this claim.

presumably if you can npm publish to a module (in the RFC, I use the example that I am theoretically a maintainer of fastify), these rights would be granted to you as well. The authentication mechanism would be the same: npm login (the real name is npm adduser but lol).

[thescientist13]

Would be curious to know how this command gets scoped so that only that particular maintainer (user #2 in my fictional scenario) is indeed them and they are in the "right" to signal this claim.

presumably if you can npm publish to a module (in the RFC, I use the example that I am theoretically a maintainer of fastify), these rights would be granted to you as well. The authentication mechanism would be the same: npm login (the real name is npm adduser but lol).

Gotcha that makes sense. So if npm audit submits to NPM, then we should be good to go then. Is npm where these reports would be submitted? Or would this local check just be a step 1 in the process?

---

Also, I think I understand a bit better the mechanism you are proposing now after your replies, so effectively from your example, running

$ npm audit assert --module=fastify --id=CVE-xxxx --impactful=<boolean>

The CVE in question then would be against one of fastify's dependencies (e.g. something from its package.json), not itself.

If so, I wonder if there's any additional context / heuristics that could be gleamed from looking at package.json or package-lock.json / yarn.lock to provide additional context at the time npm audit is run?

• is said vulnerability in dependencies / devDependencies / peerDependencies
• if not directly defined in a manifest then, "mark" it as a transitive dependency (of some sort of top level) and factor that in perhaps

Some of that info could be useful for helping others become more aware of in what specific way fastify (in this case) is using said dependency, and thus if it even falls into the scope of the CVE (or at least help paint a more accurate picture of usage). Maybe with some sort of confidence score, much like dependabot now when submitting a PR to a repo.

[iamwillbar]

I've been doing some work with the National Telecommunications and Information Administration (NTIA) around Software Bill of Materials, and they've also been working on some requirements for sharing vulnerability exploitability which aligns with some of the goals of this RFC.

They've been working with the CSAF 2.0 project in OASIS to contribute a VEX profile that allows you to communicate whether a particular vulnerability is exploitable in a particular software product.

Looking at the information they capture the only thing missing from this RFC would be the assertion specifying why the vulnerability can't be exploited: "SHALL contain a a description why the vulnerability cannot be exploited". So as @wesleytodd suggested above I recommend that this information be captured as a required field.

[ljharb]

@bnb i'm not sure what you mean. there's already such a system - dependency maintainers versus end users. This is both correct and necessary.

fwiw, CVE disclosure already works this way for the vulnerable package. I was only suggesting that this exclusivity be slightly loosened to include authors that depend on the vulnerable package.

[bnb]

fwiw, CVE disclosure already works this way for the vulnerable package.

yes, the vulnerable package maintainers need to know their package is vulnerable. Not everyone can sign up randomly to be a maintainer of that package and get early access to the information before it's publish, presumably?

I was only suggesting that this exclusivity be slightly loosened to include authors that depend on the vulnerable package.

how do you implement this in a way that isn't effectively the exact same as publishing publicly?

[bnb]

Looking at the information they capture the only thing missing from this RFC would be the assertion specifying why the vulnerability can't be exploited: "SHALL contain a a description why the vulnerability cannot be exploited". So as @wesleytodd suggested above I recommend that this information be captured as a required field.

@iamwillbar I've added --comment=<string> as a required flag. Glad to know that I've already gotten pretty far along what those other grpups are already working towards <3

[ljharb]

@bnb true enough! To be clear, I'm OK with the vuln being published - but my hope was that tooling would wait a period before surfacing this to users, so as to give transitive maintainers an opportunity to make an assertion before their consumers get the warnings.

[bnb]

@ljharb that makes sense, the concern I have there (as was the concern I had with a company that was doing this not with maintainers but with their paid users getting "early access") is that it will be trivially easy for a malicious party to get that early warning and then attack end-users when they're not aware there's a problem.

I don't see a way to achieve the stated goal - which I agree would be ideal, fwiw - without also building in a way for malicious parties to abuse it.

[ljharb]

I guess I'm not personally worried about that since in practice in the JS ecosystem, 9999 times out of 10000, the warned vuln ends up being a false positive.

[bnb]

All it takes is 1/10000 for a lot of pain and loss of trust across the entire ecosystem :P

[ljharb]

True - but that pain and loss of trust has already been happening from false positives, due to lacking a system like this RFC.

[bnb]

@thescientist13 realize I didn't answer your questions:

Gotcha that makes sense. So if npm audit submits to NPM, then we should be good to go then. Is npm where these reports would be submitted? Or would this local check just be a step 1 in the process?

Yes, they'd be reported to npm so when someone runs npm audit assertions could be taken into account.

The CVE in question then would be against one of fastify's dependencies (e.g. something from its package.json), not itself.

Yes, although presently it's structured to be used against npm advisories and not CVEs (advisories point to CVEs). I'm not opposed to CVEs (I'd actually prefer it), but the mechanism that presently exists is advisories and I wanted to make sure that we weren't introducing new concepts that were outside the scope of this feature.

The command is structured in such a way that's intentionally ambiguous and could use CVEs (or any other identifier that we'd potentially want to ship). I'd recommend this should be a follow-up RFC if people want it.

If so, I wonder if there's any additional context / heuristics that could be gleamed from looking at package.json or package-lock.json / yarn.lock to provide additional context at the time npm audit is run?

potentially, but I think this is probably outside the scope of this RFC.

[wesleytodd]

@iamwillbar This is the reason we created the Package Vulnerability Management & Reporting Collaboration Space. To better collaborate with folks working on things like you mention. Would you have time to chat about your work so we can see how best to fit it in here?

Not everyone can sign up randomly to be a maintainer of that package and get early access to the information before it's publish, presumably?

This is where I think sources of trust are important. I think that the npm registry, github and through them microsoft might be the default that ships with npm, but it is important that we build in easy ways to opt out of the microsoft ecosystem for trust.

I think it is reasonable for the registry to make decisions on what their source of counter claim trust has which might include popularity of packages on the registry or even just being an author of a package which depends on that package. Either way that is up to them to manage the rules around. The key point is making sure the implementation has a way to opt out of the default.

I don't see a way to achieve the stated goal - which I agree would be ideal, fwiw - without also building in a way for malicious parties to abuse it.

Perfection is great, but let's not let this get in the way of really good improvements. If a malicious actor proves they are malicious the source of trust can just remove their early access. If npm wants to take this on, they need policies to act as a moderator here.

[wesleytodd]

@iamwillbar I spent a bit reviewing the documents you linked. One thing I would recommend is providing some introductory material. From my perspective, this work needs a heavy developer experience focus, and unfortunately spec docs and white papers leave a lot to be desired when we try to translate this work to a broader audience. A specific example from the white paper linked, it seems to assume you know what specifically an SBOM is but provides no context for those who do not know.

Do you happen to have documents which introduce users to this work? Specifically things which go beyond the abstract sections from these documents would be great. I think both of these links assume some familiarity with prior work and might not speak to the audiences you need to reach to make this successful.

[bnb]

From my perspective, this work needs a heavy developer experience focus, and unfortunately spec docs and white papers leave a lot to be desired when we try to translate this work to a broader audience. A specific example from the white paper linked, it seems to assume you know what specifically an SBOM is but provides no context for those who do not know.

if we could keep review of external materials outside of this RFC, I'd appreciate it :)

[wesleytodd]

Sorry, my understanding is that the work was related but it is hard to tell from the links. Is there a way which that groups has spec'd out a counter claims format? That would be applicable here IMO.

[pimterry]

Thanks for creating this. As a package maintainer, package consumer, and occasional security researcher, I think this is an excellent improvement.

That said, there's an interesting dangerous case that I think the current spec here allows:

• I maintain package A which depends on package B
• Package B has a vulnerability, I assert that it doesn't affect package A, everybody's happy
• Later, I add a dependency on package C, which depends on package B. Package C is very vulnerable, my usage of it is vulnerable, and this is generally a bad thing.
• Nobody maintaining or using package A is ever aware that there's now a vulnerability, because I've previous ruled out the vulnerability completely, so everything hides it for me.

To be clear, that's this dependency graph:

 A──────────>B
 │           ^
 └────>C─────┘

It seems like this won't be super common, but I think it will definitely happen occasionally, especially for issues in widely used base libraries like lodash. If this does happen, the impact (actively hiding a real vulnerability) is pretty bad.

I think the fundamental problem is that an assertion by a maintainer of A is really an assertion that their current paths to a vulnerability are safe - not that all possible paths are safe forever.

If we care about this, we could:

• Require assertions to specify which direct dependency/dependencies they're talking about ("this package is not affected to vulnerability X from direct dependency B")
• Or: automatically track the paths to the vulnerability that are present at assert-time, to do the same thing but automatically for every current path.
• Or: limit assertions to current versions, so a new explicit assertion that vulnerabilities still don't exist is required for each future version. More generally safe, protects against smaller (imo) concerns like changing use of an existing dependency too in a way that creates vulnerabilities, but super annoying for maintainers. Could have a default range that does this (<=$current_version) which determined maintainers can override it with a warning if they're sure? Still pretty annoying, but also very safe.
• Or: tools need to detect this at the time when you install and/or publish with the new dependency ("You're about to publish this package with a new dependency that has a previously excluded vulnerability - are you sure that's safe?"). This is a bit tricky to do well imo but it's not impossible.

[wesleytodd]

I think the fundamental problem is that an assertion by a maintainer of A is really an assertion that their current paths to a vulnerability are safe - not that all possible paths are safe forever.

@pimterry Yes, this problem has many permutations (here is one even more simple example). Additionally, a package maintainer could make a change to their usage of a library, thus invalidating the original counter claim.

The goal of this work is not to be perfect, but to incrementally improve the security posture. So I think that spec'ing out all the metadata we think will be necessary to provide a robust counter claim system is only the first part. The second is making sure that the human systems are in place and align well with the needs of maintaining this. So here are my thoughts on each of your recommendations:

Require assertions to specify which direct dependency/dependencies they're talking about

This is both a part of the existing RFC, but also 100% required. Additionally we need the semver spec it depends on and what type of dep it is.

automatically track the paths to the vulnerability that are present at assert-time

This is the same as number 1, but I think you mean in an end application, which is not something trackable at the registry layer. What I think you mean is that when reporting, we should only apply the counter claim for the path which it applies. This is the example I mentioned above if express -> path-to-regexp and react-router -> path-to-regepx exist in the same tree, but a counter claim only exists from express. This would mean that the path-to-regexp CVE would still be reported on.

limit assertions to current versions

I think this could produce a huge burden on maintainers. Since the goal here is to maximize DX I am not a fan of this idea. I think it is a satisfactory middle ground to let maintainers manage the claim with a semver range.

tools need to detect this at the time when you install and/or publish with the new dependency

I think the goal here would be to make npm audit actually reliable enough that we can convince maintainers to add it to CI again. Right now it is SOOO noisy that most dont. If we can smooth out the DX here we can start working with maintainers to do this and get this without feature work on npm's side.