add support for tsa signing and verification of images #2460

hectorj2f · 2022-11-15T12:19:33Z

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

Summary

In relation to #2331, this PR adds support for image signing and verification using a RFC3161 time-stamping server instead of the transparency log.

NOTE: Rekor and TSA cannot be used together for the moment.

For testing purposes, we have integrated using https://github.com/sigstore/timestamp-authority. So we expect you to run a TSA server for testing:

$ curl http://localhost:3000/api/v1/timestamp/certchain > ts_chain.pem

$ cosign sign <YOUR_IMAGE> --tlog-upload=false --timestamp-server-url=http://localhost:3000

$ cosign verify <YOUR_IMAGE> --timestamp-server-url=http://localhost:3000 --timestamp-cert-chain=ts_chain.pem

Release Note

Add support for signing images using a RFC3161 time-stamping server

Documentation

hectorj2f · 2022-11-15T12:20:18Z

@priyawadhwa @haydentherapper I am considering to rename any reference of TSABundle to 3161Timestamp. Any thoughts ?.

hectorj2f · 2022-11-15T12:21:11Z

pkg/cosign/verify.go

+		return false, fmt.Errorf("reading base64signature: %w", err)
+	}
+
+	fmt.Println("Verifying TSA Bundle")


Perhaps we want to avoid printing this message (it will be printed per validated signature).

If you have it, it should go to STDERR

codecov-commenter · 2022-11-15T12:22:09Z

Codecov Report

Merging #2460 (6f2a95b) into main (3e870f9) will decrease coverage by 0.00%.
The diff coverage is 30.67%.

@@            Coverage Diff             @@
##             main    #2460      +/-   ##
==========================================
- Coverage   30.96%   30.96%   -0.01%     
==========================================
  Files         138      139       +1     
  Lines        8473     8635     +162     
==========================================
+ Hits         2624     2674      +50     
- Misses       5509     5600      +91     
- Partials      340      361      +21

Impacted Files	Coverage Δ
cmd/cosign/cli/options/sign.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/options/verify.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/sign.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/sign/sign.go	`15.46% <0.00%> (-0.26%)`	⬇️
cmd/cosign/cli/verify.go	`0.00% <0.00%> (ø)`
cmd/cosign/cli/verify/verify.go	`18.15% <0.00%> (-1.02%)`	⬇️
pkg/cosign/fetch.go	`0.00% <0.00%> (ø)`
pkg/oci/static/signature.go	`48.18% <0.00%> (-2.78%)`	⬇️
pkg/cosign/verify.go	`36.10% <27.65%> (-0.55%)`	⬇️
internal/pkg/cosign/tsa/signer.go	`52.11% <52.11%> (ø)`

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

pkg/cosign/verify.go

priyawadhwa · 2022-11-15T16:55:29Z

I am considering to rename any reference of TSABundle to 3161Timestamp. Any thoughts ?.

I think it might be more user-friendly to stick with TSABundle since we already have RekorBundle (also since we have user-facing flags like --bundle, something like --tsa-bundle might be more intuitive).

haydentherapper · 2022-11-15T17:17:48Z

cmd/cosign/cli/sign/sign.go

-	if ShouldUploadToTlog(ctx, ko, digest, ko.SkipConfirmation, tlogUpload) {
+
+	// TODO: For the moment you can only use the timestamped service OR the transparency log.
+	if ko.TSAServerURL != "" {


I would prefer to support both and not have this as a TODO. Timestamps are simply a way to have a third-party provide time, but don't provide transparency. Having this as a TODO risks changing the security model of Sigstore.

Following the existing code, can we have the tsa.NewSigner be wrapped by irekor.NewSigner?

cc @asraa

@haydentherapper @asraa Could we add support for both as part of a follow-up PR ? Likewise I will advocate for supporting the TSA as an alternative to the tlog. Some users may not want transparency.

As long as the follow up is before we cut a new release, sure. Is it a large change to support both? I would have thought we could support both by just wrapping the tsa signer in the rekor signer struct.

If a user doesn't want to use the transparency log, that should be controlled via an explicit flag (tlog-upload) rather than the usage of timestamping. I just want to make sure we're clear on the messaging and security model for Sigstore, that timestamping provides an alternative to how to verify expiration of a certificate. Rekor currently plays two roles, transparency and timestamping, so using a TSA only changes the second.

There should be a limited number of reasons to not use Rekor - private artifacts are the main one. Otherwise, you really should be uploading entries to Rekor.

There should be a limited number of reasons to not use Rekor - private artifacts are the main one. Otherwise, you really should be uploading entries to Rekor.

+1. We should still support this use case though. We can default to using the TSA+Rekor together, and provide an opt-in flag for folks who want to sign private artifacts and verify against the TSA but not Rekor. I think because of this use case it's more logical to have a separate TSA signer/verifier than to wrap it into Rekor.

Could we add support for both as part of a follow-up PR ?

sounds good! as a reviewer I definitely prefer lots of smaller PRs :) there's no plan to cut any releases at the moment so should be fine.

It looks like the code to support both should be pretty straightforward, since irekor.NewSigner takes a cosign.Signer and tsa.NewSigner implements that already.

This all sounds good to me: I think everyone laid it out right. We can support TSA+Rekor, with a control knob on removing hte Rekor upload.

For verification, we can start verifying TSA's on conditional requirements, soon always validating them in the long tail. The verification flow will look the same, we still will use a Rekor promise of inclusion offline, but instead use the verified TSA as the time check.

I extracted the following list of action items as result of this thread:

Add support for TSA and Rekor together.

Users could rely on TSA-only whenever they set --tlog-upload=false. We'll add a warning message and documentation about the limitations of using the timestamp verification instead of the tlog verification.

Add a --skip-tlog-verify flag to our cosign verify* commands to skip the tlog verification done by default.

cmd/cosign/cli/options/verify.go

internal/pkg/cosign/tsa/signer.go

internal/pkg/cosign/tsa/signer_test.go

pkg/cosign/verify.go

cmd/cosign/cli/options/verify.go

internal/pkg/cosign/tsa/signer.go

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

hectorj2f · 2022-11-15T20:02:56Z

@haydentherapper @priyawadhwa I've addressed all your comments. Please, have a look.

On the other hand I have a list of actions items to tackle in a different PR #2460 (comment).

haydentherapper

LGTM

pkg/cosign/verify.go

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

priyawadhwa

nice!

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

hectorj2f requested a review from priyawadhwa November 15, 2022 12:19

hectorj2f self-assigned this Nov 15, 2022

hectorj2f commented Nov 15, 2022

View reviewed changes

hectorj2f force-pushed the tsa_image_signing_verification branch from b8b3fcb to c5d7f72 Compare November 15, 2022 12:43

hectorj2f requested a review from haydentherapper November 15, 2022 12:48

hectorj2f commented Nov 15, 2022

View reviewed changes

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

haydentherapper requested changes Nov 15, 2022

View reviewed changes

priyawadhwa reviewed Nov 15, 2022

View reviewed changes

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

internal/pkg/cosign/tsa/signer.go Outdated Show resolved Hide resolved

hectorj2f added 3 commits November 15, 2022 20:30

add support for tsa signing and verification of images

45db380

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

add tsa flag to cli docs

7b9db3f

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

read tsa bundle from signatures

30403e7

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

hectorj2f force-pushed the tsa_image_signing_verification branch from c5d7f72 to abc0641 Compare November 15, 2022 19:31

address reviewer comments

54543bd

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

hectorj2f force-pushed the tsa_image_signing_verification branch from abc0641 to 54543bd Compare November 15, 2022 19:54

haydentherapper reviewed Nov 15, 2022

View reviewed changes

pkg/cosign/verify.go Show resolved Hide resolved

pkg/cosign/verify.go Show resolved Hide resolved

hectorj2f requested a review from priyawadhwa November 15, 2022 20:23

add TODOs to the code

6f2a95b

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

priyawadhwa approved these changes Nov 15, 2022

View reviewed changes

hectorj2f merged commit ba5997b into sigstore:main Nov 15, 2022

hectorj2f deleted the tsa_image_signing_verification branch November 15, 2022 22:50

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for tsa signing and verification of images #2460

add support for tsa signing and verification of images #2460

hectorj2f commented Nov 15, 2022

hectorj2f commented Nov 15, 2022

hectorj2f Nov 15, 2022

znewman01 Nov 15, 2022

codecov-commenter commented Nov 15, 2022 •

edited

Loading

priyawadhwa commented Nov 15, 2022

haydentherapper Nov 15, 2022

hectorj2f Nov 15, 2022

haydentherapper Nov 15, 2022

priyawadhwa Nov 15, 2022

haydentherapper Nov 15, 2022

asraa Nov 15, 2022

hectorj2f Nov 15, 2022

hectorj2f commented Nov 15, 2022

haydentherapper left a comment

priyawadhwa left a comment

add support for tsa signing and verification of images #2460

add support for tsa signing and verification of images #2460

Conversation

hectorj2f commented Nov 15, 2022

Summary

Release Note

Documentation

hectorj2f commented Nov 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 15, 2022 • edited Loading

Codecov Report

priyawadhwa commented Nov 15, 2022

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

hectorj2f commented Nov 15, 2022

haydentherapper left a comment

Choose a reason for hiding this comment

priyawadhwa left a comment

Choose a reason for hiding this comment

codecov-commenter commented Nov 15, 2022 •

edited

Loading