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

Release tooling: Use Github Security Advisories #9335

Open
chillu opened this issue Nov 25, 2019 · 11 comments
Open

Release tooling: Use Github Security Advisories #9335

chillu opened this issue Nov 25, 2019 · 11 comments

Comments

@chillu
Copy link
Member

chillu commented Nov 25, 2019

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

  • The process has less manual steps than the current one
  • The process can be applied to all supported modules
  • We use the Github CVE authority
  • There continued to be one feed for subscribing to Silverstripe security issues for supported moduiles
  • The process allows us to ensure Travis builds on private releases pass before disclosing issues
  • The process is documented as part of the standard release process and checklists
  • Core committers, contributing committers and stakeholders from Silverstripe Ltd. (e.g. our CISO) can collaborate on issue resolution privately

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:

  • Option A: Retroactively add all known vulns into FriendsOfPHP, work with Github to pull them in
  • Option B: Retroactively add all known vulns to CVE, work with Github to pull them in (this seems like a giant PITA?)
  • Option C: Retroactively add all known vulns to Github Advisories directly on the modules. We can't backdate these, which could cause a lot of confusion
  • Option D: Publish new advisories through Github, keep advisories on silverstripe.org as the canonical source (either manually duplicating announcements there, or integrating with Github feeds)
  • Option E: Switch to Github advisories going forward, changing feeds. Send a one off message to current subscribers of the old silverstripe.org feed (as a pseudo-entry in the feed). Note that Github also publishes RSS feeds for a project's advisories, although we currently combine them into a single feed across all supported modules.

Notes

Related

@chillu
Copy link
Member Author

chillu commented Nov 26, 2019

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.

  • Pro: One less Github organisation to manage members in (could get rid of github.com/silverstripe-security)
  • Pro: Clearer association between multiple pull requests and an advisory (they're the only custom branches on this fork)
  • Con: More repos to clutter our repo listings on github.com/silverstripe for admins (one per advisory)
  • Con: One fork per advisory (vs. one fork per module in the status quo)
  • Con: Travis builds not supported at the moment

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
Copy link
Member Author

chillu commented Dec 9, 2019

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
Copy link
Member Author

chillu commented Dec 9, 2019

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
Copy link
Member Author

chillu commented Dec 9, 2019

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
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
Copy link
Member Author

chillu commented Jan 5, 2020

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
Copy link
Contributor

brynwhyman commented Jan 13, 2020

@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: silverstripe/silverstripe-cms#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
Copy link
Contributor

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
Copy link
Member Author

chillu commented Jan 29, 2020

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

image

image

image

@chillu
Copy link
Member Author

chillu commented Feb 4, 2020

A few points I've clarified with Github support:

  • Private security forks automatically get deleted after merging and publishing the advisory, so minimising clutter
  • You can use advisories without associating pull requests on them (we can retain our current workflow)
  • You can grant access to advisories for both teams and individuals, which would work for us
  • Github is planning to improve the FriendsOfPHP import, currently semi-manual and only imported a fraction of the advisories we published there.

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
Copy link
Contributor

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

Noting that this is done now 😄

@chillu
Copy link
Member Author

chillu commented Oct 1, 2020

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.

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

No branches or pull requests

4 participants