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

Security Policies #110

Open
jonchurch opened this issue Mar 20, 2020 · 17 comments
Open

Security Policies #110

jonchurch opened this issue Mar 20, 2020 · 17 comments
Labels

Comments

@jonchurch
Copy link
Member

jonchurch commented Mar 20, 2020

After reading the Snyk Build a Backdoor article I started thinking about what the security policies and measures are in the project.

I'm curious to learn more about security practices as we are ramping up cadence of releases, merging more PRs, and getting ready to implement Repo Captains. As someone who has started reviewing PRs and committing to the project, I want to make sure I know how I can help keep the project and ecosystem safe.

Based on some quick searching through the main express repo and this one, I found a few things which I believe are part of the Express security model:

  • Limited commit bits/publish rights - Few people can publish or merge, mostly TC members I think
  • 2FA for committers and publishers - I don't actually know if/where this is stated, but I think that was a pre-req?
  • External dependency versions are pinned - To limit incidents based on permissive dep resolution (drawn from wes' comment)
  • Security Policy doc - Outlines the security disclosure policy

This is a topic that I personally am not well versed in, and I know we have some very smart people in this project and the Node.js project as well who care about Express and it's impact on the ecosystem.

This issue is meant as a means to facilitate discussion about what we currently do and what we would like to do in the future.

@jonchurch
Copy link
Member Author

Also, feel free to edit this top level comment to add to or alter that list. It would be nice to have a list as an easy reference throughout this conversation.

@dougwilson
Copy link
Contributor

Your bullet points pretty much sum it up, and especially heavy emphasis on dependency version management in these projects. I have a script I use that can also tell me for a given project exact which npm users have the ability to alter the package after publishing (i.e. reporting on the use of loose dependencies and how has publish rights to those).

The npm 2fa is desired, though afaik a package cannot force it; it is kind of up to the publishing user to actually have it turned on for publishing (not just logins).

All github orgs except expressjs require members to have 2fa on on their account; expressjs has users without it on and we may want to rectify that at some point and turn on the requirement.

@jonchurch
Copy link
Member Author

cc/ @nat I know you mentioned improvements to 2FA publishing on the npm announcement, and wanted to ping you here as an example of where the community can benefit from some synergy ❤️

Specifically from above:

I have a script I use that can also tell me for a given project exact which npm users have the ability to alter the package after publishing (i.e. reporting on the use of loose dependencies and how has publish rights to those).

The npm 2fa is desired, though afaik a package cannot force it; it is kind of up to the publishing user to actually have it turned on for publishing (not just logins).

@UlisesGascon
Copy link
Member

Thanks @jonchurch for open this discussion. I am the writer of the blog post and I will love to help with this topic. As Express is a key piece in the ecosystem we can ask for support from the security-wg, they are doing an awesome work (also a Bounty program). We can involve @lirantal too :-)

@dougwilson
Copy link
Contributor

Here is also a little write up I made towards one of our dependencies for those who are interested in the attack vector of gaining access to an npm account that can indirectly influence a large number of other node modules via indirect dependencies: debug-js/debug#688

@lirantal
Copy link

lirantal commented Mar 30, 2020

Hey folks, chiming in here after me and @jonchurch exchanged some DMs.
Great to see the security awareness for the project and the discussion going around!

@dougwilson

The npm 2fa is desired, though afaik a package cannot force it; it is kind of up to the publishing user to actually have it turned on for publishing (not just logins).
You can enable in npmjs.com that any publish would require the user to have 2fa enabled and force it for the package.

--

I think there are quite a lot we can do to upgrade the security posture of an open source project. Proper security hygiene like 2FA is a great start. I've worked with the Verdaccio team to also include pre-commit hooks tooling to prevent secrets from leaking to source control. Other ideas are tools like lockfile-lint which at at least for projects using package lock files can reveal weak links like a community user found out about a cypress nested dep.

If there's an interest in connecting the express org repos to snyk for automated package updates and security fixes I'm happy to help with that as well but that seems the obvious thing. Is express using npm audit or GHA today at least to stay up to date?

@dougwilson
Copy link
Contributor

Is express using npm audit or GHA today at least to stay up to date?

Yes

@lirantal
Copy link

Super!

@wesleytodd
Copy link
Member

wesleytodd commented Mar 30, 2020

2FA for committers and publishers

This is actually what kicked off the whole conversation in the Package Maintenance WG. I brought it up and @dominykas took it from there. This has resulted in the RFC for a staging area in npm.

I have a script I use that can also tell me for a given project exact which npm users have the ability to alter the package after publishing

Could you document this? Or maybe publish it so we can all use it?

All github orgs except expressjs require members to have 2fa on on their account; expressjs has users without it on and we may want to rectify that at some point and turn on the requirement.

I think we should change this ASAP. There is no reason for us to not have everyone enable 2fa. Anyone who doesn't should be told to enable it or they will be removed IMO.

cc/ @nat I know you mentioned improvements to 2FA publishing on the npm announcement, and wanted to ping you here as an example of where the community can benefit from some synergy ❤️

While we are pinging GitHub folks, I chatted with @clarkbw and I think he might be a person more close to this topic. Bryan, if there are any bits of feedback we can give to help set feature direction on this, let us know!

As Express is a key piece in the ecosystem we can ask for support from the security-wg

Along with the security wg, we also have the Web Server Frameworks Team which has this as part of the charter.

If there's an interest in connecting the express org repos to snyk for automated package updates

I have been digging deep on this topic lately at work, and I do not think this will be something Express should do unless something big changes with the way these CVEs are vetted or reported. It is MUCH too noisy at the moment. This is something I have been vocal in the community about. The incentives are miss-aligned here between maintainers like us, and secuirty reporting tools like Snyk and npm's security tools. If you have a "maintainer first" approach, one which prioritizes maintainer time and energy over getting virtual points for identifying and reporting CVEs, then I am happy to change my opinion here.

I would be happy to participate in a public discussion about what needs to change for a project like Express to have real value in a tool like Snyk, let me know if you are interested.

Is express using npm audit or GHA today at least to stay up to date?

@dougwilson can you talk more about what you mean by "yes"? We do not have this setup in an automated fashion, and have turned off dependabot because it is too noisy. Part of the issue here is that the auto updates don't really work in this project like it works in others. The current tooling has no indication of "these deps are locked for a reason, dont auto update them". This lack causes so much noise that the tooling is more harmful to productivity than helpful.

@dougwilson
Copy link
Contributor

I think we should change this ASAP. There is no reason for us to not have everyone enable 2fa. Anyone who doesn't should be told to enable it or they will be removed IMO.

Yea, I agree, just haven't gone though to actually get the list, notify everyone with reasonable time, etc. I think there is at least one TC member that would end up kicked due to that, so if they don't enable 2FA, we need to go through and off-board that member before removal.

@dougwilson can you talk more about what you mean by "yes"? We do not have this setup in an automated fashion, and have turned off dependabot because it is too noisy.

The question didn't ask if we had it automated, from my understanding :) Express is using npm audit today.

@wesleytodd
Copy link
Member

wesleytodd commented Apr 29, 2020

@expressjs/express-tc We will be turning on forced 2FA on github. If you don't have it setup and want to maintain your access, enable it within the next 7 days. After that we will be turning it on.

FWIW, we should also have this required for @expressjs/triagers as well. So if you are a triager and wish to continue to be, go ahead and setup 2FA.

@dougwilson
Copy link
Contributor

The setting is just for organization members without exception. So turning it on will kick all members (TC, triage, etc.) without 2FA on will get kicked. I believe it will state the reason was no 2FA on though in the auto generated email.

@jonathanong
Copy link
Member

Semi-related: I have a person emailing me trying to report some security vulnerabilities. We should have an express public mailing list that allows people to submit these. It seems like a lot of you hide your email addresses so they aren't able to contact you directly.

Or do we have a process? I didn't see anything on the expressjs.com site.

@dougwilson
Copy link
Contributor

Are they for express itself or some of the modules around it? We can make a group email with with openjsf now like we did for the coc policy. I have seen if folks do not know who to contact they would contact the owner of the npm module (emails are always public there plus in the package.json), but that doesn't mean we cannot just put an email group together. I heard that github itself is working on a way to ingest reports directly on there, but that is just future wishing , haha.

We have the security repo in this org if you want to paste the information you got in there for the time being.

@jonathanong
Copy link
Member

Yep, I pasted it in the security repo. It's for express itself.

A mailing list is also preferable for some orgs so that lawyers can keep track of vulnerability reports and fixes. Not sure if that would be beneficial here.

@dougwilson
Copy link
Contributor

It's for express itself.

I see one in there, but that is for one of the middlewares a d not express the module. Is there another one or does that report span multiple modules? If the latter can your annotate it for which bullet is which module?

@clarkbw
Copy link

clarkbw commented Jan 21, 2021

/cc @ethomson @MylesBorins ☝️

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

No branches or pull requests

7 participants