4 ms·

The problem is that there are fundamentally 2 different kinds of builds, but the current tooling is weak: * pre-merge builds on PRs. These should not have pri

by o11c 3y ago

The problem is that there are fundamentally 2 different kinds of builds, but the current tooling is weak: * pre-merge builds on PRs. These should not have privileges, but making the distinction between the two cases requires a lot of care. * official builds of "master" or a feature branch from the main repo. These "need" privileges to upload the resulting artifacts somewhere. Of course, if all it did was wake up some daemon elsewhere, which could download straight from the CI in a verified way based on the CI's notion of the job name, it would be secure without privileges, but most CI systems don't want to preserve huge artifacts, and maintaining the separate daemon is also annoying.

akx 3y ago
For the second point, PyPI's trusted publisher implementation does this very well: https://docs.pypi.org/trusted-publishers/ https://docs.pypi.org/trusted-publishers/
- LtWorf 3y ago
  Isn't that what's causing the problem? Without that there would be no need to have an action to do an upload. It could be comfortably and safely done offline.
trevyn 3y ago
It’s called GitHub secrets. Builds off of main get the secrets, pull requests from randos don’t. And public repos don’t pay for CI on GitHub. Not rocket science, people.
- n2d4 3y ago
  Pytorch did use GH secrets for the valuables and you can see that this wasn't enough, right there in the OP, because the self-hosted runners are still shared
  - adnanthekhan 3y ago
    Yup! This is what makes this kind of attack scary and very unique to GitHub Actions. The baseline GITHUB_TOKEN just blows the door open on lateral movement via workflow_dispatch and and repository_dispatch events. In several of our other operations, not just PyTorch, we leveraged workflow_dispatch to steal a PAT from another workflows. Developers tend to over-provision PATs so often. More often than not we'd end up with a PAT that has all scopes checked and org admin permissions. With that one could clean out all of the secrets from an organization in minutes using automated tools such as https://github.com/praetorian-inc/gato https://github.com/praetorian-inc/gato.
    - trevyn 3y ago
      To clarify, this secret stealing is not an issue with GitHub-hosted runners, correct?
      - adnanthekhan 3y ago
        Correct. For fork PR workflows on the pull_request trigger the GITHUB_TOKEN has read only permissions, so you can’t do anything with it. The key thing with a non-ephemeral runner is that (after obtaining persistence) you can grab the GITHUB_TOKEN from a subsequent non-fork PR build or a build on another trigger, which will have write permissions unless restricted by the repository maintainers.
  - lmeyerov 3y ago
    I think 'environments' was meant, where diff GHA environments get diff secrets, and policies dictate who gets to run what actions with what envs. But that is real work to setup, audit, and maintain. It'd be better if, like phone app capabilities, the default would be no privs, any privs are explicitly granted, and if they aren't being used, the system detects that and asks if you want to remove specific ones.
  - gabriel-samfia 3y ago
    Ouch! This is why ephemeral runners should be used. Preferably virtual machines. On an infrastructure that can define security group rules to prevent lateral movement.
- mlazos 3y ago
  If they use the same runners, couldn’t the attacker just wait? The runners would need to be sequestered too
  - kevin_nisbet 3y ago
    Absolutely. The real difficulty is tests on PR are by definition remote code execution by an untrusted source, so a full risk analysis and hardening needs to be done. Here's a similar mistake on an OSS repo a company I worked for made: https://goteleport.com/blog/hack-via-pull-request/ https://goteleport.com/blog/hack-via-pull-request/
- LtWorf 3y ago
  > Not rocket science, people. The smugness and overconfidence of someone who's about to be pwned?
  - trevyn 3y ago
    Pwn me.
snnn 3y ago
Ideally the builds for external PRs should not gain any access to any secret. But it is not practical. For example, you may want to use docker containers. Then you will need to download docker images from somewhere. For example, downloading ubuntu:20.04 from docker hub. That one requires a token, otherwise your request may get throttled. Even accessing most Github services requires a token. I agree with you that these things require a lot of care. In reality most dev teams are not willing to put enough time on securing their build system. That's why supply chain attacks are so common today.
LtWorf 3y ago
> These "need" privileges to upload the resulting artifacts somewhere. Only because pypi removed support for gpg and their only "signature" now basically is "was uploaded from a gh action". Otherwise a maintainer could do it offline without needing to give microsoft total access to uploading to pypi.
- Bognar 3y ago
  If you're using GHA to publish then you still need a trusted branch to provide secrets to. If you're not publishing using CI, then you can still upload to PyPI manually with an API Token. > without needing to give microsoft total access to uploading to pypi I assume you're referring to Trusted Publishers here? It's a per-project configuration using the industry standard of OIDC that you don't have to opt in to, so "total access" is a silly characterization. Also if you're insinuating that MS is going to generate fraudulent OIDC tokens to compromise a PyPI package, then you might want to start weaning yourself off the kool-aid.
  - LtWorf 3y ago
    > If you're using GHA to publish then you still need a trusted branch to provide secrets to The vulnerability was exactly that the secrets of the trusted branch can get leaked :) > you can still upload to PyPI manually with an API Token I can, BUT if you want to be a Trusted Publisher™ the only way is to do it via github. https://docs.pypi.org/trusted-publishers/ https://docs.pypi.org/trusted-publishers/ Most likely the plan is to make it compulsory for all projects eventually, just like they made 2fa compulsory. So less secure is considered as MORE secure by pypi :) Which is consistent with the idea that no PGP signature is more secure than signed uploads. Or the idea that a global token in a clear text file is somehow safer than a password that gets typed every time.

by o11c 3y ago

akx 3y ago

For the second point, PyPI's trusted publisher implementation does this very well: https://docs.pypi.org/trusted-publishers/ https://docs.pypi.org/trusted-publishers/

LtWorf 3y ago
Isn't that what's causing the problem? Without that there would be no need to have an action to do an upload. It could be comfortably and safely done offline.

trevyn 3y ago

It’s called GitHub secrets. Builds off of main get the secrets, pull requests from randos don’t. And public repos don’t pay for CI on GitHub. Not rocket science, people.

n2d4 3y ago
Pytorch did use GH secrets for the valuables and you can see that this wasn't enough, right there in the OP, because the self-hosted runners are still shared
- adnanthekhan 3y ago
  Yup! This is what makes this kind of attack scary and very unique to GitHub Actions. The baseline GITHUB_TOKEN just blows the door open on lateral movement via workflow_dispatch and and repository_dispatch events. In several of our other operations, not just PyTorch, we leveraged workflow_dispatch to steal a PAT from another workflows. Developers tend to over-provision PATs so often. More often than not we'd end up with a PAT that has all scopes checked and org admin permissions. With that one could clean out all of the secrets from an organization in minutes using automated tools such as https://github.com/praetorian-inc/gato https://github.com/praetorian-inc/gato.
  - trevyn 3y ago
    To clarify, this secret stealing is not an issue with GitHub-hosted runners, correct?
    - adnanthekhan 3y ago
      Correct. For fork PR workflows on the pull_request trigger the GITHUB_TOKEN has read only permissions, so you can’t do anything with it. The key thing with a non-ephemeral runner is that (after obtaining persistence) you can grab the GITHUB_TOKEN from a subsequent non-fork PR build or a build on another trigger, which will have write permissions unless restricted by the repository maintainers.
- lmeyerov 3y ago
  I think 'environments' was meant, where diff GHA environments get diff secrets, and policies dictate who gets to run what actions with what envs. But that is real work to setup, audit, and maintain. It'd be better if, like phone app capabilities, the default would be no privs, any privs are explicitly granted, and if they aren't being used, the system detects that and asks if you want to remove specific ones.
- gabriel-samfia 3y ago
  Ouch! This is why ephemeral runners should be used. Preferably virtual machines. On an infrastructure that can define security group rules to prevent lateral movement.
mlazos 3y ago
If they use the same runners, couldn’t the attacker just wait? The runners would need to be sequestered too
- kevin_nisbet 3y ago
  Absolutely. The real difficulty is tests on PR are by definition remote code execution by an untrusted source, so a full risk analysis and hardening needs to be done. Here's a similar mistake on an OSS repo a company I worked for made: https://goteleport.com/blog/hack-via-pull-request/ https://goteleport.com/blog/hack-via-pull-request/
LtWorf 3y ago
> Not rocket science, people. The smugness and overconfidence of someone who's about to be pwned?
- trevyn 3y ago
  Pwn me.

snnn 3y ago

Ideally the builds for external PRs should not gain any access to any secret. But it is not practical. For example, you may want to use docker containers. Then you will need to download docker images from somewhere. For example, downloading ubuntu:20.04 from docker hub. That one requires a token, otherwise your request may get throttled. Even accessing most Github services requires a token. I agree with you that these things require a lot of care. In reality most dev teams are not willing to put enough time on securing their build system. That's why supply chain attacks are so common today.

LtWorf 3y ago

> These "need" privileges to upload the resulting artifacts somewhere. Only because pypi removed support for gpg and their only "signature" now basically is "was uploaded from a gh action". Otherwise a maintainer could do it offline without needing to give microsoft total access to uploading to pypi.

Bognar 3y ago
If you're using GHA to publish then you still need a trusted branch to provide secrets to. If you're not publishing using CI, then you can still upload to PyPI manually with an API Token. > without needing to give microsoft total access to uploading to pypi I assume you're referring to Trusted Publishers here? It's a per-project configuration using the industry standard of OIDC that you don't have to opt in to, so "total access" is a silly characterization. Also if you're insinuating that MS is going to generate fraudulent OIDC tokens to compromise a PyPI package, then you might want to start weaning yourself off the kool-aid.
- LtWorf 3y ago
  > If you're using GHA to publish then you still need a trusted branch to provide secrets to The vulnerability was exactly that the secrets of the trusted branch can get leaked :) > you can still upload to PyPI manually with an API Token I can, BUT if you want to be a Trusted Publisher™ the only way is to do it via github. https://docs.pypi.org/trusted-publishers/ https://docs.pypi.org/trusted-publishers/ Most likely the plan is to make it compulsory for all projects eventually, just like they made 2fa compulsory. So less secure is considered as MORE secure by pypi :) Which is consistent with the idea that no PGP signature is more secure than signed uploads. Or the idea that a global token in a clear text file is somehow safer than a password that gets typed every time.