Lock down GitHub Actions Security

[briansmith]

I just merged PR #49 which minimizes the permissions of the GitHub token. I also changed the default permission of the GitHub token from read-write to read-only in the repository settings over the weekend, but I don't think people can see this.

Now we still need to follow the (rest of the) guidance in https://docs.github.com/en/actions/learn-github-actions/security-hardening-for-github-actions to lock down our CI/CD.

untrusted has no dependencies besides actions and the Rust toolchain, so it is less urgent to do work to ensure dependencies are following the recommended GitHub Actions security practices. However, eventually we'll need to extend our CI/CD to ensure that no new dependencies without such hardening are added as dependencies of untrusted.

[briansmith]

• I audited the (use of) secrets in this repository. There are no secrets defined or used.
• PR Sync GitHub Actions permissions and GITHUB_TOKEN handling with *ring*. #49 and my manual action mentioned above minimize (close enough) permissions for the github.token and disabled actions/checkout@v2's persistent credential caching.
• PR CI/CD: Remove optimization for owner's PRs. #51 removes the optimization that I put in place to avoid mostly-duplicate CI job runs for my own PRs. This isn't strictly needed as a hardening procedure and it isn't in the GitHub guide, but I feel better removing the optimization.
• PR CI/CD: Use private cache of (third-party) GitHub Actions. #52 switches this repo to use forks of each (third-party) action I use. I will update the "v1" or "v2" or whatever git tag of each of those dependencies over time to update them to newer releases of the actions. I'll be doing this instead of using the full commit SHA hash to reference from the YAML which exact version I'm using. I think my chosen approach will be easier to maintain than using the full commit SHA because using the commit SHAs in the YAML would result in a lot of error-prone copy-pasta. I'll revisit this if/when there is some effective mechanism to eliminate the repetition (some kind of variables mechanism for the GitHub Actions YAML). In terms of GitHub's guidance, this is choosing "Pin actions to a tag only if you trust the creator." Note that in my forks, I didn't enable any GitHub Actions workflows AND I also explicitly disabled GitHub Actions within my fork repos in the GitHub Actions section of their settings. Thus, we don't have to worry (so much) that they might be tampered with through any security problems in their own CI/CD.
• Except for its (implicit) use by actions/checkout, I don't use the GitHub Token.
• I verified no deploy keys, GitHub app tokens, personal access tokens, or SSH keys are used.
• I verified no self-hosted runners are used.

TODO:

• I have not implemented the "Audit the source code of the action" advice to any extent that I feel comfortable saying that I fully understand each action. Even the simplest action like action/checkout@v2 is impractical to audit meaningfully, IMO. Instead, I hope to replace most or all use of third-party actions with alternatives code/scripts that live within the repository or within a as-yet-uncreated "briansmith/github-actions" repository that would store such.
• Create a plan for auditing the actions logs as suggested in the GitHub Actions security hardening guide.
• Use "::stop-commands::" to eliminate unnecessary risk from the "commands embedded in the stdout of steps" mechanism. This requires us to first ensure that we're not relying on any actions that would be broken by doing this.
• I filed Remove persist-credentials or change the default to false actions/checkout#485 asking for a new version (v3) of the actions/checkout action that avoids persisting the credentials by default.

[briansmith]

$ grep "uses:" .github/workflows/ci.yml | sort | uniq
      - uses: briansmith/actions-cache@v2
      - uses: briansmith/actions-checkout@v2
      - uses: briansmith/actions-rs-toolchain@v1
      - uses: briansmith/codecov-codecov-action@v1

Here is my currently rough plan for replacing the actions:

• actions/cache@v2: Instead of building tools like cargo deny from source and then caching them (which is way too slow), I'm considering a couple of alternatives. The most likely to succeed is to create a repository of the pre-built tools, and then use shallow/skinny git clones/checkouts to fetch those prebuilt binaries. Eventually I hope to use smarter alternatives. I've been meaning to do this to improve the reliability and security of CI anyway by eliminating the downloading of tools from third-party servers (Now I do SHA-256 checksum checks of the downloads, but sometimes the servers are down.)

• actions/checkout@v2: It's a github-supplied action so it's not an emergency. However, I'm worried about the persist-github-token-by-default policy of the action; I might accidentally forget to disable it. It seems like it wouldn't be too hard to write a shell script that does a shallow clone of the correct branch/revision from https://github.com/briansmith/<repo name>. Then no github.token would even be required, since it's a public repo. However, sometimes I have private forks of this repo and it would be nice if the script could work with the private copies too, using the github token only in the case of a private repo.

• actions-rs/toolchain: I will extend the mk/install-build-tools.[sh|ps1] scripts with the ability to (securely) download and install the Rust toolchain. I've been needing this for other reasons anyway.

• codecov/codecov-action: I've been meaning to replace this with a script that works locally.

BTW, I noticed lots of Rust projects use the actions-rs/cargo action. I found it was pretty easy to avoid using that; it doesn't seem to do anything particularly useful, though maybe I'm just an easy-to-satisfy person.

[briansmith]

TODO:

• Above, I stated that I changed a few of the settings of this repo to make the repo more secure. It would be nice if these claims were verifiable in some way by others.

[briansmith]

TODO:

• Verify that dependabot informs me of security updates to the actions I forked.

[briansmith]

Regarding ::stop-commands::, see actions/runner#807; we need to keep the token from being logged.

[briansmith]

After switching to my forks of each action I used, I also switched the Allowed Actions setting of this repository to "Allow local actions only."

[briansmith]

For the codecov action, we might consider using the GitHub Actions artifacts mechanism (https://docs.github.com/en/actions/guides/storing-workflow-data-as-artifacts) to separate the collecting of the coverage, which requires a git checkout, from the uploading of the coverage results to codecov, which doesn't.

• Job 1 checks out the code, builds and runs the tests with coverage profiling enabled, and then uploads the coverage data files as artifacts.
• Job 2 downloads the coverage data files as artifacts and uploads them to codecov.

[briansmith]

After switching to my forks of each action I used, I also switched the Allowed Actions setting of this repository to "Allow local actions only."

Since I forked the entire repository for each action, that means every revision of every action, with every tag and every commit, is in my fork. Instead of "Allow local actions only," having an allowlist of a single specific tag (v1, v2, whatever) for each action would be safer. I'll give that a shot tomorrow.

[briansmith]

In https://docs.github.com/en/actions/security-guides/automatic-token-authentication it is noted that if we set the repository- or organization- level Actions policy to be "restricted" then we get exactly what we want. But, I couldn't find a way to publicly expose the fact that the Actions policy is "restricted," nor could I find a way to declare in the workflow YAML that I want the workflow to use "restricted" defaults.

In https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token several permissions besides "contents" are mentioned, but in our YAML we only lock don't the contents permission. So, should take the full list of permissions mentioned there and set each one explicitly to none (except contents: read) in the YAML?

Should we have a job that simply checks the workflow configuration? E.g. checks that the repository is in "restricted" default mode and/or enumerates all the permissions and verifies that they are all none? I would love to learn how one would do this.