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.

Sign up for GitHub

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

Jump to bottom

chore: introduce gpg signed shasums #3168

Merged
merged 1 commit into from
Apr 28, 2022
Merged

chore: introduce gpg signed shasums #3168

merged 1 commit into from
Apr 28, 2022

Conversation

JackuB
Copy link
Contributor

@JackuB JackuB commented Apr 25, 2022 •
edited
Loading

This PR creates a new artifact containing GPG signed shasums

Sample file:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

37f27b1a0ec685e0aa67eaeca3eabeb44f0c7c0efa36859e25102c9002e45ac4  docker-mac-signed-bundle.tar.gz
f2d41ae6f54d2bce253c996a11b77a9214a48c80199d07532f2575d89f323ace  snyk-alpine
4c83eb996b62714a41a6031acaeaaca1c15cdee2181bb21f6ca4a8336cc8ba5f  snyk-fix.tgz
c4c8dbf395a56126ae65cc567fd10b778ff6c5b8f9d51d54f2884471d8ef3f15  snyk-for-docker-desktop-darwin-arm64.tar.gz
8462700d178beb9ab28f2af5822f58bbf47732bfa35d476d1de9ca308e9fd3d7  snyk-for-docker-desktop-darwin-x64.tar.gz
6bbc575eb4476ac5b0557eb8c3ac0c5ea9e8318e5de0a96d68e2bd71d8497a8b  snyk-linux-arm64
9c7fe252cd3eb454406e43deb7277572dc7382be33f65907590e004e2dad54e0  snyk-linux
0dd124bb51794e21f2554f55c4b9155637bf3227d1a9de9604645018beb3155f  snyk-macos
72869a1508d2268c764c7bcae1c43c2d564be7bdb0276f90f9ba7075a8536822  snyk-protect.tgz
ed655047a51d50d14192d585df7b8e934b4e3c695e7c2d6cdfd1cec047727694  snyk.tgz
161bc6e9084e4d671217db5eb8e1fba860dce0a9d23faf812f20e8c62f8a0900  snyk-win.exe
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEaL+8zrd5Tm/AaiBEopwy6R9LlWkFAmJmpuwACgkQopwy6R9L
lWlfBg/6A59z3V5h+08GpwhXjo2n8Adj55f+FdX0doth1Er7Eb9lFejBxuEJm4bh
Ph4XA+cHId+WXuvNovAZkiQEmZpTqaPNCb51ac2/bZoWvm5b6TBBBsAgSxVeWBV0
t5n/JmvbcN45l/DYDx7BtsNqUS0WwKT+1SgLuAyx5HhhtblI8Z0qXokZme9ZaeIr
9YMjyRltF3HSkIu/PFHpma7+WsYA6wRnWi6XaStDmezzED/qDZZ39ZCjQsxHro4d
yLHpfhpBx5F7taTiF169WwAnyuAE1r3MZStvU457lmhj/hUEYRlK2alPZ8jr7dq4
3z9cXcIW3mh+71RwUS4IvCAX5U2vhpiIdnW/9yT4bjw8Pky9LpxlcK5TnpDbYBOh
VScTzmEP50UJ5ZseLo4pGerZg7RX5wcAejItkog2hJEBggx3/z/4uNcYeKDqN7Pr
2gqswiE+U5BcFM9kxPg4fHrxl37LgwGd60Lsc7743CxuQjbKS0ADQIqGEqm5r4uU
CIIqG63Iqt1K9ibIQ1Froi/Pd2/hnV5VeWLRlfB67FH9CYS6KZRdnTlNU6elJcvr
WlXv9bFqsp0JloANt+fmdRO8xLtlB1QBfwc1Q7BWjP7xxrO97AGHDheyVS7BSUfi
r6lfO81+cjeeFZ1Zpj/2Vp/+CkmB8drQ5IHOlLiU09MnvFtwmD4=
=kQkh
-----END PGP SIGNATURE-----

Added notes on signature verification to the readme.

@JackuB JackuB force-pushed the chore/introduce-pgp-signed-shasums branch 14 times, most recently from 227c5fb to 25c28ea Compare April 25, 2022 15:06
@JackuB JackuB marked this pull request as ready for review April 25, 2022 15:10
@JackuB JackuB requested review from a team as code owners April 25, 2022 15:10
@JackuB JackuB changed the title chore: introduce pgp signed shasums chore: introduce gpg signed shasums Apr 25, 2022
ghost
ghost reviewed Apr 25, 2022
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Should we add a step in the Release job in CI to validate the signature? Maybe as part of validate-checksums.sh (could be renamed to validate-release.sh)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated

```bash
# 68BFBCCEB7794E6FC06A2044A29C32E91F4B9569 is the key belonging to code-signing@snyk.io
# Copy of this public key is also in this repository /release-scripts/snyk-code-signing-public.pgp
Copy link

@ghost ghost Apr 25, 2022 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

To avoid customers relying on our source repo and directory structure, should we upload this key to S3/CDN and link there instead?

Or even use the openpgp URL https://keys.openpgp.org/vks/v1/by-fingerprint/68BFBCCEB7794E6FC06A2044A29C32E91F4B9569

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was considering it, but introducing signing should help in case someone would tamper with data on the CDN. So that's a no on the CDN/static.snyk.io

And we are already trusting a 3rd party (openpgp.org) - they could be down, compromised or something. I wanted to introduce another place where key could be obtained. The commit with the key is signed and could be reviewed, and GitHub is trustworthy enough.

Copy link

Choose a reason for hiding this comment

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

Tying us to GitHub's endpoints has caused problems in recent past and no doubt we'll move this key in the future as "release-scripts" doesn't seem like the place for a public key. Maybe a top-level public directory would be clearer so we don't accidently move stuff during source migrations.

@JackuB
Copy link
Contributor Author

JackuB commented Apr 26, 2022

Should we add a step in the Release job in CI to validate the signature? Maybe as part of validate-checksums.sh (could be renamed to validate-release.sh)

We are already validating individual shasums and we fail if there would be anything off. So I'd say it doesn't add much.
I think extending our release checks (through post-release Smoke Tests?) would be a better approach.

PeterSchafer
PeterSchafer reviewed Apr 26, 2022
View reviewed changes
README.md Show resolved Hide resolved
release-scripts/make-binaries.sh Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Apr 26, 2022 •
edited by ghost
Loading

We are already validating individual shasums and we fail if there would be anything off. So I'd say it doesn't add much. I think extending our release checks (through post-release Smoke Tests?) would be a better approach.

The main reason we have validate-checksums in the Release job is to ensure the transfer of files between CircleCI jobs via workspaces (which now includes the sig file) hasn't corrupted anything before release. So I think it will add confidence in our pre-release process. Though, yes, it's minor and only useful when CircleCI has a bug (version and release.json aren't validated at that point either).

@JackuB JackuB force-pushed the chore/introduce-pgp-signed-shasums branch 2 times, most recently from eeb851f to f9958d8 Compare April 28, 2022 11:08
@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2022 •
edited
Loading

Warnings
⚠️

Please make changes to snyk help text in Gitbook. Changes will be automatically synchronised to Snyk CLI as a scheduled PR.
For more information, see: help/README.md.

Generated by 🚫 dangerJS against 82de6b1

@JackuB JackuB force-pushed the chore/introduce-pgp-signed-shasums branch from f9958d8 to 96b1b73 Compare April 28, 2022 11:15
@JackuB
Copy link
Contributor Author

JackuB commented Apr 28, 2022 •
edited
Loading

Updated the commit. Moved secrets to a PGP-related context. Moved the public key to the help directory https://github.com/snyk/cli/tree/chore/introduce-pgp-signed-shasums/help/_about-this-project#about-snyk-cli-documenting-design-decisions and added a pgp signature validation step

ghost
ghost approved these changes Apr 28, 2022
View reviewed changes
release-scripts/validate-checksums.sh Outdated Show resolved Hide resolved
@JackuB
chore: introduce gpg signed shasums
82de6b1 
Signed-off-by: Jakub Mikulas <jakub@snyk.io>
@JackuB JackuB force-pushed the chore/introduce-pgp-signed-shasums branch from 96b1b73 to 82de6b1 Compare April 28, 2022 13:34
@JackuB JackuB enabled auto-merge April 28, 2022 14:27
@JackuB
Copy link
Contributor Author

JackuB commented Apr 28, 2022

Reopening for cla bot

@JackuB JackuB closed this Apr 28, 2022
auto-merge was automatically disabled April 28, 2022 14:38

Pull request was closed

@JackuB JackuB reopened this Apr 28, 2022
@JackuB JackuB enabled auto-merge April 28, 2022 14:39
@JackuB JackuB merged commit 44212b3 into master Apr 28, 2022
@JackuB JackuB deleted the chore/introduce-pgp-signed-shasums branch April 28, 2022 14:40
@EitanGayor EitanGayor mentioned this pull request Jun 29, 2022
Open
@romathur romathur mentioned this pull request Jun 30, 2022
Open
@joshje joshje mentioned this pull request Jun 30, 2022
Open
@RyanPHigh RyanPHigh mentioned this pull request Jun 30, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PeterSchafer PeterSchafer PeterSchafer approved these changes

@jonathansantilli jonathansantilli Awaiting requested review from jonathansantilli jonathansantilli was automatically assigned from snyk/snyk-open-source

@admons admons Awaiting requested review from admons admons was automatically assigned from snyk/snyk-open-source

@magdziarek magdziarek Awaiting requested review from magdziarek magdziarek was automatically assigned from snyk/snyk-open-source

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@JackuB @PeterSchafer