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: sign checksum using cosign #498

Closed
wants to merge 1 commit into from

Conversation

Dentrax
Copy link

@Dentrax Dentrax commented Nov 10, 2021

Signed-off-by: Furkan [email protected]
Signed-off-by: Batuhan Apaydın [email protected]

Fixes #491

@google-cla google-cla bot added the cla: yes label Nov 10, 2021
@Dentrax
Copy link
Author

Dentrax commented Nov 10, 2021

We used cosign-release: 'v1.3.0' here, but we can not do verify for now since we have to wait the next cosign release. Here is the related PR: sigstore/cosign#991 by @shibumi. This feature scheduled in the next v1.4.0 release. If we want to export the cert in the release, then probably we have to wait one of these PRs to get merged: sigstore/cosign#1016 and sigstore/cosign#1021 by @developer-guy

.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}"]
Copy link

Choose a reason for hiding this comment

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

This part is missing the signature: ${artifact}.sig value.. also it might makes sense to wait for the goreleaser support for releasing certificates, too :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link

Choose a reason for hiding this comment

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

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)

@google-cla
Copy link

google-cla bot commented Nov 11, 2021

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 @googlebot I consent. in this pull request.

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 cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

.goreleaser.yml Outdated Show resolved Hide resolved
@developer-guy
Copy link
Collaborator

@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 🤝

@shibumi
Copy link

shibumi commented Dec 11, 2021

@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

.github/workflows/release.yml Outdated Show resolved Hide resolved
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
Copy link

Choose a reason for hiding this comment

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

Do you really only want to sign the checksums?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@developer-guy developer-guy force-pushed the feature/sign-checksum branch 7 times, most recently from 51b042e to 6cad12e Compare December 21, 2021 08:38
@developer-guy
Copy link
Collaborator

kindly ping @mattmoor @imjasonh @shibumi, seems everything works fine.
👉 https://github.com/developer-guy/ko/releases/tag/v0.9.3-signchecksum

$ 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

.goreleaser.yml Show resolved Hide resolved
with:
go-version: 1.17.x
- uses: goreleaser/goreleaser-action@v1
- uses: sigstore/[email protected]
- uses: goreleaser/goreleaser-action@v2
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a step after goreleaser that runs cosign verify <checksum> to check that this worked as expected?

Copy link

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice let's do this.

@shibumi
Copy link

shibumi commented Dec 21, 2021

@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
Copy link
Collaborator

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?

@developer-guy
Copy link
Collaborator

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🙋🏻‍♂️

@imjasonh
Copy link
Member

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 ko and ko's SBOM of ko don't disagree. 🙃

@developer-guy
Copy link
Collaborator

I removed the --oidc-issuer flag to enable ambient credential detection support in cosign

@shibumi
Copy link

shibumi commented Dec 23, 2021

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?

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.

@developer-guy
Copy link
Collaborator

kindly ping @imjasonh 🙋🏻‍♂️

@github-actions
Copy link

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

feat: prepare for the new releases
Signed-off-by: Furkan <[email protected]>
Signed-off-by: Batuhan Apaydın <[email protected]>
@developer-guy
Copy link
Collaborator

kindly ping @imjasonh

@Dentrax
Copy link
Author

Dentrax commented May 10, 2022

Rebased, updated. Test release: https://github.com/Dentrax/ko/releases/tag/v0.0.9

@developer-guy
Copy link
Collaborator

kindly ping @imjasonh

Copy link
Member

@imjasonh imjasonh left a comment

Choose a reason for hiding this comment

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

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
Copy link
Member

Choose a reason for hiding this comment

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

Just to confirm: This needs contents: write to be able to add things to the release right?

@imjasonh
Copy link
Member

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.

@imjasonh imjasonh closed this Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sign released ko binaries
4 participants