silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
719 stars 820 forks source link

Release tooling: Use Github Security Advisories #9335

Open chillu opened 4 years ago

chillu commented 4 years ago

Overview

Our security vuln management process is quite manual: Create an issue in github.com/silverstripe-security, create private forks, review pull requests on those forks, merge into a release branch on the private fork, release private RC from private fork, merge back to public repo on public stable release, copy announcement draft from Github issue and publish on ss.org. Github Security Advisories promises to streamline some of this.

There's three levels of adopting them:

  1. Only use them to request CVEs (instead of emailing Mitre) and publish copies of advisories in addition to our existing processes and channels
  2. Also replace our current advisory publication with Github
  3. Also use Github's temporary private forks for collaborating on a fix

The acceptance criteria below assume we're doing all of the above.

Acceptance Criteria

Advisory Publication

We currently only have five issues listed in the Github Advisory Database. There are more issues in the CVE list, but it's not complete. The only complete list is on our own advisories on silverstripe.org. They list FriendOfPHP as one of their sources, but haven't reflected most of the security issues there.

Options:

Notes

Related

chillu commented 4 years ago

OK I've trialled this with a test advisory and a private temporary fork. Travis CI picks up the new private repo, see https://travis-ci.com/silverstripe/silverstripe-framework-ghsa-pfcw-wfpx-2r26/builds. But it doesn't trigger any builds, even after sending a pull request. I've asked on their community forum.

Even if the Travis builds were working, it's not a clear-cut decision.

I think we should start using Github advisories without using those private forks. They would only be a real advantage to us if Travis was somehow identifying them as "private but related to open source repos", and provide build capacity free of charge.

chillu commented 4 years ago

Another angle I hadn't considered: We can't really put any third party on the critical path to our security releases. At the point when we release, we need to be able to point to an authoritative public description of the vulnerability. And ideally one where we can easily control updates.

This definitely excludes CVE publication, I've experienced weeks of delays between request and publication through Mitre. Might be better through Github as the CNA.

And it also excludes FriendsOfPHP since that's a volunteer-driven pull request system. They even mention explicitly on the README that it's not to be used for a "primary info source". They have a good track record of merging PRs in a day or so, but even that is a pain if we'd pull in a feed directly from their repo to silverstripe.org.

So the value of Github Security Advisories to us keeps decreasing. I'll contact them to ask why they haven't pulled in the FriendsOfPHP stuff even though they claim to do so.

chillu commented 4 years ago

I've contacted Github:


Hello! We're a PHP Open Source project which publishes security vulnerabilities through the FriendsOfPHP advisory database: https://github.com/FriendsOfPHP/security-advisories/tree/master/silverstripe

According to https://help.github.com/en/github/managing-security-vulnerabilities/browsing-security-vulnerabilities-in-the-github-advisory-database, you are adding advisories from this database to the Github Advisory Database.

Quite a few seem to be missing though: FriendsOfPHP has 28 advisories for the silverstripe/* vendor namespace. Github only finds 5: https://github.com/advisories?utf8=%E2%9C%93&query=silverstripe

Specific example of a missing advisory: https://github.com/advisories?utf8=%E2%9C%93&query=CVE-2019-12437 https://github.com/FriendsOfPHP/security-advisories/blob/master/silverstripe/graphql/CVE-2019-12437.yaml

Note that we've also started publishing advisories through CVE a few months ago, but use the same identifiers between those two databases.

We're hoping for the wider community to benefit from Github Security Advisories, it's a fantastic service! Any chance you can check why you've got partial data?

Also, you obviously can't make commitments on a free service like this, but can you give us insights into much delay we should expect between publishing an issue in FriendsOfPHP, and it popping up in a Github Security Advisory? I'm conscious there's often human review involved, just looking for ballpark figures to avoid us getting confused when something is missing that might still be under review.

Thanks Ingo

chillu commented 4 years ago

Aaaand another email to Github:


Hello! We're running an Open Source project which publishes security advisories on CVE as well as the FriendsOfPHP database (https://github.com/FriendsOfPHP/security-advisories/tree/master/silverstripe/framework). I've just sent an enquiry about missing entries there (#488061).

Another potentially impactful bug I've noticed: When parsing the CVE information, the lower bounds for a dependency seems to get lost. This leads to Github repos being marked as vulnerable when they aren't. In the following example, we've marked the lower bounds as >=4.1.0 via the CVE disclosure, so not affecting projects on 3.x versions of our software. Yet the corresponding Github advisory claims vulnerable versions: < 4.3.6.

https://nvd.nist.gov/vuln/detail/CVE-2019-12204 https://github.com/advisories/GHSA-cg8j-8w52-735v

Note that we're planning to more consistently publish advisories on FriendsOfPHP, which probably has a more structured data approach to those constraints. Maybe those clearer definitions could take priority where the identifiers match? We're using the same CVEs in FriendsOfPHP advisories as well, so should be unambiguous.

Thanks! Ingo

chillu commented 4 years ago

Heard back from Github support. I'll have a call with them soon to clarify a bit


Thanks for reaching out, and sorry it's taken a bit to get back to you.

We're slowly ramping up our Advisory Database and the curation team that manages all of the intake into the database. Right now, we have a semi-manual importer that we can run to import from FriendsOfPHP. We mainly use it when we see a PHP-ecosystem CVE come through our National Vulnerability Database importer, because we've found that you generally have higher-quality and more accurate metadata. That's why you see only a subset of your advisories in GitHub Advisory Database.

In 2020, as we staff up the curation team, we hope to get the FriendsOfPHP importer running automatically, and to have our processes tuned enough that we can do a "catch-up" curation of all the existing advisories in FriendsOfPHP.

brynwhyman commented 4 years ago

@chillu

Aaaand another email to Github:

'< 4.3.6' isn't entirely accurate either. It looks like this is also resulting in GitHub advisories recommending upgrading to non-existent releases (4.3.6) too: https://github.com/silverstripe/silverstripe-cms/issues/2511

Edit: Actually, that looks to be the version that was communicated in our vendor advisory: https://www.silverstripe.org/download/security-releases/CVE-2019-12204

robbieaverill commented 4 years ago

Perhaps they're assuming that a disclosure would also go with a stable patch release with a fix in it, in which case it's on us for not releasing a patch for that release line?

chillu commented 4 years ago

Another email convo with Github support (sorry, formatting is too hard to paste)

image image image
chillu commented 4 years ago

A few points I've clarified with Github support:

So in the short term, I think we're blocked by us backfilling into FriendsOfPHP (nearly done), and Github backfilling from FriendsOfPHP, as well as automating this. Once those pieces are in place, we could start filing Github advisories instead of requesting CVEs from Mitre, but keep our current workflow around issues and private forks. The advantage here is that we could replace https://www.silverstripe.org/download/security-releases/ with a feed coming straight from Github (through the securityVulnerabilities query in their GraphQL API), and wouldn't need that duplication of info in our own CMS. Admittedly, I'm not sure this tweak is worth rewriting this logic, since we still have four places for advisories (private Github repo, FriendsOfPHP, CVE, Github advisory).

brynwhyman commented 4 years ago

I think we're blocked by us backfilling into FriendsOfPHP (nearly done), ...

Noting that this is done now 😄

chillu commented 3 years ago

Github is still not reliably importing advisories from FriendsOfPHP. I think publishing our own advisories is becoming a higher priority, since this partial alerting by Github is quite damaging - it leads to a false sense of security among Silverstripe users. Having seen some security alerts related to our repos, they'll assume it'll cover all of them - but it's just a random subset (~10%), updated at random times, potentially months after the vulnerability was disclosed.