Migrate NSP Analyzer to Node Audit Analyzer

[stevespringett]

According to https://blog.npmjs.org/post/175511531085/the-node-security-platform-service-is-shutting, NSP will be shutting down September 30, 2018.

What we know:

• Node Security Platform will be shutting down on September 30
• NPM AUDIT, the replacement for NSP CHECK, is available in NPM v6.0 and higher
• The current stable Node.js distribution still ships with NPM v5.6
• NPM AUDIT (as of v6.1 - current release) still relies heavily on Node Security Platform

After investigating the NPM AUDIT API, it is safe to assume that:

• Dependency-Check can safely migrate from using the NSP API to the NPM AUDIT API.
• The NPM AUDIT API provides nearly identical information about the advisories discovered from the package submitted.
• Vulnerability identification should continue to work as before

For organizations that rely on stable Node.js distributions, using Dependency-Check for vulnerability identification will be the only alternative.

Related: DependencyTrack/dependency-track#173

[stevespringett]

This will likely need to be a phased migration as the current Node Audit API still has dependencies on Node Security Platform resources.

1. Migrate the nsp check API to use thenpm audit API. This will result in NSP advisories with links back to nodesecurity.io.
2. If/when nodesecurity.io is shut down, migrate the links to the advisories to their new home (they currently do not exist outside of nodesecurity.io)
3. Change the SOURCE from being NSP to NPMJS

[stevespringett]

Dynamically creating package.json from fragments will no longer work.

The Audit API requires a valid package-lock.json to be submitted. The POST body can either be plaintext json or a gziped version of the lockfile.

https://docs.npmjs.com/cli/audit
https://docs.npmjs.com/files/package-locks
https://docs.npmjs.com/files/package-lock.json

The lockfile is partially scrubbed and the following fragment is added to it:

"metadata": {
    "npm_version": "6.1.0",
    "node_version": "v10.5.0",
    "platform": "linux"
  }

In addition, the requires field in package-lock.json (a boolean) is replaced by an object containing, what appears to be, the dependencies defined in package.json. Overall, there seems to be a bit of over-engineering and dramatically increased complexity compared to the existing NSP check API.

Documentation for the Audit API does not exist yet. https://github.com/npm/registry/issues/355

The response from the Audit API contains

• actions (we can ignore these)
• advisories (nearly identical to what we get from NSP today)
• muted (no idea what this is)
• metadata
  • vulnerabilities (the total # of critical, high, medium, low, info advisories)

[stevespringett]

This enhancement may need to come sooner rather than later. Although NSP is being shutdown in September, it appears that NSP is not being kept up-to-date.

For example, a malicious version of eslint-scope is able to be found using npm audit but not nsp check.

https://nodesecurity.io/advisories/673

[pioto]

When handling this migration, will it be possible to support suppression of specific advisories (a-la #892)?

[stevespringett]

@pioto The migration will simply be to swap out nsp check for npm audit API compatibility so that Dependency-Check users can continue scanning Node projects without interruption.

#892 requires modifications to the suppression schema for it to support 'source' (as in the source of vulnerability intelligence / NSP, NVD, etc). It will also require us to go back to all previous core suppressions and modify those to support source as well as provide backward compatibility for suppressions that do not specify a source. The Javascript to generate suppressions snippets from HTML reports will also need to be updated to support source. So #892 is a fairly big chunk of work by itself. PRs are always welcome ;-)

[stevespringett]

Migration Branch: https://github.com/jeremylong/DependencyCheck/tree/npmaudit

[stevespringett]

@jeremylong The migration is nearly complete.

I've run into a problem though. The Node Audit analyzer uses package-lock.json, whereas the old NSP Analyzer used package.json. Code in the NodePackageAnalyzer is removing the dependency from the engine so the Node Audit analyzer is never called. If I comment out the following line, everything works as expected.

https://github.com/jeremylong/DependencyCheck/blob/master/core/src/main/java/org/owasp/dependencycheck/analyzer/NodePackageAnalyzer.java#L169

So basically, we have two analyzers now, that both use the same file filters, and the Node Package analyzer executes before the Node Audit analyzer does thus causing the issue. Not sure how you want to handle that.

Other than that, I believe the migration is complete. I've tested it, it works.

One related issue though. NPM does not provide CVSS scores or vectors, so all the severities are 'unscored'. The RetireJS and the Node Audit analyzer both set the unscored severity, but there's nothing in the reports to account for that. This leads to all NPM findings being low severity for example because it's CVSS score is 0. The reports may need some refatoring.... Or we could make a convenience method in the Vulnerability class to handle that logic for us so the reports could be simplified.

[jeremylong]

Update the check to see if we should delete the file to check if the NspAnalyzer is enabled and then only delete the package-lock.json if the NSP analyzer is disabled.

    private boolean isNspEnabled(Engine engine) {
        for (Analyzer a : engine.getAnalyzers()) {
            if (a instanceof NspAnalyzer) {
                return a.isEnabled();
            }
        }
        return false;
    }
    
    @Override
    protected void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException {
        if (isNspEnabled(engine) && !PACKAGE_LOCK_JSON.equals(dependency.getFileName())) {
            engine.removeDependency(dependency);
        }

...

[stevespringett]

actually, its the opposite. I don't want to delete the dependency if the NspAnalyzer (now called NodeAuditAnalyzer) is enabled - it needs it. And if I reverse this logic, it causes all sorts of other problems with the NodePackageAnalyzer.

[jeremylong]

Steve - just updated the branch. Did that solve the problems with NodePackageAnalyzer you were seeing?

[Pablogra]

Hi.
I cant find information on an "Ignore list" feature in the documentation
https://docs.npmjs.com/cli/audit

This is a really important feature when running as part of a CI job and you cant upgrade all your packages to newer versions.

Is the documentation up to date or the feature still not implemented?

[stevespringett]

Documentation is up-to-date in the npmaudit branch, not the master branch.

npm audit has a lot of features outside of just vulnerability identification. And after chatting with their security team, more features are planned. The goal of the Dependency-Check NodeAuditAnalyzer is not to reproduce all the functionality of npm audit, but to identify vulnerable dependencies, which it does.

Pull requests for additional features (like scrubbing) that are outside the scope of the migration are welcome. Dependency-Check already has an 'excludes' feature, but I believe it only applies to files, not virtual dependencies.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.