Security considerations of pull_request_target github action workflows

[mkulke]

There are reports about recent supply-chain attacks that use pull_request_target as an entry vector to comprise an open source project's binaries in ways that are very hard to prevent or even detect.

There is clear advice against using foreign, untrusted code in a pull_request_target workflow that is running with the privileges and secrets of the project's repo. Our CI use case makes it hard to consider this advice, we want/have to run foreign code on private runners for architectures (s390x atm, maybe more later) not covered in github-hosted runners.

However, we can maybe carefully quarantine those s390x jobs into simple and heavily scrutinized workflows that will run in a pull_request_target context.

The amd64 jobs that we run on a PR should not require special privileges, they should be able to run in the context of the PR opener's fork. At the moment this is not possible:

PodVM

artifacts are passed around as OCI images. you need write access to a registry for that, but I suspect that's an implementation detail that can be reworked to use the GH action artifact store.

CAA Image

that's harder. since we need an actual OCI image to launch as daemonset. maybe we can use a k8s-hosted registry. problem: the test provisioner code wants to create the cluster and deploy CAA. the installation of the registry and the pushing of the CAA daemonset image would have to be somehow injected between both.

[stevenhorsman]

Sorry, allow me to restate some things to check that I understand the issues:

The difference in security posture is that pull_request only has a read-only token by default and doesn't have access to secrets. pull_request_target has a read/write token and can access secrets. The big security gaps are:
- The token having write access means that if we execute arbitrary code (i.e. a pull request) then we could leak that secret and then an attacker could use it to cause issues
- The write token permission means that actions like poisoning caches, deleting artifacts? etc could be possible.

GHA's trade-off to try and limit this is that workflows with pull_request_target only run the workflows that from the main (or more generally target branch). This is annoying, so we've talked about (and I started a PoC) moving the workflow steps into scripts that the workflow calls to make testing much easier, but this further undermines the security measures, so maybe we should not do this approach, but try and move more steps into the workflow? Or is the point that we will always need to call some code/scripts, so we will always be vulnerable.

Trying to think of some concrete steps we could take:

1. If we adjust the token permissions to read-only (which I tried a while ago and broke our workflows due to the ghcr pushes amongst others):

how much of the issue does that mitigate? 2. As you mention - we could try and isolate the issue by running the amd64 jobs with pull_request (if we can remove the need for secrets and write access). - As a side question - does running merged code in the `main` branch have access to a write token, as I assume we still want to build images in the daily/merged branch triggered? I guess that means we'd need to do sloppy reviews to enable this attack 3. We've already enabled the approval needed for all external users This should limit the impacts as a member of the project is required to let the CI run. Obviously this isn't foolproof, but I guess getting any code merged isn't either.

Sorry if this is rambling, I'm trying to remind myself of the main issues here.

[mkulke]

• The token having write access means that if we execute arbitrary code (i.e. a pull request) then we could leak that secret and then an attacker could use it to cause issues

• The write token permission means that actions like poisoning caches, deleting artifacts? etc could be possible.

yes, executing untrusted code in the context of the project repo is just a large attack vector. there are ways to siphon secrets from those runs through various means that are hard to spot in a review (like string extrapolation). see here

We use one risky practice (pull_request_target) as a remedy for another insecure practice (operating self-hosted runners on a public repo)

I don't think there is an easy way out. We do want to run untrusted code on-hosted-runners, we cannot harden this 100%, we can only replace problem X with problem Y.

I think we can cordon off problem Y to areas where problem X exists, and try to find more remedies for Y. Currently we extend problem Y to places where we do not have problem X (no private runners). Mostly for architectural reasons to keep things DRY.

I'd argue we should architect the pipelines security-first and then consolidate what's possible with the less-secure workflows. That would imply, if I'm not missing something, that a random user is able to open a PR and this PR is running github-hosted e2e tests without any privileges required.

As a side question - does running merged code in the main branch have access to a write token, as I assume we still want to build images in the daily/merged branch triggered?

I would say the merged code we do trust - or I wouldn't know what else to trust. The crux of pull_request_target: it might mess with your build environment (caches, artifacts, secrets, ...) in malicious ways. That means we cannot simply derive trust in the build environment from trust in the code and the project admins, we now have to make sure that we don't use caches in artifact builds, etc. very annoying.