Review Github Actions for Security

[hoffie]

We are using Github Actions in several places:

• jamulus: Autobuild (including Releases and CodeQL)
• jamuluswebsite:
  • Jekyll
  • Merge between branches

We are not only using official Github-provided Actions there, but also multiple third-party actions (see below). I am not seeing any use of the permission: keyword there, implying that they run with default permissions. This means that those actions have access to a GITHUB_TOKEN with read and write permission to the relevant repo, as far as I understand.

I have reviewed the following docs and articles: https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ https://francoisbest.com/posts/2020/the-security-of-github-actions

My conclusion is that we should do the following:

• [x] jamulus: Review all referenced non-official actions for correctness and safety and reference them by sha1 hash instead of branch or tag. #2779
• [ ] jamuluswebsite: Review all referenced non-official actions for correctness and safety and reference them by sha1 hash instead of branch or tag.
• [x] Ensure that Dependabot is active as it supports alerting us if we miss updates to those pinned actions. #2778
• [ ] Enable Dependabot on jamuluswebsite as well
• [x] Set Github default action permissions to be restrictive (read-only) and update those workflows which need it to have write permission as well.
• [ ] Document the necessity to keep all of this in mind when reviewing PRs which touch these workflow files. I'm planning to include this in the Admin wiki page.

Note: I'm little worried about Github-official actions such as actions/ or github/ (we are trusting Github anyway!) or actions for other large open source projects with high reputation (ruby/), but I do worry about actions by third-party persons or orgs which we (or at least I?) don't know.

$ grep uses: jamulus*/.github/workflows/*.yml
jamulus/.github/workflows/autobuild.yml:           uses:                    actions/checkout@v2
jamulus/.github/workflows/autobuild.yml:           uses:                    dev-drprasad/[email protected]
jamulus/.github/workflows/autobuild.yml:           uses:                    actions/create-release@v1
jamulus/.github/workflows/autobuild.yml:        uses:                       maxim-lobanov/setup-xcode@v1
jamulus/.github/workflows/autobuild.yml:        uses:                       actions/checkout@v2
jamulus/.github/workflows/autobuild.yml:        uses:                       github/codeql-action/init@v1
jamulus/.github/workflows/autobuild.yml:        uses:                       actions/upload-artifact@v2
jamulus/.github/workflows/autobuild.yml:        uses:                       actions/upload-artifact@v2
jamulus/.github/workflows/autobuild.yml:        uses: devbotsxyz/xcode-notarize@d7219e1c390b47db8bab0f6b4fc1e3b7943e4b3b
jamulus/.github/workflows/autobuild.yml:        uses: devbotsxyz/xcode-staple@v1
jamulus/.github/workflows/autobuild.yml:        uses:                       actions/upload-release-asset@v1
jamulus/.github/workflows/autobuild.yml:        uses:                       actions/upload-release-asset@v1
jamulus/.github/workflows/autobuild.yml:        uses:                       github/codeql-action/analyze@v1
jamulus/.github/workflows/coding-style-check.yml:    - uses: actions/checkout@v2
jamulus/.github/workflows/coding-style-check.yml:    - uses: DoozyX/clang-format-lint-action@2a28e3a8d9553f244243f7e1ff94f6685dff87be
jamulus/.github/workflows/update-copyright-notices.yml:      - uses: actions/checkout@v2
jamulus/.github/workflows/update-copyright-notices.yml:      - uses: actions/checkout@v2
jamuluswebsite/.github/workflows/add-lang.yml:      - uses: actions/checkout@v2
jamuluswebsite/.github/workflows/add-lang.yml:        uses: actions/[email protected]
jamuluswebsite/.github/workflows/add-lang.yml:        uses: EndBug/add-and-commit@v7
jamuluswebsite/.github/workflows/add-lang.yml:        uses: peter-evans/create-or-update-comment@v1
jamuluswebsite/.github/workflows/jekyll.yml:      - uses: actions/checkout@v2
jamuluswebsite/.github/workflows/jekyll.yml:      - uses: dorny/paths-filter@v2
jamuluswebsite/.github/workflows/jekyll.yml:        uses: actions/[email protected]
jamuluswebsite/.github/workflows/jekyll.yml:        uses: EndBug/add-and-commit@v7
jamuluswebsite/.github/workflows/jekyll.yml:      - uses: actions/upload-artifact@v2
jamuluswebsite/.github/workflows/main.yml:      - uses: actions/checkout@v2
jamuluswebsite/.github/workflows/main.yml:        uses: actions/[email protected]
jamuluswebsite/.github/workflows/main.yml:      - uses: dorny/paths-filter@v2
jamuluswebsite/.github/workflows/main.yml:        uses: EndBug/add-and-commit@v7
jamuluswebsite/.github/workflows/main.yml:        uses: ruby/setup-ruby@v1
jamuluswebsite/.github/workflows/main.yml:        uses: limjh16/jekyll-action-ts@v2
jamuluswebsite/.github/workflows/main.yml:        uses: peaceiris/actions-gh-pages@v3
jamuluswebsite/.github/workflows/main.yml:      - uses: actions/checkout@v2
jamuluswebsite/.github/workflows/main.yml:        uses: devmasx/[email protected]

Not sure, if/when I'll have time for further work on this. Feel free to comment here and take over.

cc @jamulussoftware/maindevelopers @nefarius2001

See also: https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions https://github.com/ossf/scorecard/blob/main/docs/checks.md#pinned-dependencies

[ann0see]

Ok. Jekyll is an official action which doesn't require write access --> we could limit it to read easily. name: Sync release to changes and Build and deploy jekyll site are not official, need write access and should be reviewed.

Autobuild is much more complicated and involves a lot more actions. Currently this one is the most dangerous one, I think.

[ann0see]

Ok. I've now set the GitHub actions permissions to be more restrictive. However, we would still need to do the verification you mentioned and start pinning by hash.

[ann0see]

Comment by @hoffie in https://github.com/jamulussoftware/jamulus/pull/2527#discussion_r830526985 related to my suggestion of adding hash verification:

---

Well, but where do I get the hash from in the first place? I would have to download from the very same source, so the only protection benefit would be a benefit over time, i.e. if Google became malicious or a malicious actor could replace the file there. We aren't handling that attack vector anywhere else either (Qt, choco, pip, Android SDK contents, ...), do we?

I'm not opposed to doing that per-se, but I'd rather have a discussion and do it in all places if we decide to do it. Remember that it will carry a rather big maintenance burden though.

I think the current consensus is:

• Ensure authenticity/integrity of downlods by using HTTPS
• Use trusted download URLs
• Pin versions, especially for user-generated content (e.g. pip, choco) [and elsewhere for reproducibility reasons)

[hoffie]

Ah, now I understand what you're after. :)

This issue was mainly created for the referenced Github actions. The idea is to use hashes here in order to do version pinning on immutable versions (v1.2.3 are tags which are mutable, IIRC).

I think we have to distinguish several scenarios:

• Is the publisher equal to the author? In case of Github actions, that's not the case. Github is the publisher (or platform) and actions are often written by third-party authors. Same for pip, choco, brew. In case of Google Android Downloads, I'd consider publisher and author to be the same entity.
• Does the platform guarantee immutability? In case of Github, versions probably don't, but commit hashes do. In case of Google, we don't know. We assume that once released versions will not be replaced, but we can't be sure either. That's where you suggested using checksums and I think it would help there, yes.

Pinning Github action references by commit hash is trivial as it's designed to be done that way. Pinning external sources (e.g. curl/wget downloads or even artifacts from downloader tools such as SDK manager, aqt, ...) may be way harder.

[ann0see]

I've now locked down the allowed actions on the repo:

similar to the website.

[ann0see]

Note: Since the CI didn't start when I specifically allowed an action user/repo@*, I've now allowed actions based on users user/**. syntax. This should be a good tradeoff.

[ann0see]

I‘ve now set the workflow read only permission as my PR got merged.

[pljones]

@ann0see is this something that can be picked up for 3.11.0?

[ann0see]

I believe especially the website needs review. Tagging it for the next release is certainly possible

[pljones]

Detagging and moving to triage.