feat: sign checksum using cosign #498
Conversation
|
We used |
.goreleaser.yml
Outdated
| @@ -34,6 +34,10 @@ archives: | |||
| amd64: x86_64 | |||
| checksum: | |||
| name_template: 'checksums.txt' | |||
| signs: | |||
| - cmd: cosign | |||
| args: ["sign-blob", "--oidc-issuer=https://token.actions.githubusercontent.com", "--output=${signature}", "${artifact}"] | |||
There was a problem hiding this comment.
This part is missing the signature: ${artifact}.sig value.. also it might makes sense to wait for the goreleaser support for releasing certificates, too :)
There was a problem hiding this comment.
IMHO we don't need this one because we're using artifacts: checksum, so it creates checksums.txt.sig by default, as you can see from here. Also, we don't need to wait to release the certificates feature because in cosign with sigstore/cosign#991, we can verify blobs in keyless mode. The problem here is that these certificates are short-lived IIRC, so for users who might want to use these to verify the ko binary might be problematic because these certificates will be expired in a short time, WDYT @shibumi @imjasonh?
There was a problem hiding this comment.
Just for the record (@developer-guy know this already). Short-Lived certificates are not problematic, because it just refers to the signature generation, not the signature verification. See also: goreleaser/goreleaser#2659 (comment)
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
developer-guy
force-pushed
the
feature/sign-checksum
branch
from
06fb65d
to
71dbea1
Compare
|
@shibumi cosign v1.4.1 with some bunch of fixes is released today as you might know, would you like to give this issue a hand? 🤩 Or I can do it if you want if you don't have time 🤝 |
|
@developer-guy I can confirm this works: https://github.com/shibumi/kubectl-htpasswd/releases/tag/v0.1.7 But you must set the experimental flag with cosign v1.4.1 |
| certificate: '{{ trimsuffix .Env.artifact ".txt" }}.pem' | ||
| signature: "${artifact}.sig" | ||
| args: ["sign-blob", "--oidc-issuer=https://token.actions.githubusercontent.com", "--output-signature=${signature}", "--output-certificate=${certificate}", "${artifact}"] | ||
| artifacts: checksum |
There was a problem hiding this comment.
Do you really only want to sign the checksums?
There was a problem hiding this comment.
developer-guy
force-pushed
the
feature/sign-checksum
branch
7 times, most recently
from
51b042e
to
6cad12e
Compare
|
kindly ping @mattmoor @imjasonh @shibumi, seems everything works fine. $ httpie --download https://github.com/developer-guy/ko/releases/download/v0.9.3-signchecksum/checksums.txt.sig
$ httpie --download https://github.com/developer-guy/ko/releases/download/v0.9.3-signchecksum/checksums.txt.pem
$ httpie --download https://github.com/developer-guy/ko/releases/download/v0.9.3-signchecksum/checksums.txt
$ cosign verify-blob --signature checksums.txt.sig --cert checksums.txt.pem checksums.txt
Verified OK👉 https://github.com/developer-guy/ko/runs/4592518036?check_suite_focus=true |
.github/workflows/release.yml
Outdated
| with: | ||
| go-version: 1.17.x | ||
| - uses: goreleaser/goreleaser-action@v1 | ||
| - uses: sigstore/cosign-installer@v1.4.1 | ||
| - uses: goreleaser/goreleaser-action@v2 |
There was a problem hiding this comment.
Could we add a step after goreleaser that runs cosign verify <checksum> to check that this worked as expected?
There was a problem hiding this comment.
Question: Is cosign validating this already? I like the idea of validating the signature after signature creation, but I am not sure if validating makes sense if cosign is validating it already during creation.
There was a problem hiding this comment.
nice let's do this.
|
@Dentrax can you add these two lines to the goreleaser configuration? They ensure that goreleaser will produce a source tarball and signs this tarball as well. It provides an easy way to download a signed tarball via curl source:
enabled: true |
developer-guy
force-pushed
the
feature/sign-checksum
branch
from
6cad12e
to
0ee1a85
Compare
|
we only sign the chekcsums.txt file right now, I think it is enough for verifying sha256 of the files has not been tampered with but, the final decision belongs to @imjasonh, let's ask that to him and wait for his response, then according to his response, maybe we can start signing tar files instead of just checksums.txt file. WDYT @shibumi? |
|
Btw @imjasonh GoReleaser is now capable of generating SBOMs by using the Syft tool under the hood, so, that we can add that support to the ko project🙋🏻♂️ |
I'm not opposed, but I think that's a separate issue. I'd like to make sure that Syft's SBOM of |
developer-guy
force-pushed
the
feature/sign-checksum
branch
from
0ee1a85
to
6da18da
Compare
|
I removed the --oidc-issuer flag to enable ambient credential detection support in cosign |
You misunderstood me. The problem is: If you do not generate source tarballs with goreleaser, goreleaser will not create a source tarball for the project. Instead, Github will create the source tarball, but there will be no checksum file for the source tarball. |
|
kindly ping @imjasonh 🙋🏻♂️ |
|
This Pull Request is stale because it has been open for 90 days with |
Dentrax
force-pushed
the
feature/sign-checksum
branch
from
6da18da
to
6a357fb
Compare
feat: prepare for the new releases Signed-off-by: Furkan <furkan.turkal@trendyol.com> Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
Dentrax
force-pushed
the
feature/sign-checksum
branch
from
6a357fb
to
4a0f1af
Compare
|
kindly ping @imjasonh |
|
Rebased, updated. Test release: https://github.com/Dentrax/ko/releases/tag/v0.0.9 |
|
kindly ping @imjasonh |
There was a problem hiding this comment.
Sorry it's taken so long to get back to this. 😞
Some time shortly before or after we cut a ko release we should include some docs about how to verify the releases, and what verifying proves. But I don't think we should add docs to README.md yet that won't be useful for current releases.
| @@ -7,6 +7,9 @@ on: | |||
|
|
|||
| jobs: | |||
| goreleaser: | |||
| permissions: | |||
| id-token: write | |||
| contents: write | |||
There was a problem hiding this comment.
Just to confirm: This needs contents: write to be able to add things to the release right?
|
I think with #730 we have a fair assurance that the artifacts that we've released have come from reliable sources and are tamper-evident. I'll close this unless folks think there's more we should do to gain more assurance. |
Signed-off-by: Furkan furkan.turkal@trendyol.com
Signed-off-by: Batuhan Apaydın batuhan.apaydin@trendyol.com
Fixes #491