Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal to provide package safety check #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

viztor
Copy link

@viztor viztor commented Aug 14, 2018

No description provided.


The design is pretty straightforward:

Any network access would end up in requiring certain node modules or using certain browser api, therefore, it should not be difficult to do static analyses of those packages and provide basic safety check as for whether those packages requires those modules and/or calls those globals.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is in fact impossible to statically determine anything like that with certainty in JavaScript or in node.

Copy link
Author

@viztor viztor Aug 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. The purpose here isn't to build an anti-virus software, but rather to check what we can detect. The reason I recommend static check at this stage is because the implementation cost would be relatively low, consider other measures taken by Chrome or macOS - Sandbox and such.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure the implementation cost would be that low. Also consider that such a measure would be pointless unless followed by the ecosystem. And the ecosystem would only follow it if it actually worked. So imo, any solution will have to be designed to be the silver bullet and solve all the use cases from the very beginning.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, any "check" against malice is either 100% effective, or it's useless.


The result of such analysis can and should be cached on server. // it can and probably should be signed by the server of the package hash, analysis result and version.

When a publisher tries to publish a package with `network capacity` which previously doesn't, they will be asked to re-authenticate with their credentials, or provide a 2FA code. // not sure if yarn can do this. maybe only npm?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would have to be on the registry side, and would be impossible to detect or prevent without runtime cooperation from both node and the OS.

@arcanis
Copy link
Member

arcanis commented Aug 14, 2018

Yeah, static analysis isn't doable in Javascript (unless we talk about a super strict subset of JS), and would be kinda out of scope for Yarn.

@ljharb Btw, has the Node project ever considered working on a secure sandbox model for Node? More things would become possible, like restricting requires for some modules in such a way that they wouldn't be able to access fs, http(s), or any other builtin that would expose the system. It could be opt-in based on settings in the package.json (with the understanding that fs access would basically be "all permissions", of course).

@ljharb
Copy link

ljharb commented Aug 14, 2018

There’s an open PR exploring it now, i believe.

@viztor
Copy link
Author

viztor commented Aug 14, 2018

Much simpler proposal would be:
It appears one of the reason why eslint-scope created issues is because currently there doesn't seems to be a way to disable a package, and many package.json does not pin minor version. Would it be possible to maintain a public list of compromised packages, so people would be warned if they try to upgrade to that version? It could be an opt-in. or allow people to supply such list created by their team, or within company

This doesn't make any decision on how safefy-check are done, but simply a way to mark them unsafe to use in a team, organization, company, or globally if the user choose to use the global list.

@arcanis
Copy link
Member

arcanis commented Aug 14, 2018

There’s an open PR exploring it now, i believe.

Seems to be nodejs/node#22112. Nice! I wonder if it could be integrated with Yarn so that we could print a "are-you-sure" prompt at install time?

It appears one of the reason why eslint-scope created issues is because currently there doesn't seems to be a way to disable a package, and many package.json does not pin minor version

I'm not sure it was the issue - the compromised package was swiftly removed from the npm registry, so it became unavailable as soon as it was detected. A blacklist wouldn't have been much faster.

I suppose such a feature wouldn't hurt (I heard it being suggested a few times already), the main issue I have with it is that it would make yet another thing to maintain.

@viztor
Copy link
Author

viztor commented Aug 14, 2018

@arcanis I assumed so as it appears there are pull-requests to many libraries to revert the update of eslint-scope. I wasn't exactly sure of the timing by reading the post-mortem published by eslint team.

Combining those thoughts, it strikes me that instead of a blacklist, can we have some sort of whitelist? like a for signatures on packages?
For example, maintainers can sign package hash with their GPG key, and post it to the list.
and people can create bots to sign those packages too.

The benefit of such approach is that Instead of passive response towards incidents, the process requires active signing. And it would enable customized policies for project owner, say the project owner can configure that a package release will only be available to public if they pass certain bot checks i.e. receiving signatures from those bots/or just certain human. And company can enforce policy where a package must have the signatures of their security team or certain someones to be used with the internal packages.

This can be optionally configured at the cli end. It can also be configured to rely on multiple signatures. it might happen that one person's key get compromised and package get published, and is much more unlikely that multiple person's key got compromised at the same time.

It has more advantages over a blacklist as a blacklist must be maintained by someone/or team, and the proposed network of signature can be maintained distributed.

Wonder your thoughts on this.

:)

@ljharb
Copy link

ljharb commented Aug 15, 2018

That kind of whitelist would stifle growth of the ecosystem and enormously privilege existing maintainers; that imo is also a nonstarter.

@alloy
Copy link

alloy commented Oct 3, 2018

My 2 cents.

Having experimented with utilising macOS’ sandbox facility to sandbox CocoaPods installations, my takeaways are that it is hard to get right and moreover many ideas about securing the installation phase are alas futile.

I may be overlooking something unique to npm/yarn, but as long as you eventually end-up executing the code un-sandboxed then there’s really no point in securing the installation too much. Focussing only on the installation phase is unfortunately mostly security theatrics.

A holistic proposal such as nodejs/node#22112 makes a ton of sense to me, as long as it applies to both installation and execution phases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants