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

feat: Add SPEC 8 - Securing the Release Process #325

Merged
merged 18 commits into from
Jul 29, 2024
Merged
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
225 changes: 225 additions & 0 deletions spec-0008/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
# SPEC-8: Securing the Release Process

lagru marked this conversation as resolved.
Show resolved Hide resolved
---
title: "SPEC 8 — Securing the Release Process"
number: 8
date: 2024-06-04
author:
- "Matthew Feickert <[email protected]>"
- "Pamphile Roy <[email protected]>"
- "Juanita Gomez <[email protected]>"
- "Seth Larson <[email protected]>"
- "Lars Grüter <[email protected]>"
discussion: https://discuss.scientific-python.org/t/spec-8-supply-chain-security
endorsed-by:
---

## Description

<!--
Briefly and clearly describe the recommendation.
https://github.com/scientific-python/summit-2024/issues/9
-->

Open source libraries constitute a significant portion of the world's digital infrastructure. Securing the Open Source supply chain (OSSC) is therefore an increasing concern, with examples of sophisticated attacks against the ecosystem (e.g., the 2024 [`xz` utils backdoor](https://en.wikipedia.org/wiki/XZ_Utils_backdoor)) and [malware attacks on PyPI](https://blog.pypi.org/posts/2024-04-10-domain-abuse/) highlighting the need for supply chain security to be taken seriously.
The Python Software Foundation (PSF) is also taking the importance of the OSSC seriously, as demonstrated by the [creation of the PSF Security Developer in Residence position in 2023](https://pyfound.blogspot.com/2023/06/announcing-our-new-security-developer.html).

With the [Supply-chain Levels for Software Artifacts (SLSA) framework](https://slsa.dev/) and [OpenID Connect (OIDC) standard](https://openid.net/developers/how-connect-works/) being widely adopted, several high level developer tools, maintained by professional security teams, have been created with clear recommendations on how to use them.

This SPEC outlines pragmatic recommendations for adopting these security tools and recommendations on how to publish release artifacts securely.
Securely _building_ release artifacts will be covered in a later SPEC. This set of recommendations complements the recommendations from [SPEC 6 — Keys to the Castle](https://github.com/scientific-python/specs/blob/main/spec-0006/index.md).

While this SPEC is written with GitHub in mind, the same recommendations apply to other services, such as GitLab.

## Implementation

With a focus on securing the release artifact distribution process, the following processes and standards should be adopted.

### Document the release process

The release process should be clearly and fully documented in the developer documentation and describe each step to make a release and the permissions required to do so.
tupui marked this conversation as resolved.
Show resolved Hide resolved
It is recommended that this is a dedicated page in the developer section of the documentation website, though providing instructions in a `RELEASING.md` in the top level of the repository is also a common approach.

### Hardening workflow environment permissions
matthewfeickert marked this conversation as resolved.
Show resolved Hide resolved

* Workflows that publish release artifacts should have _run triggers_ that require intentional actions by the release team (e.g., `workflow_dispatch` in GitHub Actions) and require multiple the release team members to approve the workflow to run (c.f. "Use GitHub Actions environments" section below).
matthewfeickert marked this conversation as resolved.
Show resolved Hide resolved
This is to safeguard the project from any one maintainer having the ability to commit to the default branch and make a release directly.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confused; wouldn't most projects want the release manager have this ability? Also, instead of using technical jargon, perhaps the principle can be described in more generic terms first. E.g., if you want to highlight that one maintainer should not be able to make a release by themselves, then that sentence can go first.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the thing, you don't need a release manager (from the artifact's standpoint) if the whole process is automated. What you need is multiple maintainers to approve the release flow to run.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stefanv (Asking really because I don't know) In your experience is the expectation that a release manager is the only person who should be involved in a release (and that they don't coordinate with the other project maintainers)?

I agree that a release manager is going to be the one clicking the button, but the recommendation is to make sure that multiple maintainers are on the same page that a release should be cut and that it is happening from the correct branch. You could imagine that the release manager is actually the person that is the second approver here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I practice what ensures that there is enough maintainers around to not just blindly push the button? E.g. in practice bigger projects are struggling to have multiple release managers, so if cutting a release requires coordinated effort of multiple individuals, that may not make things better or safer (I already see cases where PR approvals are given without a careful review, or even PR related CI failures)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The staging area is useful and nice, but the "any couple of maintainers can push the button" is certainly not universal. Thinking about how that would work for NumPy or SciPy: it'd be both more cumbersome and significantly less secure. We now have 5 people with access to PyPI, and 30-40 people with commit rights, and the bar to gain PyPI access is way higher than that to gain commit rights.

I think that that's true for a majority of projects. Giving out commit rights to promising new contributors fairly quickly can be good to engage them; having those be the keys to PyPI as well would make the bar to giving out commit rights higher, or the setup less secure. (requiring 2 maintainers to hit the button only helps a little, since it's not hard for a bad actor to start contributing under 2 different github handles).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And as someone being a release manager for many years for a large package, I would definitely argue that it is not at all easy to get careful review/sign-off from multiple people even when the release cycle is on the 6 month timescale.
Checking on the release notes is not at all equals with checking off on everything required for a release, and if that is not required for ticking that box, then I would argue it's not at all a safer process.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's safer as in, now to make a release, we need to compromise 2 people if we want to inject something at the last minute. But sure, if the people you would allow to release would not even bother looking at the last commits... then it won't do much.

I still think that a release should be considered as more critical than a PR, hence require a more careful review.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At minimum, I would like to see changing all usage of maintainers in this document to be release team, as that really throws the discussion off track. As my experience just doesn't overlap on how difficult it is to get a release team together and even just alternating releases within them, let alone requiring multiple of them doing all the checks and work for all the releases. I agree it would be nice, but I don't think it's realistic for any of the projects I know, neither big nor small.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After talking with @bsipocz IRL I've added "release team" as an adjective to member/maintainer to make it clear who we are talking about.

Also there was some confusion about the number of people: To make it explicit, we are saying 1 additional person review (so 2 people total -> "4 eyes").

👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap, not using the word maintainer but a much smaller group as the "release team" addresses my main concern. Also 4 vs 2 is now clear, maybe due to having more coffee or having more discussions about it...


* It is also strongly recommended that the repository requires [signed commits](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits) so that each release corresponds to a verified commit.
* The branch from which the release is made should also be protected.

#### Restrict permissions in CI runners to the minimum required

To restrict the attack surface area of arbitrary code execution in CI runners, the _default_ runner permissions should be restricted to the minimum possible (read access). In the GitHub Action workflow, this is accomplished by defining the following workflow global permissions block before any jobs are defined.

```yaml
permissions:
contents: read
```

Elevating permissions beyond this should be done at the job level by redefining the permissions block in the job.

#### Restrict permitted actions in workflows

GitHub allows restricting the actions that workflows can use via the repository actions permissions settings at `https://github.com/$ORG/$PROJECT/settings/actions`.
A reasonable default is to select the

* Allow $ORG, and select non-$ORG, actions and reusable workflows

option and the suboptions:

* Allow actions created by GitHub
* Allow specified actions and reusable workflows

Consult [Managing GitHub Actions permissions for your repository](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#managing-github-actions-permissions-for-your-repository) for more details.

#### Use GitHub Actions environments

Use a [GitHub Actions environment](https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment)

```yaml
environment:
name: publish-package
```

and enforce additional review by at least one other release team maintainer to run a GitHub Actions workflow that publishes to PyPI.
Additional reviewer requirements can be configured per GitHub Actions environment under `https://github.com/$ORG/$PROJECT/settings/environments/` in the "Deployment protection rules" section.

![github-actions-environment](https://hackmd.io/_uploads/S1SErQ0EC.png)

### Pin GitHub Actions release workflows to their full release commit SHAs

GitHub actions must be pinned using full commit SHA corresponding to the release version being used.
Using versions or small hashes is susceptible to attacks.

```yaml
- uses: actions/some-action@1fe14e04876783b259436247a3898d2fe7d5548f #vX.Y.Z
```

Dependabot can be used to automatically update the hashes.
It is important that this happens as part of a reviewed process.

```yaml!
# .github/dependabot.yml
version: 2
updates:
# Maintain dependencies for GitHub Actions
- package-ecosystem: "github-actions"
directory: "/"
schedule:
interval: "monthly"
groups:
actions:
patterns:
- "*"
```

### Adopt SLSA through use of GitHub Attestations

A component of SLSA is [software attestation](https://slsa.dev/attestation-model) which allows for public validation of software artifacts and provenance.
GitHub provides the [`actions/attest-build-provenance`](https://github.com/actions/attest-build-provenance) GitHub Action which implements SLSA to generate signed build provenance attestations for workflow artifacts.
Attestations are published to the project GitHub under `https://github.com/$ORG/$PROJECT/attestations/`.

```yaml
- uses: actions/attest-build-provenance@<full action commit SHA> # vX.Y.Z
with:
subject-path: "dist/<package name>-*"
```

GitHub has also added the [`gh attestation verify`](https://cli.github.com/manual/gh_attestation_verify) command to the GitHub CLI utility, which verifies the integrity and provenance of an artifact using its associated cryptographically signed attestations.
This can be used by individual users and also in GitHub Actions workflows where the GitHub CLI utility is installed by default.

```yaml
- name: Verify artifact attestation
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
shell: bash
run: |
for artifact in dist/*; do
echo "# ${artifact}"
gh attestation verify "${artifact}" --repo ${{ github.repository }}
done
```

### Adopt OIDC through the use of PyPI Trusted Publishers

[Trusted Publishers](https://docs.pypi.org/trusted-publishers/) provide a way to securely establish a short lived authentication token between a project repository and a distribution platform &mdash; such as PyPI.
It replaces the need to use a long lived token to authenticate, reducing the security risks associated with authentication tokens (e.g., tokens being compromised, the need to rotate tokens).

Trusted Publishers can be used in GitHub Actions by using the [`pypa/gh-action-pypi-publish`](https://github.com/pypa/gh-action-pypi-publish) GitHub Action defaults in a GitHub Actions environment.

```yaml
jobs:
publish:
name: Publish release to PyPI
runs-on: ubuntu-latest
environment: publish-package
permissions:
# IMPORTANT: this permission is mandatory for trusted publishing
id-token: write
steps:
# retrieve your distributions here
# ...

- name: Publish distribution to PyPI
uses: pypa/gh-action-pypi-publish@<full action commit SHA> # vX.Y.Z
with:
print-hash: true
```

### Example workflow

The following is a complete example of a workflow which can be used as a starting point:

```yaml
name: publish distributions

on:
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read

jobs:
publish:
name: Publish Python distribution to PyPI
runs-on: ubuntu-latest
permissions:
id-token: write
attestations: write
environment:
name: publish-package

steps:
# - name: Collect built artifacts
# ...

- name: Generate artifact attestation for sdist and wheels
uses: actions/attest-build-provenance@<full action commit SHA> # vX.Y.Z
with:
subject-path: "dist/<package name>-*"

- name: Verify artifact attestation
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
shell: bash
run: |
for artifact in dist/*; do
echo "# ${artifact}"
gh attestation verify "${artifact}" --repo ${{ github.repository }}
done

- name: Publish distribution to PyPI
uses: pypa/gh-action-pypi-publish@<full action commit SHA> # vX.Y.Z
with:
print-hash: true
```

## Notes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is out of scope for the SPEC, but how does it look from the user's perspective? Can they use the attestation to check anything? Or do they just pip/conda install and hope for the best. Does conda forge follow these practices? Any recommendations we need to make w.r.t. their release process?

Copy link
Member

@tupui tupui Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

End users can check the attestations and check the whole flow yes. It would be awesome if there could be an integration in pip checking things like that (like a --secure flag). @sethmlarson do you know if that was ever brought up?
Slightly out of scope I would say, but definitely should be part of the page we would put on the learn website.

For conda I am not sure. They don't even support (yet) basics like 2FA (just some precision: anaconda accounts, diff than conda-forge), so they have a long way ahead still...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though thinking a more about it. It should not be hard to add SLSA to the workflows and the good thing with conda-forge is that releases need a PR. So there is a clear lineage vs pip not enforcing that.

A recommendation could be to start the conda workflow by checking the SLSA attestation (if from pip, otherwise we need to check for signed commits or something else to attest the source authenticity), building for conda, then signing everything with a new SLSA.

Copy link
Member

@tupui tupui Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To verify, here is the post from @sethmlarson https://sethmlarson.dev/python-and-slsa#verifying-provenance-of-a-python-package. This is the sort of thing that I would like to see directly built into pip. This way we could ask to get some warnings.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this is out of scope for the SPEC, but how does it look from the user's perspective? Can they use the attestation to check anything? Or do they just pip/conda install and hope for the best.

I think this is out of scope to address in the SPEC itself, but it is a good question, @stefanv. I agree with everything @tupui already said but to give some an explicit example (using the gh command line too, sorry) if they download the sdist or wheel they can verify the artifact's attestation:

$ python -m pip download --no-deps scikit-image
Collecting scikit-image
  Downloading scikit_image-0.24.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (14 kB)
Downloading scikit_image-0.24.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (14.9 MB)
   ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 14.9/14.9 MB 720.6 kB/s eta 0:00:00
Saved ./scikit_image-0.24.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
Successfully downloaded scikit-image
$ gh attestation verify scikit_image-*.whl --repo scikit-image/scikit-image
Loaded digest sha256:fa27b3a0dbad807b966b8db2d78da734cb812ca4787f7fbb143764800ce2fa9c for file://scikit_image-0.24.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl
Loaded 1 attestation from GitHub API
✓ Verification succeeded!

sha256:fa27b3a0dbad807b966b8db2d78da734cb812ca4787f7fbb143764800ce2fa9c was attested by:
REPO                       PREDICATE_TYPE                  WORKFLOW                                                       
scikit-image/scikit-image  https://slsa.dev/provenance/v1  .github/workflows/wheel_tests_and_release.yml@refs/tags/v0.24.0

This is obviously not something that is nicely integrated into a security install at the moment, but I also wouldn't expect that yet as that's extra work from the pip team and attestations are somewhat new. (Maybe worth a feature request Issue to the uv team though. 🤔)

Does conda forge follow these practices? Any recommendations we need to make w.r.t. their release process?

@jakirkham has an Issue open on this somewhere (I forget where, but know that I'm tagged in it) but I think there was hesitancy on the part of Conda-forge to use a tool from GitHub and not use Sigstore directly(?). @jakirkham can link us and also give more explicit context on the conda-forge maintainer perspective.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charliermarsh is this something you thought about maybe for uv?

The quick TLDR, adding SLSA verification when installing packages.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is probably hard given that to verify current attestations you need to provide information like the repository that created the attestation. Not sure how to do this programmatically during an install.


- [Concise Guide for Developing More Secure Software from the OpenSSF](https://best.openssf.org/Concise-Guide-for-Developing-More-Secure-Software)
- [GitHub Blog: Introducing Artifact Attestations–now in public beta](https://github.blog/2024-05-02-introducing-artifact-attestations-now-in-public-beta/)