nodejs / package-maintenance

Repository for work for discussion of helping with maintenance of key packages in the ecosystem.
Other
407 stars 144 forks source link

[baseline practices] Security Reporting #159

Closed rxmarbles closed 4 years ago

rxmarbles commented 5 years ago

This is in continuation of #119 to keep the discussion focused on one part as we can further flesh out each piece to create a draft proposal around baseline practices.

Security Reporting

If I am missing anything please add a comment and tag me so I can update

mcollina commented 5 years ago

We should be vendor neutral, as there are plenty of vendors that provide the security scanning. We should not express bias.

ljharb commented 5 years ago

i don’t think there’s any bias with recommending tools that ship with node.

Eomm commented 5 years ago

Maybe we could suggest the command with npm (that is out of the box with node) and say "there are any other solutions, search for them and choose what fit better for your needs"

We can suggest some common topics to investigate that concerns how the auditing is done. Only generals titles that are shared across all the providers

dominykas commented 5 years ago

There is no reasonable way to utilize npm audit in ci/cd at the moment, because in most cases you can't fix the problem yourself, and there are no ignore files.

"Look for other tools" is not exactly valuable guidance... That said, I think various scanners are important for package consumers, i.e. apps, whereas from maintainer perspective the best that can be done is staying up to date, helping other maintainers do the same, and forking+fixing when all else fails.

Security guidance should include best practices on responsible disclosure and providing security contacts.

ljharb commented 5 years ago

@dominykas i use it in CI all the time - https://github.com/ljharb/object-keys/blob/master/package.json#L43-L45 - the "fix" functionality isn't helpful, but failing tests whenever there's an unaccounted-for vuln is. I use https://github.com/ljharb/object-keys/blob/master/.npmrc#L2 atm to bypass things (but it'd be much better to instead have an explicit list of packages whose vulns i'm ignoring, ofc - that'd need to be an npm RFC)

Eomm commented 5 years ago

I think the discussion could go ahead if we explore the possibilities (like in the #206 ) and define an output like blogpost or draft doc here

wesleytodd commented 5 years ago

I must have missed this issue when it was first opened, but we could also make some recommendations on handling reports best practice. Here is a link to the reporting procedure for express: https://github.com/expressjs/express/blob/master/Security.md

Maybe we could draft a standard reporting SECURITY.md document for projects to copy/paste?

MarcinHoppe commented 5 years ago

Is this something that the @nodejs/security-wg could help with?

mhdawson commented 5 years ago

@MarcinHoppe help from the @nodejs/security-wg would be great. Any chance you are volunteering to help lead it?

MarcinHoppe commented 5 years ago

@mhdawson Yes, I think I can help. What is the best way for me to understand the problem we are trying to address here?

Eomm commented 5 years ago

@MarcinHoppe our target is to write a guideline that helps the community to adopt good practices. This will improve the overall quality and security of the modules in order to let the ecosystem grows in a heltier way possible.

This is an example for the ci/cd world.

MarcinHoppe commented 5 years ago

Thanks, this helps a lot.

Just to explore a bit of background here: is there a consensus on whether we should require package maintainers to triage the reports themselves or do we want to lean on the Node.js Ecosystem disclosure and bug bounty program we are running on HackerOne?

wesleytodd commented 5 years ago

IMO we probably need to take two approaches to this. Some maintainers will probably not want to rely on the Node.js process as it is more involved.

I know for Express we use the HackerOne system, but also take reports directly. I think if we could just do HackerOne we might, but it has never been clear on how we get to that eaisly. If it was really easy to onboard and get involved in, then just HO might be fine, but I think a more simple approach for maintainers of smaller projects might be good.

MarcinHoppe commented 5 years ago

My personal take on this is that we should leave the choice with the maintainers. So if they have the capacity to respond to security vulnerabilities and handle advisories themselves, great. If not, we can offer an way to opt-in to the Node.js Ecosystem program on HackerOne.

Does this sound like a reasonable strategy? If it is, I can start drafting the guidelines.

wesleytodd commented 5 years ago

This sounds great to me. The goal is to provide value while reducing work/friction, so this approach seems great to me! Thanks @MarcinHoppe!!

sam-github commented 5 years ago

@wesleytodd Quick note: the express policy points to https://nodesecurity.io/report which doesn't exist anymore, it just redirects to the top page of https://www.npmjs.com/

@MarcinHoppe You can't really opt-out of H1. People who find vulns can report them to npm, H1, snyk... anwhere they want, and a package will have to deal with them when they find out (or not, I guess that's an option).

What they can do is advertise a primary contact to get notified of reports/vulns related to the package (as they should!). If they want, they can request that the initial contact is made through H1, or not.

This is useful for vulns reported directly, but its also useful when a vuln is reported to some other org, because then that org (the nodejs sec-wg, npminc, etc.) when they go looking for the package maintainer have a private communication channel.

All of which is to say, the most important thing is that the policy describe a communication channel that will be promptly responded to.

https://github.com/node-red/node-red/security/policy is another example. Short and sufficient.

wesleytodd commented 5 years ago

the express policy points to https://nodesecurity.io/report which doesn't exist anymore, it just redirects to the top page of https://www.npmjs.com/

Thanks for the call out! Does anyone know the correct url? Or is there no longer a place to send people to submit reports?

You can't really opt-out of H1. People who find vulns can report them to npm, H1, snyk... anwhere they want, and a package will have to deal with them when they find out (or not, I guess that's an option).

This is actually the problem I am worried about. We have gotten bitten by reports coming via the new platform of the week", and expecting maintainers to know how to respond or resolve issues from all the different platforms is just a broken system.

MarcinHoppe commented 5 years ago

@sam-github

What they can do is advertise a primary contact to get notified of reports/vulns related to the package (as they should!). If they want, they can request that the initial contact is made through H1, or not.

This is exactly what I am going to include in the guidance.

@wesleytodd

This is actually the problem I am worried about. We have gotten bitten by reports coming via the new platform of the week", and expecting maintainers to know how to respond or resolve issues from all the different platforms is just a broken system.

From a perspective of person triaging reports on HackerOne: a clear protocol to reach the maintainers and not do detective work of extracing e-mails from git log should actually prevent the "new platform of the week effect". Whoever gets the report, will likely use the advertised channel to reach out.

wesleytodd commented 5 years ago

a clear protocol to reach the maintainers and not do detective work of extracing e-mails from git log should actually prevent the "new platform of the week effect".

I am not yet convinced 😉. Express has had "Report security bugs by emailing the lead maintainer in the Readme.md file." in the security.md since 2016, and I have seen this happen multiple times, to great pain on the part of Doug (the lead maintainer).

But I am hopeful we can find a good path forward, and I think more standardization will help in the long run.

sam-github commented 5 years ago

@wesleytodd I don't understand your last comment.

You say "not yet convinced", but then go on to agree with the statement you quoted, namely, that projects that state how they want to get contacted, get contacted that way. This is also what @MarcinHoppe is saying, that its painful to use git log, or other techniques to try to figure out who to contact. Its better for projects to state it clearly, like express does, so there isn't any detective work.

As far as I can tell, everyone is in agreement here: projects should choose how they want to be contacted and document it clearly, so vulnerability reporters know how to reach the project.

wesleytodd commented 5 years ago

projects that state how they want to get contacted, get contacted that way.

Sorry If my comment did not make this clear. We do not get contacted that way, almost ever. We get public issues opened. We get invalid reports opened on random websites we don't know about, then get users reporting that their builds are failing because they use that service's audit. I think we one time got an invalid report via the stated reporting flow. Every other report in 3 years has been miss-handled.

sam-github commented 5 years ago

@wesleytodd

Express has had "Report security bugs by emailing the lead maintainer in the Readme.md file." in the security.md since 2016, and I have seen this happen multiple times,

Sorry, I read that as meaning that the lead maintainer got emailed multiple times!

If a project has a sec policy document, but people ignore it, I'm not sure what can been done about that.

Even so, I'd still suggesting packages have a policy, at least it offers a possibility of people following it, whereas with no policy its hard to credibly complain about the flavour-of-the-week reporting.

Do you disagree?

wesleytodd commented 5 years ago

I do not disagree, but I am still not convinced it will make maintainers lives easier because people just don't read or follow instructions 😆

If most projects follow a similar pattern, then I think it will make it more likely that security researchers or reporters will have read it at least once, and look for the same pattern in other projects. So my hope is that everyone following makes it more likely that projects like Express will not get the brunt of miss following the reporting best practices.

MarcinHoppe commented 5 years ago

We can at least give vulnerability reporters a chance of making life a little bit easier on maintainers!

MarcinHoppe commented 4 years ago

Now that GitHub offers security policies and advisories, I think that might be a very sensible default for the majority of projects.

A SECURITY.md file could be a fallback option for projects hosted outside of GitHub.

ljharb commented 4 years ago

Every repo now has a /security/policy link, which is populated by the repo or the org’s .github repo’s SECURITY.md file. I think we should strongly encourage people to visit that link for a given repo, and then strongly encourage maintainers to populate it by whatever means github supports.

lholmquist commented 4 years ago

TIL that there is a security tab in github.

wesleytodd commented 4 years ago

I am pretty sure the advisories and policy parts are brand new. The security tab has been there, but it was mostly these dumb and inaccurate alerts. I think every library I support has been flagged because mocha depends on lodash, and utility libraries like lodash have voulns reported on single methods (which probably are not even used in the libraries which import them) all the time. These have been the most annoying feature of github for me for the past few years 😢

That said, I really like the feature of reporting them on the platform, this might make the problems I discussed above much easier to solve.

MarcinHoppe commented 4 years ago

I submitted #277. I would love to get feedback and start the discussion on specific recommendation or even the general outline of the document.

/cc @mhdawson @wesleytodd @sam-github