-
Notifications
You must be signed in to change notification settings - Fork 145
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
Suggestion: Provide standards around integrity between source code and published package #77
Comments
npm doesn’t require a repo, and any package with a prepublish build process (read: every Babel user) will correctly have the published package not matching any sha in their git repo. I’m not sure how we’d be able to do any sort of verification in a consistent and automated way. |
The only way to guarantee this is to compute an hash of all the content of a module (in a. what are the keys that are allowed to sign a given package People have been asking for a similar feature to npm for the last 2-3 years. |
Thanks @ljharb @mcollina and sorry for the late reply. This article by @skonves gave me a nice reminder of this topic. And it seems like there are tools in his tbv and another in npm-verified that attempt to do similar. There would need to be a blessed, yet optional process to add some sort of |
My 1 cent out of 2 (don't have time for more): npm doesn't require a repo, but a package verification service on top of npm might require whatever is needed, and package authors that want to conform and get the "verified" badge will try and make sure their package is good. For example there are similar efforts to standardize open source repositories: https://github.com/todogroup/repolinter — there could be tools that help package authors to manage packages efficiently (I personally miss the kind of tool that would automatically set up CI and releases for me reproducibly and reliably). |
Indeed, that’s something npm can solve - but i don’t think it’s something node, and thus we, can. |
Chiming in here, we really need something like this... ZijianHe/koa-router@bd780c9#diff-04c6e90faac2675aa89e2176d2eec7d8R3 Screenshot (in case commit is force deleted): I've version locked |
@niftylettuce i'd suggest reporting that to npm; i doubt their TOS permits the sale of a package name. |
@niftylettuce You did the right thing here even if nothing negative comes of it. Its a red flag for sure and others deserve to be alerted by this information. Good job on being alert. |
To anyone reading this, I'm building a tool to automate this nonsense, at least until Node/NPM do something about it. Email me at niftylettuce@gmail.com if you want to get notified once it's up. I'll notify everyone that posted in this thread and/or left reactions as well. It will be free and open-source. |
Maybe it's time to think to an NPM alternative. |
What's more, even if npm validated against the git repo at publish time, the user can just force push after publication. Checking this kind of thing does nothing. If you want to validate at install time, well, I hope you enjoy your multi-hour install times. =p (Seriously: In modern npm or yarn, install time per package is under 10ms—adding a git clone to that mix would massively increase overall run time.) This kind of action gets you no security whatsoever. It solves no actual problem. It validates nothing. |
@iarna what do you think about a new command for npm, I've been thinking about a This would effectively allow you to review the code of new/updated packages, similar to how adding |
@niftylettuce Hey, if you're going to work on a solution, it would be way better to start a repo on github, so people can contribute ideas/feedback via issues |
@freewil That seems much more useful, but I am concerned how you scale that out to the thousands of deps in a typical modern deployment. 'cause yeah, the one thing you asked for may have a reasonable diff, but what about the dozen transitive deps that also updated? Still, I think this would be an excellent place to begin experimenting with, to see how it feels ( |
@6a68 yes it will be on GitHub, I will post the link here once I have a proof of concept up |
To anyone reading this - please do not harass, email, or contact the original maintainer of the package mentioned in the above discussion. It was never my intention for anyone to harass them. I simply wanted to raise awareness about this issue with NPM and the potential of this becoming a security issue in general. This is not the only package like this. |
Another update on the koa-router issue for anyone subscribed to this thread. The new maintainer @ZijianHe has provided us with an update ZijianHe/koa-router#494 (comment). Hopefully this eases concern and it looks like we have a new contributor in open source land. For the CLI |
See nodejs/package-maintenance#77 (comment) Might be that nothing malicious is done with the package after a transfer but let's rather be safe than sorry.
Maybe the best "way" to handle the change of a maintainer is that the new maintainer should open its own repository on NPM. They should not allow a "takeover" procedure: instead, the installation may fails with a message like "hi, this library is not maintained anymore by |
I think the current way npm does this is by forcing a bump in a major release. That's enough to protect users form a malicious "takeover" in the case of a non-responsive maintainer. |
Is that so? Any way to confirm? |
I've done this several time, and that's how it works. I looked in https://www.npmjs.com/policies/disputes but there is no mention about that. |
They certainly don’t; and can’t, because a legitimate new maintainer should be able to backport fixes as needed anyways. Any owner can always and forever publish to any previously unused version number, and that’s how it must stay. |
I assume you assume the evil user force pushes to remove malicious code? If such force push leads to file changes, git commit ids will change. Npm would record the commit id it verified against, and a standalone checksum of published files. If there's no such commit in the repo, it's a red flag. If running the same checksum at the same commit id files results in a different checksum, it's a red flag. I believe this verification has to be a background task in npm; if one of the red flags has triggered, this package downloads are paused to prevent further spread until they are resolved. There has to be cache invalidation mechanism that would notify downstream caches to not use the flagged package. I'm in no way a security expert though, just brainstorming. What do the npm security people that npm aquihired say? |
The way I experimented in npm-verified POC is I needed a standardized package build+publish process (I used npm prepare for that). The packages that conform to the process can be marked as verified if the checks pass. The packages that do not — never. Like in Chrome, the sites that do not use HTTPS are marked as "Not Secure" by default, and even more, colored in red if they collect some user input via a form (for npm, this can be translated to using some OS APIs like process, network, and filesystem). |
Yeah, I think that would be one version/use-case of the command. The way I'm currently thinking, it'd be nice to also have a no argument version similar to |
One alternative to prevent the need for tracking changes/state would be just to add a |
lockfiles are a very good idea for applications, which is the high majority of users. However, they are not really the target of this initiative. |
@mcollina this is not about lockfiles. I don't think they make sense as part of the package or part of the repo. They do make sense as a snapshot in time to achieve reproducible builds. Once again - they should not be published on git or npm - only kept around as a convenient metadata format. |
@dominykas That might be reasonable, however - a sort of |
@ljharb that is also an option, but it depends on the toolchain. I've literally spent this week setting up semantic-release, because we have locked masters and therefore can't have version commits. This also means that we need to push release meta data outside of git repo. We also have a pattern where we push shrinkwraps into dangling tags (i.e. a I wish npm had a way to store this, and some other, release meta data, but not necessarily distribute it with the package. |
The ownership of the koa-router package was transferred to an "unknown" user. See more on Github: nodejs/package-maintenance#77, Hacker News: https://news.ycombinator.com/item?id=19156707 and the npm blog: https://blog.npmjs.org/post/182828408610/the-security-risks-of-changing-package-owners
A possible solution to this problem (integrity between source code and published package) is to provide a "builder" service that take the source code, builds it, then publishes the output. ZEIT Now is probably 80% of the way there because builds are immutable, every build has a unique URL, the source code is captured during the build process, and you don't need a repo to publish. But if you do have a repo, you can connect it to the build system so that pushing to master (or any branch) automatically publishes. There's a couple changes that would need to be implemented in npm:
|
I was initially thinking the same thing. But what stops someone from having a build task that inject malicious code into the "built package"? Wouldn't a per module sandbox / permission system to protect against certain actions (Network, Disk Access, etc) help? |
I was also thinking this. Basically the idea is to have a trusted third-party do the packaging from source. Docker Hub might be a good place to look for inspiration. With Docker, it can be an even bigger issue, as docker images are built into binary files and pushed up to a public repository. Although, Dockerfiles/images are generally much more simple. |
There shouldn't be any user-provided build task code while using one of the trusted builders, only a build pipeline configuration, for example, like https://github.com/pikapkg/pack does.
Yes, that, too, may increase confidence, and complexity. That is needed, too, but is orthogonal to this particular discussion about package build-from-source integrity. |
The build process itself needs to be able to contain user-written transforms, if it's babel, or a bundler, to support common and valid use cases. |
The ownership of the koa-router package was transferred to an "unknown" user. See more on Github: nodejs/package-maintenance#77, Hacker News: https://news.ycombinator.com/item?id=19156707 and the npm blog: https://blog.npmjs.org/post/182828408610/the-security-risks-of-changing-package-owners
It doesn't if all the common and valid use cases like babel and bundler are standardized into trusted builders. I guess there should be either no way, or a hard and impractical way, to tamper with the executable content (e.g. JavaScript code) of a trusted build by providing a malicious build step. But again, I'm not a security expert. I pinged the Node.js SecurityWG to take a look at this discussion as I think it is rather opitionated, not engineered as a document with clear sets of requirements, tradeoffs and solutions. |
i think you missed a very important detail in the point you quoted:
part of the valid use of babel is to apply custom transformations beyond those provided by babel itself or even official plugins. these are often included as |
I'd like to see some data about how popular these are, and whether they can be standardized. They can be published as a trusted package that itself does not use them, thus building a chain of trust. I hear you, but there's always a tradeoff between security and convenience. |
@sompylasar packages are all already implicitly trusted; the attack vector i thought you're trying to address here is when a trusted package is hijacked, for which the same risks exist for any package, including ones in the build system. |
Yeah I guess you're right. Well that's what I mentioned above. We all would benefit from a document (like an RFC) with the list of potential attack vectors, and how they are addressed or can be. |
I have actually already put together a draft of a graph of the NPM ecosystem for just that purpose. Each edge and vertex definitely has its own unique set of challenges. For the past few evenings, I have been working on a writeup, but I wouldn't mind opening it up for community feedback/contribution. Here is the draft of the graph I have so far: I have a draft on Medium right now, but I will move that to a more public place in a heartbeat if it means more eyes on it. Thoughts? FWIW, my work with TBV/verifynpm and @sompylasar's similar work with npmverified seems reasonably confined to the "publishes to" edge near the top; however, I strongly agree with Ivan that understanding the full picture helps inform any proposed solution (particularly what is in and out of scope). |
I saw an interesting post about this idea from the NPM perspective not too long ago: https://hackernoon.com/npm-package-permissions-an-idea-441a02902d9b |
FWIW, we built Package Diff to gain visibility into npm package updates by directly examining the package instead of looking at the potentially-misleading source code repository: |
FWIW, I built package diff for the purpose of comparing next prepared version with the latest published one for the review and version bump purposes https://github.com/sompylasar/zmey-gorynych/blob/9029972/bin/cli.js#L512 |
@tlhunter very nice. I had one thought, one way to get around it. What if you were using |
For the system to be perfect we'd need to throw away the concept of npm install scripts. One could perform an npm install of every package ever made, on a set schedule, and constantly diff the artifacts left behind, but even that wouldn't be perfect. Like, the fetching of pre-compiled binaries might include information about the host machine. The event-stream incident, for example, would only inject malicious code if certain criteria were met. I'd like to see some sort of package manager service which does the following:
|
Yeah, I agree that sounds like the only way. I could imagine npm flagging releases that were published that way and you could then opt to only accept verified dependencies if you wanted. It would be tricky for pre-compiled binary modules as they could have a large build matrix for platforms and versions but it would be enormously helpful for confidence and security. |
Interesting and related proposal in the golang ecosystem: |
Right now, it seems that most maintainers may publish their packages from their local environment. There should be a way to verify what is published against the public source code or specific git sha to maintain transparency of what is being published. Not only will this mitigate out of sync issues or accidents, but will provide greater confidence that additions aren't added as they are published (potentially malicious).
Not sure if this is the best place for this, but after reading through other issues and recent resources I thought I better put this down somewhere. And it brings up the discussion of maintainers permissions to not only package registry, but SCM as well.
The text was updated successfully, but these errors were encountered: