From b4e6fce3ef671f752935144736aad1f701456303 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 26 Oct 2022 14:10:16 -0400 Subject: [PATCH 1/2] Remove experimental flag from cosign sign and cosign verify this changes cosign behavior to automatically add all signatures to the transparency log. in the next commit i'll move the verification for this into the 'ShouldUploadTlog' function. Signed-off-by: Priya Wadhwa --- cmd/cosign/cli/attest/attest.go | 2 +- cmd/cosign/cli/sign.go | 4 +-- cmd/cosign/cli/sign/sign.go | 43 +++++++++++++++++---------------- cmd/cosign/cli/verify.go | 5 +--- cmd/cosign/cli/verify/verify.go | 16 +++++++++--- 5 files changed, 38 insertions(+), 32 deletions(-) diff --git a/cmd/cosign/cli/attest/attest.go b/cmd/cosign/cli/attest/attest.go index b6e3ef81ea1..99f591db3d8 100644 --- a/cmd/cosign/cli/attest/attest.go +++ b/cmd/cosign/cli/attest/attest.go @@ -159,7 +159,7 @@ func AttestCmd(ctx context.Context, ko options.KeyOpts, regOpts options.Registry } // Check whether we should be uploading to the transparency log - if sign.ShouldUploadToTlog(ctx, digest, force, noTlogUpload, ko.RekorURL) { + if sign.ShouldUploadToTlog(ctx, ko, digest, force, noTlogUpload) { bundle, err := uploadToTlog(ctx, sv, ko.RekorURL, func(r *client.Rekor, b []byte) (*models.LogEntryAnon, error) { return cosign.TLogUploadInTotoAttestation(ctx, r, signedPayload, b) }) diff --git a/cmd/cosign/cli/sign.go b/cmd/cosign/cli/sign.go index 4ce1e1d128a..d97f06c8b03 100644 --- a/cmd/cosign/cli/sign.go +++ b/cmd/cosign/cli/sign.go @@ -40,8 +40,8 @@ race conditions or (worse) malicious tampering. `, Example: ` cosign sign --key | [--payload ] [-a key=value] [--upload=true|false] [-f] [-r] - # sign a container image with Google sign-in (experimental) - COSIGN_EXPERIMENTAL=1 cosign sign + # sign a container image with the Sigstore OIDC flow + cosign sign # sign a container image with a local key pair file cosign sign --key cosign.key diff --git a/cmd/cosign/cli/sign/sign.go b/cmd/cosign/cli/sign/sign.go index 690994a9666..a68c34cdf70 100644 --- a/cmd/cosign/cli/sign/sign.go +++ b/cmd/cosign/cli/sign/sign.go @@ -60,17 +60,13 @@ import ( const TagReferenceMessage string = `WARNING: Image reference %s uses a tag, not a digest, to identify the image to sign. -This can lead you to sign a different image than the intended one. Please use a -digest (example.com/ubuntu@sha256:abc123...) rather than tag -(example.com/ubuntu:latest) for the input to cosign. The ability to refer to -images by tag will be removed in a future release. + This can lead you to sign a different image than the intended one. Please use a + digest (example.com/ubuntu@sha256:abc123...) rather than tag + (example.com/ubuntu:latest) for the input to cosign. The ability to refer to + images by tag will be removed in a future release. ` -func ShouldUploadToTlog(ctx context.Context, ref name.Reference, force bool, noTlogUpload bool, url string) bool { - // Check whether experimental is on! - if !options.EnableExperimental() { - return false - } +func ShouldUploadToTlog(ctx context.Context, ko options.KeyOpts, ref name.Reference, force, noTlogUpload bool) bool { // We are forcing publishing to the Tlog. if force { return true @@ -80,9 +76,14 @@ func ShouldUploadToTlog(ctx context.Context, ref name.Reference, force bool, noT return false } + // If we aren't using keyless signing, we can skip the tlog + if !keylessSigning(ko) { + return false + } + // Check if the image is public (no auth in Get) if _, err := remote.Get(ref, remote.WithContext(ctx)); err != nil { - fmt.Fprintf(os.Stderr, "%q appears to be a private repository, please confirm uploading to the transparency log at %q [Y/N]: ", ref.Context().String(), url) + fmt.Fprintf(os.Stderr, "%q appears to be a private repository, please confirm uploading to the transparency log at %q [Y/N]: ", ref.Context().String(), ko.RekorURL) var tlogConfirmResponse string if _, err := fmt.Scanln(&tlogConfirmResponse); err != nil { @@ -126,16 +127,6 @@ func ParseOCIReference(refStr string, out io.Writer, opts ...name.Option) (name. func SignCmd(ro *options.RootOptions, ko options.KeyOpts, regOpts options.RegistryOptions, annotations map[string]interface{}, imgs []string, certPath string, certChainPath string, upload bool, outputSignature, outputCertificate string, payloadPath string, force bool, recursive bool, attachment string, noTlogUpload bool) error { - if options.EnableExperimental() { - if options.NOf(ko.KeyRef, ko.Sk) > 1 { - return &options.KeyParseError{} - } - } else { - if !options.OneOf(ko.KeyRef, ko.Sk) { - return &options.KeyParseError{} - } - } - ctx, cancel := context.WithTimeout(context.Background(), ro.Timeout) defer cancel() @@ -233,7 +224,7 @@ func signDigest(ctx context.Context, digest name.Digest, payload []byte, ko opti if sv.Cert != nil { s = ifulcio.NewSigner(s, sv.Cert, sv.Chain) } - if ShouldUploadToTlog(ctx, digest, force, noTlogUpload, ko.RekorURL) { + if ShouldUploadToTlog(ctx, ko, digest, force, noTlogUpload) { rClient, err := rekor.NewClient(ko.RekorURL) if err != nil { return err @@ -528,3 +519,13 @@ func (c *SignerVerifier) Bytes(ctx context.Context) ([]byte, error) { } return pemBytes, nil } + +func keylessSigning(ko options.KeyOpts) bool { + if ko.KeyRef != "" { + return false + } + if ko.Sk { + return false + } + return true +} diff --git a/cmd/cosign/cli/verify.go b/cmd/cosign/cli/verify.go index 69679ada938..81c17b9aa44 100644 --- a/cmd/cosign/cli/verify.go +++ b/cmd/cosign/cli/verify.go @@ -36,7 +36,7 @@ func Verify() *cobra.Command { against the transparency log.`, Example: ` cosign verify --key || [ ...] - # verify cosign claims and signing certificates on the image + # verify cosign claims and signing certificates on the image with the transparency log cosign verify # verify multiple images @@ -45,9 +45,6 @@ against the transparency log.`, # additionally verify specified annotations cosign verify -a key1=val1 -a key2=val2 - # (experimental) additionally, verify with the transparency log - COSIGN_EXPERIMENTAL=1 cosign verify - # verify image with an on-disk public key cosign verify --key cosign.pub diff --git a/cmd/cosign/cli/verify/verify.go b/cmd/cosign/cli/verify/verify.go index b6039bdc381..90961cff061 100644 --- a/cmd/cosign/cli/verify/verify.go +++ b/cmd/cosign/cli/verify/verify.go @@ -92,9 +92,6 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) { c.HashAlgorithm = crypto.SHA256 } - if !options.OneOf(c.KeyRef, c.CertRef, c.Sk) && !options.EnableExperimental() { - return &options.PubKeyParseError{} - } ociremoteOpts, err := c.ClientOpts(ctx) if err != nil { return fmt.Errorf("constructing client options: %w", err) @@ -116,7 +113,8 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) { if c.CheckClaims { co.ClaimVerifier = cosign.SimpleClaimVerifier } - if options.EnableExperimental() { + + if c.keylessVerification() { if c.RekorURL != "" { rekorClient, err := rekor.NewClient(c.RekorURL) if err != nil { @@ -398,3 +396,13 @@ func loadCertChainFromFileOrURL(path string) ([]*x509.Certificate, error) { } return certs, nil } + +func (c *VerifyCommand) keylessVerification() bool { + if c.KeyRef != "" { + return false + } + if c.Sk { + return false + } + return true +} From 4a8c396888cd713cb92f1ac7977871caacd9ec3d Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 26 Oct 2022 16:02:05 -0400 Subject: [PATCH 2/2] Fix tests Signed-off-by: Priya Wadhwa --- cmd/cosign/cli/sign/sign.go | 4 ++++ doc/cosign_sign.md | 4 ++-- doc/cosign_verify.md | 5 +---- test/e2e_test.go | 34 +++++++++++++++++----------------- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/cmd/cosign/cli/sign/sign.go b/cmd/cosign/cli/sign/sign.go index a68c34cdf70..60e367701d1 100644 --- a/cmd/cosign/cli/sign/sign.go +++ b/cmd/cosign/cli/sign/sign.go @@ -130,6 +130,10 @@ func SignCmd(ro *options.RootOptions, ko options.KeyOpts, regOpts options.Regist ctx, cancel := context.WithTimeout(context.Background(), ro.Timeout) defer cancel() + if options.NOf(ko.KeyRef, ko.Sk) > 1 { + return &options.KeyParseError{} + } + sv, err := SignerFromKeyOpts(ctx, certPath, certChainPath, ko) if err != nil { return fmt.Errorf("getting signer: %w", err) diff --git a/doc/cosign_sign.md b/doc/cosign_sign.md index 6b73f9af978..bdbf86bbf6b 100644 --- a/doc/cosign_sign.md +++ b/doc/cosign_sign.md @@ -20,8 +20,8 @@ cosign sign [flags] ``` cosign sign --key | [--payload ] [-a key=value] [--upload=true|false] [-f] [-r] - # sign a container image with Google sign-in (experimental) - COSIGN_EXPERIMENTAL=1 cosign sign + # sign a container image with the Sigstore OIDC flow + cosign sign # sign a container image with a local key pair file cosign sign --key cosign.key diff --git a/doc/cosign_verify.md b/doc/cosign_verify.md index 29197748afd..a35454fbcac 100644 --- a/doc/cosign_verify.md +++ b/doc/cosign_verify.md @@ -16,7 +16,7 @@ cosign verify [flags] ``` cosign verify --key || [ ...] - # verify cosign claims and signing certificates on the image + # verify cosign claims and signing certificates on the image with the transparency log cosign verify # verify multiple images @@ -25,9 +25,6 @@ cosign verify [flags] # additionally verify specified annotations cosign verify -a key1=val1 -a key2=val2 - # (experimental) additionally, verify with the transparency log - COSIGN_EXPERIMENTAL=1 cosign verify - # verify image with an on-disk public key cosign verify --key cosign.pub diff --git a/test/e2e_test.go b/test/e2e_test.go index de2f0ef95d3..2d2a646844a 100644 --- a/test/e2e_test.go +++ b/test/e2e_test.go @@ -1146,16 +1146,13 @@ func TestTlog(t *testing.T) { // Now verify should work! must(verify(pubKeyPath, imgName, true, nil, ""), t) - // Now we turn on the tlog! - defer setenv(t, env.VariableExperimental.String(), "1")() - - // Verify shouldn't work since we haven't put anything in it yet. - mustErr(verify(pubKeyPath, imgName, true, nil, ""), t) + // TODO: priyawadhwa@ to figure out how to add an entry to the tlog without using keyless signing + // We could add an --upload-tlog flag, but it's a bit weird since we have a --no-upload-tlog flag too right now. - // Sign again with the tlog env var on - must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t) - // And now verify works! - must(verify(pubKeyPath, imgName, true, nil, ""), t) + // // Sign again with the tlog env var on + // must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", false), t) + // // And now verify works! + // must(verify(pubKeyPath, imgName, true, nil, ""), t) } func TestNoTlog(t *testing.T) { @@ -1184,16 +1181,19 @@ func TestNoTlog(t *testing.T) { // Now verify should work! must(verify(pubKeyPath, imgName, true, nil, ""), t) - // Now we turn on the tlog! - defer setenv(t, env.VariableExperimental.String(), "1")() + // TODO: priyawadhwa@ to figure out how to add an entry to the tlog without using keyless signing + // We could add an --upload-tlog flag, but it's a bit weird since we have a --no-upload-tlog flag too right now. - // Verify shouldn't work since we haven't put anything in it yet. - mustErr(verify(pubKeyPath, imgName, true, nil, ""), t) + // // Now we turn on the tlog! + // defer setenv(t, env.VariableExperimental.String(), "1")() - // Sign again with the tlog env var on with option to not upload tlog - must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", true), t) - // And verify it still fails. - mustErr(verify(pubKeyPath, imgName, true, nil, ""), t) + // // Verify shouldn't work since we haven't put anything in it yet. + // mustErr(verify(pubKeyPath, imgName, true, nil, ""), t) + + // // Sign again with the tlog env var on with option to not upload tlog + // must(sign.SignCmd(ro, ko, options.RegistryOptions{}, nil, []string{imgName}, "", "", true, "", "", "", false, false, "", true), t) + // // And verify it still fails. + // mustErr(verify(pubKeyPath, imgName, true, nil, ""), t) } func TestGetPublicKeyCustomOut(t *testing.T) {