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

Adding protobuf bundle support to sign-blob and attest-blob #3752

Merged
merged 14 commits into from
Jul 23, 2024

Conversation

steiza
Copy link
Member

@steiza steiza commented Jun 28, 2024

Summary

This pull requests addresses the first part of #3139: adding protobuf bundle support for cosign sign-blob and cosign attest-blob.

You can test this by generating the new bundles, for example signing a local file with a cosign provisioned key (requesting a signed timestamp to corroborate):

go run cmd/cosign/main.go sign-blob --bundle="test-bundle.sigstore.json" --new-bundle-format=true --key="cosign.key" --tlog-upload=false --timestamp-server-url="https://timestamp.githubapp.com/api/v1/timestamp" ../sigstore-go/examples/sigstore-go-signing/hello_world.txt

Or using Fulcio to get a signing certificate for an attestation:

go run cmd/cosign/main.go attest-blob --bundle="pgi-bundle.sigstore.json" --new-bundle-format=true --fulcio-url='https://fulcio.sigstore.dev' --rekor-url='https://rekor.sigstore.dev' --identity-token='...' --type=something --predicate="../sigstore-go/examples/sigstore-go-signing/intoto.txt" ../sigstore-go/examples/sigstore-go-signing/hello_world.txt

You can then verify the public good instance bundle using sigstore-go doing something like:

go run cmd/sigstore-go/main.go -artifact examples/sigstore-go-signing/hello_world.txt -trustedrootJSONpath public-good-trusted-root.json -expectedIssuer "https://token.actions.githubusercontent.com" -expectedSANRegex ".+" ../cosign/pgi-bundle.sigstore.json

Release Note

NONE - we probably want to finish #3139 (especially the more comprehensive conformance testing!) before we announce this as released.

Documentation

N/A - same as above

Copy link

codecov bot commented Jul 1, 2024

Codecov Report

Attention: Patch coverage is 34.44976% with 137 lines in your changes missing coverage. Please review.

Project coverage is 37.67%. Comparing base (2ef6022) to head (5892fed).
Report is 166 commits behind head on main.

Files Patch % Lines
cmd/cosign/cli/attest/attest_blob.go 5.68% 82 Missing and 1 partial ⚠️
cmd/cosign/cli/sign/sign_blob.go 46.75% 34 Missing and 7 partials ⚠️
cmd/cosign/cli/options/attest_blob.go 0.00% 4 Missing ⚠️
cmd/cosign/cli/options/signblob.go 0.00% 4 Missing ⚠️
pkg/cosign/bundle/protobundle.go 91.17% 2 Missing and 1 partial ⚠️
cmd/cosign/cli/attest_blob.go 0.00% 1 Missing ⚠️
cmd/cosign/cli/signblob.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3752      +/-   ##
==========================================
- Coverage   40.10%   37.67%   -2.44%     
==========================================
  Files         155      201      +46     
  Lines       10044    12436    +2392     
==========================================
+ Hits         4028     4685     +657     
- Misses       5530     7175    +1645     
- Partials      486      576      +90     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Fantastic!! Just a few tiny comments, also needs a rebase.

cmd/cosign/cli/attest/attest_blob.go Outdated Show resolved Hide resolved
cmd/cosign/cli/attest/attest_blob.go Outdated Show resolved Hide resolved
pkg/cosign/bundle/protobundle_test.go Show resolved Hide resolved
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Great work on this!

@haydentherapper
Copy link
Contributor

@steiza can you rebase? Just to check, anything else here or any other testing, or is this good to merge?

steiza added 14 commits July 23, 2024 08:47
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
Signed-off-by: Zach Steindler <steiza@github.com>
@steiza
Copy link
Member Author

steiza commented Jul 23, 2024

Just to check, anything else here or any other testing, or is this good to merge?

I think this is good to merge!

@haydentherapper haydentherapper merged commit c6cdf1b into sigstore:main Jul 23, 2024
22 checks passed
steiza added a commit to steiza/cosign that referenced this pull request Sep 11, 2024
When adding bundles support to `attest-blob`, we sent the wrong data to
the timestamp authority to sign.

Signed-off-by: Zach Steindler <steiza@github.com>
haydentherapper pushed a commit that referenced this pull request Sep 18, 2024
…les (#3877)

* Fix bug in #3752

When adding bundles support to `attest-blob`, we sent the wrong data to
the timestamp authority to sign.

Signed-off-by: Zach Steindler <steiza@github.com>

* Only change timestamp authority signature behavior for new bundles

Also add TODO when we get to updating `cosign attest`

Signed-off-by: Zach Steindler <steiza@github.com>

* Add happy path e2e test

Signed-off-by: Zach Steindler <steiza@github.com>

---------

Signed-off-by: Zach Steindler <steiza@github.com>
dmitris pushed a commit to dmitris/cosign that referenced this pull request Oct 17, 2024
…les (sigstore#3877)

* Fix bug in sigstore#3752

When adding bundles support to `attest-blob`, we sent the wrong data to
the timestamp authority to sign.

Signed-off-by: Zach Steindler <steiza@github.com>

* Only change timestamp authority signature behavior for new bundles

Also add TODO when we get to updating `cosign attest`

Signed-off-by: Zach Steindler <steiza@github.com>

* Add happy path e2e test

Signed-off-by: Zach Steindler <steiza@github.com>

---------

Signed-off-by: Zach Steindler <steiza@github.com>
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.

3 participants