Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

RC npm: recommend committing a secure .npmrc file? #20

Open
lirantal opened this issue May 29, 2022 · 22 comments
Open

RC npm: recommend committing a secure .npmrc file? #20

lirantal opened this issue May 29, 2022 · 22 comments

Comments

@lirantal
Copy link
Contributor

Some of the best practices that have been discussed here, or featured in the document, can be enforced using a common configuration file (.npmrc) as part of the files in source control. The upside in having this committed is that when devs clone and run the repo, they are secure by default, rather than rely on pre-configuration steps.

One clear example is to enforce the --ignore-scripts option:

ignore-scripts = true

# another example for uplifting other _secure by default_  config settings
strict-ssl = true
@ljharb
Copy link
Member

ljharb commented May 29, 2022

Isn’t strict-ssl the default already?

I still don’t think ignore-scripts is a reasonable thing to recommend because of all the caveats.

@lirantal
Copy link
Contributor Author

Isn’t strict-ssl the default already?

Ah it is.

@laurentsimon
Copy link
Contributor

laurentsimon commented Jul 14, 2022

We do suggest using an .npmrc file in the "Private packages" section. Is there something we should add?
We're discussing ignore-scripts in #15

@lirantal
Copy link
Contributor Author

What I'm referring to here is not only suggesting having the file, but rather suggesting specific defaults to the file that would help for a "secure by default". At the very least I will advocate for ignoring scripts. I do wonder if there are others that we should add.

@laurentsimon
Copy link
Contributor

+1 on finding out what other useful config we could recommend.

@ljharb
Copy link
Member

ljharb commented Jul 15, 2022

I think that until npm has solved the problem by providing alternative methods for packages to achieve the use cases that currently require scripts, it would be far too disruptive to advocate ignoring scripts by default.

I'd be interested to hear what other config might improve safety - and since npm v9 is coming, i could advocate for its default to change.

@lirantal
Copy link
Contributor Author

lirantal commented Jul 16, 2022

Reading up on academic research is something I do regularly, and reading up on Wolf at the Door: Preventing Install-Time Attacks in npm with Latch they mention:

At the time of our analysis, we found 1,493,231 distinct packages in npm, 36,438 of which contain an install script.

which isn't a very high number and I'd like to see further evidence and data-based information to make that decision and would like to encourage us to create secure defaults. I understand this might be a breaking change for some, but we're not advocating here for npm to enable that by default as the next version (although I'm happy to discuss that too) but rather about the best practice we want to advise users to follow, and I believe that not running some arbitrary code by strangers on the Internet is a good one.

@ljharb
Copy link
Member

ljharb commented Jul 16, 2022

If npm isn’t willing to make the breaking change (they’ve previously said they are not) then i don’t see why OpenSSF would feel confident enough to recommend it.

@lirantal
Copy link
Contributor Author

I would think we don't want to make such directional connections between community-lead efforts and vendors?

Case study is the new Buffer related security issue which has been used through-out many ecosystem packages in their codebases but the Node.js project couldn't just deprecate that completely as that would be an API breaking change and so the project had steered off to an education and awareness efforts to work around it and help maintainers migrate away from that (reference: https://nodejs.org/en/docs/guides/buffer-constructor-deprecation)

Regardless, I've yet seen any hard data that says a large part of projects for users would just break if they do a --ignore-scripts. Let's look at that data first and take decisions from there?

@ljharb
Copy link
Member

ljharb commented Jul 16, 2022

Right - but before deprecating it, node provided alternatives to recommend. npm does not yet provide those alternatives (although it is working on it).

Totally fine to look at the data, but even without it, deprecating something without pointing to an alternative does not seem like it will help things.

@laurentsimon
Copy link
Contributor

laurentsimon commented Jul 18, 2022

Reading up on academic research is something I do regularly, and reading up on Wolf at the Door: Preventing Install-Time Attacks in npm with Latch they mention:

Thanks for sharing.

At the time of our analysis, we found 1,493,231 distinct packages in npm, 36,438 of which contain an install script.

which isn't a very high number and I'd like to see further evidence and data-based information to make that decision and would like to encourage us to create secure defaults.

The numbers look small, but keep in mind that we're looking at graphs, so it suffices for a single node to use install scripts for the installation to break. This is explicitly called out in the paper this number understates the likelihood of an install script being encountered during npm package installation, since the scripts of all dependencies will be invoked.

I think this is the premise for their proposal around a policy language, in order to provide more granularity instead of unilaterally blocking the scripts.

As a thought experiment, let's say that a medium-sized project contains on average 50 dependencies. Assuming dependencies are uniformly popular / likely to be used; and since each package has a 2% likelihood of using a script (numbers from paper), this means that the project will on average have 1 dependency that uses an install script, and will fail in practice. The probability goes up as you have more dependencies in a project. So there is a non-negligible probability a project would fail, which makes it hard for us to recommend using it.

@elizabethwyss @drewjdavidson as the authors of the paper, could you share your perspective?

I understand this might be a breaking change for some, but we're not advocating here for npm to enable that by default as the next version (although I'm happy to discuss that too) but rather about the best practice we want to advise users to follow, and I believe that not running some arbitrary code by strangers on the Internet is a good one.

Agreed. Is there a reason for treating the install script differently from the main node code in the project? They are developed by the same maintainer, so trust in the code should be the same as trust in the manifest file. Is the argument that the risk is higher because one need not run the project, just install it?

@ljharb
Copy link
Member

ljharb commented Jul 18, 2022

I don't think there should be much of a difference there - code that's installed will inevitably be run.

@ruyadorno
Copy link

@lirantal another caveat of --ignore-scripts in its current form in npm is the fact that it will still run scripts for git (and direct tarball urls iirc) deps, ref: https://github.com/npm/pacote/blob/87d4b4612b73b5d0b27852bf3f23533d8de7c7ed/lib/git.js#L160

With that in mind I wouldn't recommend blindly using that option as a security improvement at least not until the Only Registry Tarballs RFC lands, once both these options can be used together then it may become a way to ensure no install scripts are going to run.

I believe a good way to help move the needle to a safer place for the entire ecosystem is to help move forward the different features that fills the gap that install scripts are currently fulfilling (such as the Package Distributions RFC in order to have a reliable way to distribute platform-specific binaries). Making it more possible for the npm cli client to deprecate install scripts at some point.

@lirantal
Copy link
Contributor Author

I don't think there should be much of a difference there - code that's installed will inevitably be run.

Agree with @ljharb that if someone is able to trickle in a malicious dependency or malicious code changes, there are run-time attack vectors that can happen that are outside of the install process hooks. That is, given that dependency has a legitimate code path leading to it being used indeed.

@ruyadorno thanks for that observation, I wasn't actually aware of this caveat :(
It makes an even greater case for lockfile-lint usage with a trust policy to be used in conjunction with the --ignore-script as a security control.

@jeffmendoza
Copy link
Member

It seems like there is hopefully some development coming in this space, but currently --ignore-scripts cannot be recommended to be used generally. It looks like we will need to revisit this in a future iteration of this guide to see if things have changed. With that, I'll close this issue for now.

Also, back to the original comment: a recommended .npmrc file would make sense for the guide, but looks like we don't have anything to include currently.

@naugtur
Copy link

naugtur commented Sep 2, 2022

I still don’t think ignore-scripts is a reasonable thing to recommend because of all the caveats.

Caveats have been eliminated by @lavamoat/allow-scripts

It covers:

  • allowing selected scripts
  • setting up the repository and adding the flag
  • avoiding running an install script from a package that spoofed its name (eg. A bundled package that calls itself bcrypt won't get run if you allowed the real bcrypt)
  • stopping the installation early if the ignore-scripts flag was somehow accidentally removed or forgotten

Are there any caveats left?

There is no other way to defend against well crafted malicious packages using this technique.

https://m.youtube.com/watch?v=y2p1e7UmGYY

@ljharb
Copy link
Member

ljharb commented Sep 2, 2022

It’s a userland package, not something built in to npm, and one that does not have sufficient userland adoption.

@laurentsimon
Copy link
Contributor

Re-opening for visibility

@laurentsimon laurentsimon reopened this Sep 2, 2022
@ljharb
Copy link
Member

ljharb commented Sep 2, 2022

Moreover I'd say that if a viable technique exists, please file an npm rfc - once npm has such a feature, the recommendation will definitely become "use it".

If the RFC lands and is shippable quickly enough, it might even be possible to make it the default behavior in v9, requiring no user action at all.

@naugtur
Copy link

naugtur commented Sep 3, 2022

I tried the RFC process on and off for 6 years and didn't succeed much. I'll try again with this and see what happens.

I don't really get the argument about it not being acceptable because it's userland. Is deps.dev not userland? This recommendation is the first time I've ever heard of it.

allow-scripts solves the most severe danger we're now facing and making it a recommendation vs adoption is a chicken and egg problem.

I'll create the RFC but what about yarn users, are they not supposed to benefit from this set of recommendations?
allow-scripts works with both npm and yarn regardless of version.

The argument for not bringing up defences against malicious install scripts was there's no solution that doesn't break or have weird caveats.
Here's the solution. Let's tell people they don't need to risk their files deleted, AWS keys stolen and wallets compromised.

@ljharb
Copy link
Member

ljharb commented Sep 3, 2022

I feel like the npm RFC process hasn’t existed for that long - 2-3 years at most, i think?

It’s not accessible because you’d need to install something else first to be able to use it - it’s a chicken and egg problem.

Claiming this is the “most severe” danger is a bit alarmist, i think.

yarn (and pnpm) builds whatever npm does, or else they lose users - they’re welcome to participate in npm’s rfc process as well.

I definitely think that it’s valuable to have a userland solution (i haven’t evaluated this one personally yet), but a best practice is documenting things that are already done widely - the purpose of best practices isn’t to convince the entire ecosystem to do something.

@naugtur
Copy link

naugtur commented Sep 3, 2022

npm/rfcs#18 2018 altho I started it before the official RFC process on a forum and got auto banned for too much activity 😅 so a bit under 5y really

Anyway. I'll try and see where it gets us.

I could go into best practices vs common practices but that's not what I came here to do.

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

No branches or pull requests

6 participants