Add bundle create helper command #3901

steiza · 2024-10-09T20:29:58Z

Summary

Working towards #3139, #3794

There are several cosign commands that now support the new protobuf bundle format. Users may have signed material in other formats, and this pull request adds a cosign bundle create... helper to assemble that material into a new protobuf bundle for use with the work in #3139.

You can test this with something like:

$ go run cmd/cosign/main.go attest-blob --bundle "old.bundle" --identity-token="..." --type=something --predicate=predicate.txt ../sigstore-go/examples/sigstore-go-signing/hello_world.txt

$ go run cmd/cosign/main.go bundle create --bundle=old.bundle --attestation=intoto.txt --artifact=../sigstore-go/examples/hello_world.txt --out test.sigstore.json

$ go run cmd/cosign/main.go verify-blob-attestation --bundle="test.sigstore.json" --new-bundle-format=true --type=something --certificate-oidc-issuer="https://token.actions.githubusercontent.com" --certificate-identity="https://github.com/sigstore-conformance/extremely-dangerous-public-oidc-beacon/.github/workflows/extremely-dangerous-oidc-beacon.yml@refs/heads/main" ../sigstore-go/examples/sigstore-go-signing/hello_world.txt

Release Note

Added cosign bundle create command to assist users in assembling signed content into the new protobuf bundle format to use in cosign verification commands

Documentation

sigstore/docs#327

Signed-off-by: Zach Steindler <[email protected]>

codecov · 2024-10-09T20:32:40Z

Codecov Report

Attention: Patch coverage is 25.21490% with 261 lines in your changes missing coverage. Please review.

Project coverage is 36.26%. Comparing base (2ef6022) to head (0d55987).
Report is 235 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/cosign/cli/verify/verify_bundle.go	24.81%	94 Missing and 9 partials ⚠️
cmd/cosign/cli/bundle/bundle.go	24.80%	78 Missing and 19 partials ⚠️
cmd/cosign/cli/options/bundle.go	0.00%	40 Missing ⚠️
cmd/cosign/cli/bundle.go	47.50%	20 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3901      +/-   ##
==========================================
- Coverage   40.10%   36.26%   -3.84%     
==========================================
  Files         155      209      +54     
  Lines       10044    13335    +3291     
==========================================
+ Hits         4028     4836     +808     
- Misses       5530     7874    +2344     
- Partials      486      625     +139

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

cmurphy

I had trouble getting this to work with a regular legacy bundle generated from sign-blob:

$ go run cmd/cosign/main.go sign-blob /tmp/blob -y --bundle /tmp/test.old.bundle 
Using payload from: /tmp/blob
Generating ephemeral keys...
Retrieving signed certificate...
[snipped]
Wrote bundle to file /tmp/test.old.bundle
MEUCIQD7GLMc2j2zn5SlejBDtvHIjZXzQB0sTe+bDHzFfMAJ1gIgEWMeFd/rjfwXb5jlimZY1kcQB/ulwP0ZNeuUye9AcS8=
$ go run cmd/cosign/main.go bundle create --bundle /tmp/test.old.bundle --out /tmp/test.converted.bundle --artifact /tmp/blob 
Error: searching log query: [POST /api/v1/log/entries/retrieve][400] searchLogQueryBadRequest  &{Code:400 Message:validation failure list:
validation failure list:
proposedContent.proposedContent.verifiers in body is required}
main.go:74: error during command execution: searching log query: [POST /api/v1/log/entries/retrieve][400] searchLogQueryBadRequest  &{Code:400 Message:validation failure list:
validation failure list:
proposedContent.proposedContent.verifiers in body is required}
exit status 1

Signed-off-by: Zach Steindler <[email protected]>

steiza · 2024-10-23T21:15:55Z

Nice catch! I did some refactoring around a nil envelope pointer vs an empty envelope and missed a check.

loosebazooka · 2024-10-25T15:25:39Z

cmd/cosign/cli/bundle/bundle_test.go

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package bundle


I'm not sure where go tests should go that load data from "resources" and run tests against them. But I think we should have tests that use known signature data (including ones with certs) and generates a bundle? vs generating the data here?

That's an interesting idea!

Most of the existing cosign tests generate the material they work with on the fly. Some advantages of generating the material is you don't need to include known data, and you're testing with a variety of inputs. Of course, a disadvantage is that you could have a test that intermittently fails!

I think this could be something cosign considers more broadly, but might not make sense to address in this specific pull request.

cmurphy · 2024-10-29T17:14:39Z

cmd/cosign/cli/verify/verify_bundle.go

+			payload = buf.Bytes()
+		}
+
+		tlogEntries, err := cosign.FindTlogEntry(ctx, rekorClient, sigB64, payload, pem)


Something seems to be wrong with how the signature is being encoded when I try to use the detached signature generated by cosign:

$ go run cmd/cosign/main.go sign-blob /tmp/blob -y --output-signature /tmp/blob.sig --key cosign.key Using payload from: /tmp/blob [snipped] tlog entry created with index: 144824671 Wrote signature to file /tmp/blob.sig $ go run cmd/cosign/main.go bundle create --signature /tmp/blob.sig --out /tmp/test.converted.bundle --artifact /tmp/blob --key cosign.pub Error: searching log query: [POST /api/v1/log/entries/retrieve][400] searchLogQueryBadRequest &{Code:400 Message:verifying signature: invalid signature when validating IEEE_P1363 encoded signature} main.go:74: error during command execution: searching log query: [POST /api/v1/log/entries/retrieve][400] searchLogQueryBadRequest &{Code:400 Message:verifying signature: invalid signature when validating IEEE_P1363 encoded signature} exit status 1 $ go run cmd/cosign/main.go sign-blob /tmp/blob -y --output-signature /tmp/blob.sig --key cosign.key --b64 Using payload from: /tmp/blob [snipped] tlog entry created with index: 144825079 Wrote signature to file /tmp/blob.sig $ go run cmd/cosign/main.go bundle create --signature /tmp/blob.sig --out /tmp/test.converted.bundle --artifact /tmp/blob --key cosign.pub Error: searching log query: [POST /api/v1/log/entries/retrieve][400] searchLogQueryBadRequest &{Code:400 Message:verifying signature: invalid signature when validating IEEE_P1363 encoded signature} main.go:74: error during command execution: searching log query: [POST /api/v1/log/entries/retrieve][400] searchLogQueryBadRequest &{Code:400 Message:verifying signature: invalid signature when validating IEEE_P1363 encoded signature} exit status 1

If you want to confirm what's been uploaded against what's being constructed, the --bundle can contain the canonicalized entry -

cosign/pkg/cosign/bundle/rekor.go

Line 29 in 9b17791

Body interface{} `json:"body"`

Oops! Yes, this is another bug in handling detached signatures, which should now be fixed.

cmurphy · 2024-10-29T17:17:26Z

cmd/cosign/cli/bundle/bundle_test.go

+		IgnoreTlog:    true,
+		Out:           outPath,
+		SignaturePath: sigPath,
+	}


Could more cases be tested here? For example, for converting an old-style bundle, or for when there's a fulcio identity certificate instead of a key?

This seems fair; I have added another test that does both the old style bundle and when a Fulcio identity certificate is used.

haydentherapper · 2024-10-29T22:39:25Z

cmd/cosign/cli/verify/verify_bundle.go

+			payload = buf.Bytes()
+		}
+
+		tlogEntries, err := cosign.FindTlogEntry(ctx, rekorClient, sigB64, payload, pem)


If you want to confirm what's been uploaded against what's being constructed, the --bundle can contain the canonicalized entry -

cosign/pkg/cosign/bundle/rekor.go

Line 29 in 9b17791

Body interface{} `json:"body"`

haydentherapper · 2024-10-29T22:42:34Z

cmd/cosign/cli/verify/verify_bundle.go

+		if len(tlogEntries) == 0 {
+			return nil, fmt.Errorf("unable to find tlog entry")
+		}
+		// Attempt to verify with the earliest integrated entry


Since FindTlogEntry is searching based on entry hash, you should only match one entry, so you could add that as a check.

I was copying

cosign/pkg/cosign/verify.go

Lines 446 to 469 in 9b17791

tlogEntries, err := FindTlogEntry(ctx, client, b64sig, payload, pem)

if err != nil {

return nil, err

}

if len(tlogEntries) == 0 {

return nil, fmt.Errorf("no valid tlog entries found with proposed entry")

}

// Always return the earliest integrated entry. That

// always suffices for verification of signature time.

var earliestLogEntry models.LogEntryAnon

var earliestLogEntryTime *time.Time

entryVerificationErrs := make([]string, 0)

for _, e := range tlogEntries {

entry := e

if err := VerifyTLogEntryOffline(ctx, &entry, rekorPubKeys); err != nil {

entryVerificationErrs = append(entryVerificationErrs, err.Error())

continue

}

entryTime := time.Unix(*entry.IntegratedTime, 0)

if earliestLogEntryTime == nil || entryTime.Before(*earliestLogEntryTime) {

earliestLogEntryTime = &entryTime

earliestLogEntry = entry

}

}

, but this sounds fine to me - I updated the implementation to check for exactly one entry.

Also add test for Fulcio certificate and old bundle format Signed-off-by: Zach Steindler <[email protected]>

haydentherapper

Looks great! I'll leave it open til EoD Monday in case there are any other comments.

cmurphy

lgtm

Add bundle create helper command

ff8128a

Signed-off-by: Zach Steindler <[email protected]>

This was referenced Oct 9, 2024

Support trusted root in cosign verification #3854

Closed

Add docs on using cosign bundle create sigstore/docs#334

Merged

cmurphy reviewed Oct 23, 2024

View reviewed changes

Check for empty envelope

0fff093

Signed-off-by: Zach Steindler <[email protected]>

loosebazooka reviewed Oct 25, 2024

View reviewed changes

cmurphy reviewed Oct 29, 2024

View reviewed changes

haydentherapper reviewed Oct 29, 2024

View reviewed changes

Fix bug with detached signature

0d55987

Also add test for Fulcio certificate and old bundle format Signed-off-by: Zach Steindler <[email protected]>

haydentherapper approved these changes Nov 1, 2024

View reviewed changes

cmurphy approved these changes Nov 4, 2024

View reviewed changes

haydentherapper merged commit 38c8a28 into sigstore:main Nov 5, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bundle create helper command #3901

Add bundle create helper command #3901

steiza commented Oct 9, 2024 •

edited by haydentherapper

Loading

codecov bot commented Oct 9, 2024 •

edited

Loading

cmurphy left a comment

steiza commented Oct 23, 2024

loosebazooka Oct 25, 2024 •

edited

Loading

steiza Oct 25, 2024

cmurphy Oct 29, 2024

haydentherapper Oct 29, 2024

steiza Nov 1, 2024

cmurphy Oct 29, 2024

steiza Nov 1, 2024

haydentherapper Oct 29, 2024

haydentherapper Oct 29, 2024

steiza Nov 1, 2024

haydentherapper left a comment

cmurphy left a comment

	tlogEntries, err := FindTlogEntry(ctx, client, b64sig, payload, pem)
	if err != nil {
	return nil, err
	}
	if len(tlogEntries) == 0 {
	return nil, fmt.Errorf("no valid tlog entries found with proposed entry")
	}
	// Always return the earliest integrated entry. That
	// always suffices for verification of signature time.
	var earliestLogEntry models.LogEntryAnon
	var earliestLogEntryTime *time.Time
	entryVerificationErrs := make([]string, 0)
	for _, e := range tlogEntries {
	entry := e
	if err := VerifyTLogEntryOffline(ctx, &entry, rekorPubKeys); err != nil {
	entryVerificationErrs = append(entryVerificationErrs, err.Error())
	continue
	}
	entryTime := time.Unix(*entry.IntegratedTime, 0)
	if earliestLogEntryTime == nil \|\| entryTime.Before(*earliestLogEntryTime) {
	earliestLogEntryTime = &entryTime
	earliestLogEntry = entry
	}
	}

Add bundle create helper command #3901

Add bundle create helper command #3901

Conversation

steiza commented Oct 9, 2024 • edited by haydentherapper Loading

Summary

Release Note

Documentation

codecov bot commented Oct 9, 2024 • edited Loading

Codecov Report

cmurphy left a comment

Choose a reason for hiding this comment

steiza commented Oct 23, 2024

loosebazooka Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haydentherapper left a comment

Choose a reason for hiding this comment

cmurphy left a comment

Choose a reason for hiding this comment

steiza commented Oct 9, 2024 •

edited by haydentherapper

Loading

codecov bot commented Oct 9, 2024 •

edited

Loading

loosebazooka Oct 25, 2024 •

edited

Loading