Attestation/Blob signing and verification using a RFC3161 time-stamping server #2464

hectorj2f · 2022-11-16T10:02:51Z

Summary

This PR adds support for signing and verification for blobs/attestations using a RFC3161 time-stamping server. This PR assumes we can rely on two different bundles when signing by passing the flags --rfc3161-timestamp-bundle and the former --bundle.

$ cosign sign-blob --timestamp-server-url=http://localhost:5555 --key cosign.key README.md  --rfc3161-timestamp-bundle mytimestampbundle

$ cosign verify-blob --timestamp-cert-chain ts_chain.pem --key cosign.pub README.md --rfc3161-timestamp-bundle mytimestampbundle

Bundle format in case both verification mechanism are used (TSA and Rekor):

{
	"base64Signature": "xxxx/8tmKUTTOHLBTHn",
	"cert": "xx==",
	"rekorBundle": {
		"SignedEntryTimestamp": "xxx+T3qDZtg0=",
		"Payload": {
			"body": "xxxxxxx=",
			"integratedTime": 1,
			"logIndex": 1,
			"logID": "xxxxx"
		}
	},
	"rfc3161timestamp": {
		"SignedRFC3161Timestamp": "xxxx"
	}
}

A similar set of command flags are used for attestations:

$ cosign attest --predicate predicate --timestamp-server-url=http://localhost:5555 <YOUR_IMAGE> 

$ cosign verify-attestation --timestamp-cert-chain ts_chain.pem <YOUR_IMAGE>  --insecure-skip-tlog-verify

Release Note

Add support for signing/verification for attestations and blobs using a RFC3161 time-stamping server.

Documentation

codecov-commenter · 2022-11-16T10:21:19Z

Codecov Report

Merging #2464 (3f6115b) into main (8f66577) will decrease coverage by 0.36%.
The diff coverage is 10.69%.

@@            Coverage Diff             @@
##             main    #2464      +/-   ##
==========================================
- Coverage   30.34%   29.98%   -0.37%     
==========================================
  Files         139      139              
  Lines        8469     8605     +136     
==========================================
+ Hits         2570     2580      +10     
- Misses       5546     5666     +120     
- Partials      353      359       +6

Impacted Files	Coverage Δ
cmd/cosign/cli/attest.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/options/attest.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/options/policy.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/options/signblob.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/options/verify.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/policy_init.go	`1.30% <0.00%> (-0.06%)`	⬇️
cmd/cosign/cli/sign/sign.go	`15.34% <0.00%> (ø)`
cmd/cosign/cli/sign/sign_blob.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/signblob.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/verify.go	`0.00% <0.00%> (ø)`
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

asraa · 2022-11-16T15:32:01Z

cmd/cosign/cli/verify/verify_blob.go

-	// b. We can make an online lookup to the transparency log since we don't have an entry.
+		// b. We can make an online lookup to the transparency log since we don't have an entry.
+	case tsaBundle != nil:
+		// TODO: Verify TSA only and return the result with the tsa bundle


you would need to set the validity time here

asraa · 2022-11-16T15:35:02Z

RE the open questions, I think they should be handled separately.

Can we supersede the logic changes in the verifyBlob inner call with the PR that combines the OCI logic?

hectorj2f · 2022-11-17T10:27:48Z

I am pausing this work until #2461 gets merged!

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

hectorj2f · 2022-11-22T15:27:46Z

@priyawadhwa @haydentherapper This PR is ready for a review. I was planning to increase the test coverage and perhaps add this experimental flag/env var that we discussed, as part of future PRs.

priyawadhwa

tests look great! just a few comments.

cmd/cosign/cli/options/verify.go

cmd/cosign/cli/sign/sign.go

cmd/cosign/cli/sign/sign_blob.go

cmd/cosign/cli/verify/verify_attestation.go

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

haydentherapper · 2022-11-22T16:28:58Z

Cc @asraa

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

priyawadhwa

lgtm!

haydentherapper

@hectorj2f Left some comments, happy to implement the changes if you are busy.

haydentherapper · 2022-11-29T20:50:48Z

cmd/cosign/cli/options/signblob.go

+		"url to the Timestamp RFC3161 server, default none")
+
+	cmd.Flags().StringVar(&o.RFC3161TimestampPath, "rfc3161-timestamp-bundle", "",
+		"write everything required to verify the blob to a FILE")


Description needs to be updated, this was copied from BundlePath.

haydentherapper · 2022-11-29T20:52:14Z

cmd/cosign/cli/policy_init.go

+					return fmt.Errorf("failed to create TSA client: %w", err)
+				}
+				// Here we get the response from the timestamped authority server
+				_, err = tsa.GetTimestampedSignature(signed.Signed, clientTSA)


nit for style: if _, err := tsa.GetTimestampedSignature(...); err != nil { ... }

haydentherapper · 2022-11-29T21:07:15Z

cmd/cosign/cli/sign/sign_blob.go

+		if err != nil {
+			return nil, fmt.Errorf("failed to create TSA client: %w", err)
+		}
+		b64Sig := []byte(base64.StdEncoding.EncodeToString(sig))


Should we document somewhere that we're generating a timestamp over the base64 signature, not the raw bytes? This was somewhat discussed in sigstore/timestamp-authority#116 - Cosign is being opinionated in using the base64-encoded sig, which I agree with, just think we should be explicit about this.

Update the SPEC docs for the OCI annotations?

haydentherapper · 2022-11-29T21:09:10Z

cmd/cosign/cli/sign/sign_blob.go

@@ -82,6 +98,20 @@ func SignBlobCmd(ro *options.RootOptions, ko options.KeyOpts, regOpts options.Re
 		signedPayload.Bundle = cbundle.EntryToBundle(entry)
 	}

+	// if bundle is specified, just do that and ignore the rest
+	if ko.RFC3161TimestampPath != "" {


Should we require that if ko.TSAServerURL is set, ko.RFC3161TimestampPath must also be? Otherwise it's unnecessarily fetching a timestamp without persisting it.

haydentherapper · 2022-11-29T21:10:07Z

cmd/cosign/cli/sign/sign_blob.go

+	if ko.RFC3161TimestampPath != "" {
+		signedPayload.Base64Signature = base64.StdEncoding.EncodeToString(sig)
+
+		contents, err := json.Marshal(signedPayload)


Shouldn't this be writing signedPayload.RFC3161Timestamp, not signedPayload.Base64Signature?

haydentherapper · 2022-11-29T21:13:41Z

cmd/cosign/cli/verify/verify_blob.go

@@ -72,16 +73,16 @@ func (c *VerifyBlobCmd) Exec(ctx context.Context, blobRef string) error {
 	opts := make([]static.Option, 0)

 	// Require a certificate/key OR a local bundle file that has the cert.
-	if options.NOf(c.KeyRef, c.CertRef, c.Sk, c.BundlePath) == 0 {
-		return fmt.Errorf("please provide a cert to verify against via --certificate or a bundle via --bundle")
+	if options.NOf(c.KeyRef, c.CertRef, c.Sk, c.BundlePath, c.RFC3161TimestampPath) == 0 {


This doesn't seem correct, RFC3161TimestampPath should contain a timestamp, not a code-signing certificate.

haydentherapper · 2022-11-29T21:14:34Z

cmd/cosign/cli/verify/verify_blob.go

+		if err != nil {
+			return err
+		}
+		// Note: RFC3161 timestamp does not set the certificate.


I'm confused by this, b.Cert is always "", so isn't this never run?

haydentherapper · 2022-11-29T21:15:46Z

cmd/cosign/cli/verify/verify_blob.go

@@ -273,6 +323,12 @@ func base64signature(sigRef string, bundlePath string) (string, error) {
 			return "", err
 		}
 		targetSig = []byte(b.Base64Signature)
+	case rfc3161TimestampPath != "":


I left a comment above about this, but rfc3161TimestampPath should be storing a timestamp, not a signature, correct?

Further, a 3161 timestamp should be in addition to the bundle, this makes it either/or.

haydentherapper · 2022-11-30T23:45:57Z

Follow up PR #2499

hectorj2f added the enhancement New feature or request label Nov 16, 2022

hectorj2f self-assigned this Nov 16, 2022

hectorj2f mentioned this pull request Nov 16, 2022

refactor: share logic between verify-blob, verify, and verify-attestation #2461

Merged

asraa reviewed Nov 16, 2022

View reviewed changes

hectorj2f force-pushed the blob_signing_with_tsa branch from e0982e6 to d7a28bd Compare November 21, 2022 17:24

hectorj2f added 9 commits November 22, 2022 12:15

add support for tsa signing and verification of images

47be791

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

address reviewer comments

de64e54

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

add support for both tsa and rekor verifications

5ea599a

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

signing and verification of blobs using time-stamping

c8b930b

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

adjust code to latest changes

cbd56f5

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

add support for attestations

3367200

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

set right annotation

b969b8b

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

add test for blob signing/verification

749f796

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

ensure b64 signature is time-stamped

3f6115b

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

hectorj2f force-pushed the blob_signing_with_tsa branch from d7a28bd to 3f6115b Compare November 22, 2022 15:15

hectorj2f changed the title ~~Blob signing and verification using a RFC3161 time-stamping server~~ Attestation/Blob signing and verification using a RFC3161 time-stamping server Nov 22, 2022

hectorj2f marked this pull request as ready for review November 22, 2022 15:23

hectorj2f requested review from asraa, priyawadhwa and haydentherapper November 22, 2022 15:25

priyawadhwa reviewed Nov 22, 2022

View reviewed changes

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

cmd/cosign/cli/sign/sign.go Show resolved Hide resolved

cmd/cosign/cli/sign/sign_blob.go Outdated Show resolved Hide resolved

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

test: add attestation verify test

1a0f088

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

refactor functions and flags

954fd50

Signed-off-by: Hector Fernandez <hector@chainguard.dev>

hectorj2f requested a review from priyawadhwa November 22, 2022 22:10

priyawadhwa approved these changes Nov 23, 2022

View reviewed changes

priyawadhwa merged commit 7e967e8 into sigstore:main Nov 23, 2022

github-actions bot added this to the v1.14.0 milestone Nov 23, 2022

hectorj2f deleted the blob_signing_with_tsa branch November 23, 2022 16:34

haydentherapper reviewed Nov 29, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Attestation/Blob signing and verification using a RFC3161 time-stamping server #2464

Attestation/Blob signing and verification using a RFC3161 time-stamping server #2464

hectorj2f commented Nov 16, 2022 •

edited

Loading

codecov-commenter commented Nov 16, 2022 •

edited

Loading

asraa Nov 16, 2022

asraa commented Nov 16, 2022

hectorj2f commented Nov 17, 2022

hectorj2f commented Nov 22, 2022

priyawadhwa left a comment

haydentherapper commented Nov 22, 2022

priyawadhwa left a comment

haydentherapper left a comment

haydentherapper Nov 29, 2022

haydentherapper Nov 29, 2022

haydentherapper Nov 29, 2022

haydentherapper Nov 29, 2022

haydentherapper Nov 29, 2022

haydentherapper Nov 29, 2022

haydentherapper Nov 29, 2022

haydentherapper Nov 29, 2022

haydentherapper Nov 29, 2022

haydentherapper commented Nov 30, 2022

Attestation/Blob signing and verification using a RFC3161 time-stamping server #2464

Attestation/Blob signing and verification using a RFC3161 time-stamping server #2464

Conversation

hectorj2f commented Nov 16, 2022 • edited Loading

Summary

Release Note

Documentation

codecov-commenter commented Nov 16, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

asraa commented Nov 16, 2022

hectorj2f commented Nov 17, 2022

hectorj2f commented Nov 22, 2022

priyawadhwa left a comment

Choose a reason for hiding this comment

haydentherapper commented Nov 22, 2022

priyawadhwa left a comment

Choose a reason for hiding this comment

haydentherapper left a 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

Choose a reason for hiding this comment

haydentherapper commented Nov 30, 2022

hectorj2f commented Nov 16, 2022 •

edited

Loading

codecov-commenter commented Nov 16, 2022 •

edited

Loading