Refactor verifyNewBundle into library function #4013

codysoyland · 2025-01-14T19:35:38Z

Summary

This is a refactor of the bundle verification logic that replaces verifyNewBundle with a re-written library function cosign.VerifyNewBundle, which accepts a simplified set of arguments, using CheckOpts to supply the majority of the configuration. It has been moved out of the cli package into the cosign package and made public.

This is partially extracted from #3889.

Release Note

Documentation

codecov · 2025-01-14T19:37:50Z

Codecov Report

Attention: Patch coverage is 48.57143% with 108 lines in your changes missing coverage. Please review.

Project coverage is 36.84%. Comparing base (2ef6022) to head (cf47c49).
Report is 299 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/cosign/cli/verify/verify_blob.go	38.37%	43 Missing and 10 partials ⚠️
cmd/cosign/cli/verify/verify_blob_attestation.go	39.28%	28 Missing and 6 partials ⚠️
pkg/cosign/verify.go	74.13%	10 Missing and 5 partials ⚠️
pkg/cosign/verify_bundle.go	40.00%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4013      +/-   ##
==========================================
- Coverage   40.10%   36.84%   -3.26%     
==========================================
  Files         155      210      +55     
  Lines       10044    13356    +3312     
==========================================
+ Hits         4028     4921     +893     
- Misses       5530     7817    +2287     
- Partials      486      618     +132

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

codysoyland · 2025-01-23T18:24:05Z

@haydentherapper @cmurphy @steiza I would appreciate a look at this whenever you have a moment!

cmd/cosign/cli/verify/verify_blob.go

cmd/cosign/cli/verify/verify_blob_attestation.go

steiza

Nice work! I don't have any further comments other than the things already raised by @malancas.

cmd/cosign/cli/verify/verify_blob_attestation.go

codysoyland · 2025-01-24T22:01:48Z

I will take a look at the conformance failures Monday. Thanks all!

malancas

LGTM 🚀

haydentherapper · 2025-02-04T21:36:37Z

Needs a rebase to fix the tests that didn't run

Signed-off-by: Cody Soyland <codysoyland@github.com>

This var has a default value so shouldn't be checked Signed-off-by: Cody Soyland <codysoyland@github.com>

These tests are incorrect: they set the signature field which is not allowed when doing bundle verification. Previously they were passing due to logic errors. Signed-off-by: Cody Soyland <codysoyland@github.com>

cmurphy

Generally lgtm, couple minor questions and a typo

pkg/cosign/verify.go

Co-authored-by: Colleen Murphy <cmurphy@users.noreply.github.com> Signed-off-by: Cody Soyland <codysoyland@github.com>

haydentherapper

few small nits, looks great!

haydentherapper · 2025-02-07T01:03:57Z

cmd/cosign/cli/verify/verify_bundle.go

-
-	return sev.Verify(bundle, verify.NewPolicy(artifactOpt, identityPolicies...))
-}
-
 func AssembleNewBundle(ctx context.Context, sigBytes, signedTimestamp []byte, envelope *dsse.Envelope, artifactRef string, cert *x509.Certificate, ignoreTlog bool, sigVerifier signature.Verifier, pkOpts []signature.PublicKeyOption, rekorClient *client.Rekor) (*sgbundle.Bundle, error) {


For a later PR, we should move this to somewhere under pkg/ rather than be under the cli package. It looks like this is only used by the cosign bundle command.

cmd/cosign/cli/verify/verify_blob_attestation.go

cmd/cosign/cli/verify/verify_blob.go

Signed-off-by: Cody Soyland <codysoyland@github.com>

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cosign](https://github.com/sigstore/cosign) | patch | `2.4.2` -> `2.4.3` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>sigstore/cosign (cosign)</summary> ### [`v2.4.3`](https://github.com/sigstore/cosign/blob/HEAD/CHANGELOG.md#v243) [Compare Source](sigstore/cosign@v2.4.2...v2.4.3) #### Features - Bump sigstore/sigstore to support KMS plugins ([#4073](sigstore/cosign#4073)) - Enable fetching signatures without remote get. ([#4047](sigstore/cosign#4047)) - Feat/file flag completion improvements ([#4028](sigstore/cosign#4028)) - Update builder to use go1.23.6 ([#4052](sigstore/cosign#4052)) #### Bug Fixes - fix parsing error in --only for cosign copy ([#4049](sigstore/cosign#4049)) #### Cleanup - Refactor verifyNewBundle into library function ([#4013](sigstore/cosign#4013)) - fix comment typo and imports order ([#4061](sigstore/cosign#4061)) - sync comment with parameter name in function signature ([#4063](sigstore/cosign#4063)) - sort properly Go imports ([#4071](sigstore/cosign#4071)) #### Contributors - Bob Callaway - Carlos Tadeu Panato Junior - Cody Soyland - Dmitry Savintsev - Hayden B - Tomasz Janiszewski - Ville Skyttä </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

codysoyland marked this pull request as ready for review January 21, 2025 21:06

codysoyland changed the title ~~[DRAFT] Refactor verifyNewBundle into library function~~ Refactor verifyNewBundle into library function Jan 21, 2025

codysoyland mentioned this pull request Jan 22, 2025

Add support for new bundle specification for attesting/verifying OCI image attestations #3889

Merged

malancas reviewed Jan 23, 2025

View reviewed changes

cmd/cosign/cli/verify/verify_blob.go Outdated Show resolved Hide resolved

cmd/cosign/cli/verify/verify_blob_attestation.go Show resolved Hide resolved

cmd/cosign/cli/verify/verify_blob_attestation.go Outdated Show resolved Hide resolved

steiza reviewed Jan 23, 2025

View reviewed changes

cmd/cosign/cli/verify/verify_blob_attestation.go Show resolved Hide resolved

malancas approved these changes Jan 29, 2025

View reviewed changes

codysoyland added 10 commits February 5, 2025 14:04

Add new func cosign.VerifyNewBundle which invokes sigstore-go verifier

ea79ec8

Signed-off-by: Cody Soyland <codysoyland@github.com>

Refactor verify-blob to use cosign.VerifyNewBundle

4dfdba2

Signed-off-by: Cody Soyland <codysoyland@github.com>

Refactor verify-blob-attestation to use cosign.VerifyNewBundle

977fd4c

Signed-off-by: Cody Soyland <codysoyland@github.com>

Add more tests

7090783

Signed-off-by: Cody Soyland <codysoyland@github.com>

Remove old verifyNewBundle

d732e2e

Signed-off-by: Cody Soyland <codysoyland@github.com>

Add support for verifying by payload digest and custom trusted root

f7531ca

Signed-off-by: Cody Soyland <codysoyland@github.com>

Add support for custom trusted root path in verify-blob-attestation

490802a

Signed-off-by: Cody Soyland <codysoyland@github.com>

Fix logic: require none of these fields to be set

32bc334

Signed-off-by: Cody Soyland <codysoyland@github.com>

Remove RekorURL from list of checked flags

4fe910b

This var has a default value so shouldn't be checked Signed-off-by: Cody Soyland <codysoyland@github.com>

Fix a couple of tests

4a08df9

These tests are incorrect: they set the signature field which is not allowed when doing bundle verification. Previously they were passing due to logic errors. Signed-off-by: Cody Soyland <codysoyland@github.com>

codysoyland force-pushed the verify-new-bundle-refactor branch from 5932f1f to 4a08df9 Compare February 5, 2025 19:05

codysoyland requested a review from a team as a code owner February 5, 2025 19:05

cmurphy reviewed Feb 6, 2025

View reviewed changes

pkg/cosign/verify.go Outdated Show resolved Hide resolved

pkg/cosign/verify.go Show resolved Hide resolved

pkg/cosign/verify.go Show resolved Hide resolved

Update pkg/cosign/verify.go

a3871df

Co-authored-by: Colleen Murphy <cmurphy@users.noreply.github.com> Signed-off-by: Cody Soyland <codysoyland@github.com>

haydentherapper reviewed Feb 7, 2025

View reviewed changes

codysoyland added 3 commits February 7, 2025 10:42

Remove unneeded log

a5bb207

Signed-off-by: Cody Soyland <codysoyland@github.com>

Do not allow --trusted-root flag without --new-bundle-format

a06f0eb

Signed-off-by: Cody Soyland <codysoyland@github.com>

Ignore context param

cf47c49

Signed-off-by: Cody Soyland <codysoyland@github.com>

codysoyland requested a review from haydentherapper February 7, 2025 16:22

haydentherapper approved these changes Feb 7, 2025

View reviewed changes

codysoyland merged commit 737c83c into sigstore:main Feb 7, 2025
29 checks passed

codysoyland deleted the verify-new-bundle-refactor branch February 7, 2025 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor verifyNewBundle into library function #4013

Refactor verifyNewBundle into library function #4013

codysoyland commented Jan 14, 2025 •

edited

Loading

codecov bot commented Jan 14, 2025 •

edited

Loading

codysoyland commented Jan 23, 2025

steiza left a comment

codysoyland commented Jan 24, 2025

malancas left a comment

haydentherapper commented Feb 4, 2025

cmurphy left a comment

haydentherapper left a comment

haydentherapper Feb 7, 2025

Refactor verifyNewBundle into library function #4013

Refactor verifyNewBundle into library function #4013

Conversation

codysoyland commented Jan 14, 2025 • edited Loading

Summary

Release Note

Documentation

codecov bot commented Jan 14, 2025 • edited Loading

Codecov Report

codysoyland commented Jan 23, 2025

steiza left a comment

Choose a reason for hiding this comment

codysoyland commented Jan 24, 2025

malancas left a comment

Choose a reason for hiding this comment

haydentherapper commented Feb 4, 2025

cmurphy left a comment

Choose a reason for hiding this comment

haydentherapper left a comment

Choose a reason for hiding this comment

haydentherapper Feb 7, 2025

Choose a reason for hiding this comment

codysoyland commented Jan 14, 2025 •

edited

Loading

codecov bot commented Jan 14, 2025 •

edited

Loading