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

Generate SLSA provenance for released binaries #9702

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

laurentsimon
Copy link

@laurentsimon laurentsimon commented Aug 1, 2022

Hi,

I'm reaching out on behalf of the Open Source Security Foundation (openssf.org). We work on improving the security of critical open source projects like yours.

Together with GitHub, we designed a free, easy-to-use method of code signing. It will help your users verify that your release binaries were built from your repository’s workflow and not altered by anyone. It’s just a few lines of code, but it will make your project more secure against third-party tampering and attacks like Codecov and CTX.

This PR shows how to add this seamless code signing to your workflow. You don’t have to be a cryptography expert or learn complicated tools and verification is simple for your users.

You can read more on the SLSA blog. Please reach out if you have any questions!

@@ -30,14 +32,50 @@ jobs:
run: make GIT_TAG=${{ github.event.inputs.tag }} -f builder.Makefile cross

- name: Compute checksums
run: cd bin; for f in *; do shasum --binary --algorithm 256 $f | tee -a checksums.txt > $f.sha256; done
working-directory: bin/
Copy link
Author

Choose a reason for hiding this comment

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

note: this change is not needed. I added working-directory to avoid the cd bin, but I can revert.

@laurentsimon
Copy link
Author

friendly ping. Are there any question I could answer?

Copy link
Member

@nicksieger nicksieger left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for setting this up.

.github/workflows/release.yaml Show resolved Hide resolved
- uses: actions/download-artifact@v3
with:
name: "${{ needs.provenance.outputs.attestation-name }}"
# Upload to release.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Upload to release.
# Verify binaries if dry run
- name: Verify binaries
if: 'inputs.dry-run'
run: |
echo TODO: verify with slsa-verifier
# Upload to release.

Copy link
Member

Choose a reason for hiding this comment

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

I guess we need to put in the actual steps here. Don't suppose you have a packaged action for verifying yet?

Copy link
Author

@laurentsimon laurentsimon Aug 9, 2022

Choose a reason for hiding this comment

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

Not yet, but we are working on it.
Would you like me to add the steps in this PR or do that in a follow-up PR?
We have 2 options:

  • go install [email protected]. This will build from source. Takes about 2-3mn for the verifier to build
  • pull the binary from our release. This would allow simulating what users do, ie using the latest version of the verifier. (Note: we can verify the hash of the pulled binary)

Let me know what you prefer.

Copy link
Author

Choose a reason for hiding this comment

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

The cheapest option is to use the pre-compiled binary. But if you don't mind 2-3mn to build the verifier, it's the simplest option.

Copy link
Author

Choose a reason for hiding this comment

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

I added the code for verification. PTAL

I also created a tracking issue slsa-framework/slsa-verifier#206 on our repo as an AI to send a PR to update your workflow when we have the GHA released.

.github/workflows/release.yaml Show resolved Hide resolved
@laurentsimon
Copy link
Author

Thanks @nicksieger !

name: "${{ needs.provenance.outputs.attestation-name }}"
# Verify binaries if dry run
- name: Verify binaries
if: 'inputs.dry-run'
Copy link

Choose a reason for hiding this comment

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

why only on dry-run?

Copy link

Choose a reason for hiding this comment

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

it seems like SLSA provenance is generated either way

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't only have to be on a dry run. On a real release it's theoretically extra work, but might still be nice to make sure the attestation loop is complete?

Copy link
Author

Choose a reason for hiding this comment

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

I've made the changes so that the verification happens in any both cases.

laurentsimon and others added 19 commits August 11, 2022 01:08
Signed-off-by: laurentsimon <[email protected]>
Co-authored-by: Nick Sieger <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Co-authored-by: Nick Sieger <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Co-authored-by: Nick Sieger <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Can give 1.19 a bit of time before we upgrade ;)

Signed-off-by: Milas Bowman <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
Signed-off-by: laurentsimon <[email protected]>
@laurentsimon
Copy link
Author

friendly ping for feedback.

@glours
Copy link
Contributor

glours commented Aug 12, 2022

Hello @laurentsimon

Regarding the current exchanges in your buildx PR, we're discussing internally to give you a common response quickly.
If that's fine for you, could we use the buildx PR for all the discussions?

@laurentsimon
Copy link
Author

Hello @laurentsimon

Regarding the current exchanges in your buildx PR, we're discussing internally to give you a common response quickly. If that's fine for you, could we use the buildx PR for all the discussions?

Absolutely. SGTM. Happy to answer further questions if you have any.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants