Add SLSA provenance generator to release; closes #222 #223

diogoteles08 · 2022-09-26T17:48:56Z

Summary

This pull request adds a provenance generator to the Github Action build workflow. It enhances the security of the end-user by allowing them to verify the integrity of the artifact they are consuming. Closes #222.

Also, this PRs divides the build workflow into different jobs, each of them with their specific permissions.

IMPORTANT: The new build flow was not able to be fully tested because I did not have the credentials to publish on PYPI and also could not upload the release on Github because I did not added tags. But until this point of the build, everything worked out fine.

Release Note

Documentation

I also changed the README to instruct the users on how to use the provenance to verify their artifacts.

…issions Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

…ild provenance Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

This reverts commit 88583f0.

di · 2022-09-26T17:53:11Z

/gcbrun

woodruffw

LGTM. I can try doing a pre-release to shake things out once this is merged!

diogoteles08 · 2022-09-26T19:12:56Z

Great! Should I squash commits for you?

di · 2022-09-26T19:19:08Z

Not necessary, thanks!

di · 2022-09-26T19:21:50Z

.github/workflows/release.yml

+      contents: write # To add assets to a release.
+    uses: slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml@v1.2.0
+    with:
+      attestation-name: sigstore-python-provenance.intoto.jsonl


This seems arbitrary. Is it possible to make this based on the name of the artifact that the provenance is generated for? Also, how does this handle multiple artifacts? Are they all bundled in the same file?

E.g., could we make the provenance for sigstore-0.6.5.tar.gz be sigstore-0.6.5.tar.gz.intoto.jsonl, and so on?

Based on my local testings, you can use that unique provenance file to verify both artifacts. That's why I used an "arbitrary" name

We could indeed separate the provenances, but AFAIK for that I would need to create and output two different hashes and then call the slsa-github-generator twice, which seems a bit redundant

I'm personally okay with them being bundled together, although I also have a slight preference for the attestation filename having the version number in it somewhere (that way, someone who wants to pull down every attestation we've ever generated doesn't need to do the renaming on their own).

Perhaps this is better as a feature request upstream for the generator. I think in the Python world, it makes more sense to have provenance be per-artifact instead.

I understand. I'll talk to someone that I know who works on SLSA project and ask if they have this feature in mind and possibly raise the issue.
In the meanwhile, I'll try to change the code to automatically fetch the release version and create the provenance with the name provenance-sigstore-{version number}.intoto.jsonl. Does it sound good?

Just commited the proposed changes, please take a look

Hey, @laurentsimon from the upstream SLSA generator project.

The SLSA specs currently encourage the use of a single multiple.intoto.jsonl provenance covering all artifacts (see in-toto/attestation#107). Having a version is definitely good feedback for them, so I replied to the thread.

I think in the Python world, it makes more sense to have provenance be per-artifact instead.

Can you explain why it makes more sense?

It's not the case here, but when Python builds include native code, builds have to be run across multiple platforms and architectures. In a hypothetical future, we might have a project that needs to generate provenance from multiple discrete SLSA-compatible builders, for multiple artifacts. So, the first problem is: what we name that provenance, so at verification time, users know what to use with a given artifact?

Then, if we then want to distribute provenance along with the artifact, we have the same problem: on a repository like PyPI, we have distribution files, and then additional artifacts that directly relate to those files. So for example, if I was signing something with PGP, I would upload sigstore-0.6.5.tar.gz and sigstore-0.6.5.tar.gz.asc. I think we want to follow a similar pattern here.

Makes sense. @diogoteles08 when you get to a project like the one @di describes, let me know. I think the builds use strategy matrix, so we could handle them similar to slsa-framework/slsa-github-generator#707

README.md

On the instructions on how to verify the provenance, we were reffering to the instructions on how to do so, but linking to the wrong URL. Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

woodruffw · 2022-09-26T20:58:42Z

/gcbrun

.github/workflows/release.yml

On release-pypi job on release workflow, the token-id permission was set to "write", but it's not necessary and was removed. Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

di · 2022-09-27T19:12:35Z

/gcbrun

woodruffw

Two small formatting/typo nits; I'll merge these using the GH web flow.

README.md

Signed-off-by: William Woodruff <william@yossarian.net>

woodruffw · 2022-09-27T19:34:17Z

/gcbrun

woodruffw · 2022-09-27T19:36:03Z

.github/workflows/release.yml

+      actions: read   # To read the workflow path.
+      id-token: write # To sign the provenance.
+      contents: write # To add assets to a release.
+    uses: slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml@v1.2.0


Nonblocking: for consistency with the other actions in this workflow, we should probably use a hash rev instead of a symbolic (tag) rev.

Agreed, let's update this.

hash is not supported for the builder, due to a lack of GH support to map a hash to a branch. This is an intentional design decision we have made. We aim to change that once the relevant GitHub support is there. See the reasoning at slsa-framework/slsa-verifier#12

I agree that it would maintain the consistency and also follow the best practice rules, but currently SLSA generator does not support hash pinning, although there already is an issue about that. More details here

However I think it would be a good idea to add a comment on code explaining this decision, because it's indeed contradictory and more people can question that. Will work on it

+1, with a link to the documentation. We can add a link to the tracking issue in the documentation of our repo.

Because of a demand of SLSA Generator, their action cannot be used through pinned hashing. As using tags goes agains the best practices, I'm letting explicit the reason why we are using them. Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

di · 2022-09-29T14:39:54Z

/gcbrun

…sigstore#223)" This reverts commit e5acd69.

…#232) This reverts commit e5acd69.

…tore#222 (sigstore#223)" (sigstore#232)" This reverts commit 2e438cd.

* Revert "Revert "Add SLSA provenance generator to release; closes #222 (#223)" (#232)" This reverts commit 2e438cd. * Bump generator workflow version for new release

diogoteles08 added 7 commits September 26, 2022 13:47

feat: split release workflows into different jobs with different perm…

50c7f71

…issions Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

feat: add workflow_dispatch trigger for release action

88583f0

Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

feat: add provenance generation to release workflow

9926aa6

Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

fix: wrong line breaking on sign step of release

ba05380

Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

docs: update README to add SLSA 3 badge and instructions to verify bu…

ef17cc5

…ild provenance Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

fix: issue downloading artifacts for release on pypi

055231a

Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

Revert "feat: add workflow_dispatch trigger for release action"

a271a40

This reverts commit 88583f0.

diogoteles08 mentioned this pull request Sep 26, 2022

Integration with SLSA security framework #222

Closed

di requested a review from woodruffw September 26, 2022 17:53

woodruffw previously approved these changes Sep 26, 2022

View reviewed changes

di reviewed Sep 26, 2022

View reviewed changes

fix: wrong URL being reffered on README

beee99d

On the instructions on how to verify the provenance, we were reffering to the instructions on how to do so, but linking to the wrong URL. Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

woodruffw dismissed their stale review via beee99d September 26, 2022 20:10

feat: change provenance name to have the version number

009ac66

Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

laurentsimon mentioned this pull request Sep 26, 2022

Need better clarity on bundle filename for chains of attestations in-toto/attestation#107

Closed

Merge branch 'main' into main

ede0539

jku reviewed Sep 27, 2022

View reviewed changes

.github/workflows/release.yml Outdated Show resolved Hide resolved

feat: remove unnecessary permission on job workflow

3c59263

On release-pypi job on release workflow, the token-id permission was set to "write", but it's not necessary and was removed. Signed-off-by: Diogo Teles Sant'Anna <diogoteles08@gmail.com>

woodruffw reviewed Sep 27, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Show resolved Hide resolved

woodruffw added 2 commits September 27, 2022 15:33

Update README.md

74f678b

Signed-off-by: William Woodruff <william@yossarian.net>

Update README.md

8fbd1d7

Signed-off-by: William Woodruff <william@yossarian.net>

woodruffw reviewed Sep 27, 2022

View reviewed changes

woodruffw previously approved these changes Sep 27, 2022

View reviewed changes

di previously approved these changes Sep 28, 2022

View reviewed changes

diogoteles08 dismissed stale reviews from di and woodruffw via 5747562 September 28, 2022 16:24

di enabled auto-merge (squash) September 29, 2022 14:40

di approved these changes Sep 29, 2022

View reviewed changes

di merged commit e5acd69 into sigstore:main Sep 29, 2022

di added a commit to di/sigstore-python that referenced this pull request Oct 5, 2022

Revert "Add SLSA provenance generator to release; closes sigstore#222 (…

0424382

…sigstore#223)" This reverts commit e5acd69.

di added a commit that referenced this pull request Oct 5, 2022

Revert "Add SLSA provenance generator to release; closes #222 (#223)" (…

2e438cd

…#232) This reverts commit e5acd69.

di added a commit to di/sigstore-python that referenced this pull request Oct 20, 2022

Revert "Revert "Add SLSA provenance generator to release; closes sigs…

8c21352

…tore#222 (sigstore#223)" (sigstore#232)" This reverts commit 2e438cd.

di mentioned this pull request Oct 20, 2022

Restore SLSA provenance generator #256

Merged

woodruffw pushed a commit that referenced this pull request Oct 20, 2022

Restore SLSA provenance generator (#256)

315b078

* Revert "Revert "Add SLSA provenance generator to release; closes #222 (#223)" (#232)" This reverts commit 2e438cd. * Bump generator workflow version for new release

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SLSA provenance generator to release; closes #222 #223

Add SLSA provenance generator to release; closes #222 #223

diogoteles08 commented Sep 26, 2022 •

edited by di

Loading

di commented Sep 26, 2022

woodruffw left a comment

diogoteles08 commented Sep 26, 2022

di commented Sep 26, 2022

di Sep 26, 2022

diogoteles08 Sep 26, 2022

diogoteles08 Sep 26, 2022

woodruffw Sep 26, 2022

di Sep 26, 2022

diogoteles08 Sep 26, 2022

diogoteles08 Sep 26, 2022

laurentsimon Sep 26, 2022

di Sep 26, 2022

laurentsimon Sep 27, 2022 •

edited

Loading

woodruffw commented Sep 26, 2022

di commented Sep 27, 2022

woodruffw left a comment

woodruffw commented Sep 27, 2022

woodruffw Sep 27, 2022

di Sep 28, 2022

laurentsimon Sep 28, 2022 •

edited

Loading

diogoteles08 Sep 28, 2022

diogoteles08 Sep 28, 2022

laurentsimon Sep 28, 2022 •

edited

Loading

di commented Sep 29, 2022

Add SLSA provenance generator to release; closes #222 #223

Add SLSA provenance generator to release; closes #222 #223

Conversation

diogoteles08 commented Sep 26, 2022 • edited by di Loading

Summary

Release Note

Documentation

di commented Sep 26, 2022

woodruffw left a comment

Choose a reason for hiding this comment

diogoteles08 commented Sep 26, 2022

di commented Sep 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurentsimon Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

woodruffw commented Sep 26, 2022

di commented Sep 27, 2022

woodruffw left a comment

Choose a reason for hiding this comment

woodruffw commented Sep 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurentsimon Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurentsimon Sep 28, 2022 • edited Loading

Choose a reason for hiding this comment

di commented Sep 29, 2022

diogoteles08 commented Sep 26, 2022 •

edited by di

Loading

laurentsimon Sep 27, 2022 •

edited

Loading

laurentsimon Sep 28, 2022 •

edited

Loading

laurentsimon Sep 28, 2022 •

edited

Loading