From cbfe9b432d0b532e98e7e80bd150d2a8e4256f7e Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Tue, 14 Sep 2021 12:03:10 -0700 Subject: [PATCH 1/2] Have the keyless `cosign sign` flow use a single 3LO. With this change, the keyless flow builds a single signer for all of the images, which means a single key and 3LO for all of the references we sign: ```shell $ COSIGN_EXPERIMENTAL=true cosign sign ghcr.io/mattmoor/controller ghcr.io/mattmoor/webhook Generating ephemeral keys... Retrieving signed certificate... Your browser will now be opened to: https://oauth2.sigstore.dev/auth/auth?REDACTED Successfully verified SCT... tlog entry created with index: 693418 Pushing signature to: ghcr.io/mattmoor/controller:sha256-b10f4b2e04cde2e799e080068f162ef668c4db3099382798b5fe1a208023105d.sig tlog entry created with index: 693420 Pushing signature to: ghcr.io/mattmoor/webhook:sha256-ed1b1c778685ae0739cd4c6354fa2d724351b01e998a019d1ddc2909c377483d.sig ``` Fixes: /~https://github.com/sigstore/cosign/issues/658 Signed-off-by: Matt Moore --- cmd/cosign/cli/sign.go | 62 +++++++++++++++++++------------------ cmd/cosign/cli/sign_test.go | 2 +- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/cmd/cosign/cli/sign.go b/cmd/cosign/cli/sign.go index 3c143a87207..ab11d072915 100644 --- a/cmd/cosign/cli/sign.go +++ b/cmd/cosign/cli/sign.go @@ -186,13 +186,11 @@ EXAMPLES OIDCClientID: *oidcClientID, OIDCClientSecret: *oidcClientSecret, } - for _, img := range args { - if err := SignCmd(ctx, ko, annotations.annotations, img, *cert, *upload, *payloadPath, *force, *recursive, *attachment); err != nil { - if *attachment == "" { - return errors.Wrapf(err, "signing %s", img) - } - return errors.Wrapf(err, "signing attachement %s for image %s", *attachment, img) + if err := SignCmd(ctx, ko, annotations.annotations, args, *cert, *upload, *payloadPath, *force, *recursive, *attachment); err != nil { + if *attachment == "" { + return errors.Wrapf(err, "signing %v", args) } + return errors.Wrapf(err, "signing attachement %s for image %v", *attachment, args) } return nil }, @@ -257,12 +255,7 @@ func getTransitiveImages(rootIndex *remote.Descriptor, repo name.Repository, opt } func SignCmd(ctx context.Context, ko KeyOpts, annotations map[string]interface{}, - inputImg string, certPath string, upload bool, payloadPath string, force bool, recursive bool, attachment string) error { - // A key file or token is required unless we're in experimental mode! - imageRef, err := getAttachedImageRef(ctx, inputImg, attachment) - if err != nil { - return fmt.Errorf("unable to resolve attachment %s for image %s", attachment, inputImg) - } + imgs []string, certPath string, upload bool, payloadPath string, force bool, recursive bool, attachment string) error { if EnableExperimental() { if nOf(ko.KeyRef, ko.Sk) > 1 { @@ -279,27 +272,36 @@ func SignCmd(ctx context.Context, ko KeyOpts, annotations map[string]interface{} remote.WithContext(ctx), } - ref, err := name.ParseReference(imageRef) - if err != nil { - return errors.Wrap(err, "parsing reference") - } - get, err := remote.Get(ref, remoteOpts...) - if err != nil { - return errors.Wrap(err, "getting remote image") - } + var toSign []name.Digest + for _, inputImg := range imgs { - repo := ref.Context() - img := repo.Digest(get.Digest.String()) - - toSign := []name.Digest{img} + // A key file or token is required unless we're in experimental mode! + imageRef, err := getAttachedImageRef(ctx, inputImg, attachment) + if err != nil { + return fmt.Errorf("unable to resolve attachment %s for image %s", attachment, inputImg) + } - if recursive && get.MediaType.IsIndex() { - imgs, err := getTransitiveImages(get, repo, remoteOpts...) + ref, err := name.ParseReference(imageRef) if err != nil { - return err + return errors.Wrap(err, "parsing reference") + } + get, err := remote.Get(ref, remoteOpts...) + if err != nil { + return errors.Wrap(err, "getting remote image") + } + + repo := ref.Context() + toSign = append(toSign, repo.Digest(get.Digest.String())) + + if recursive && get.MediaType.IsIndex() { + imgs, err := getTransitiveImages(get, repo, remoteOpts...) + if err != nil { + return err + } + toSign = append(toSign, imgs...) } - toSign = append(toSign, imgs...) } + sv, err := signerFromKeyOpts(ctx, certPath, ko) if err != nil { return errors.Wrap(err, "getting signer") @@ -339,7 +341,7 @@ func SignCmd(ctx context.Context, ko KeyOpts, annotations map[string]interface{} continue } - sigRepo, err := TargetRepositoryForImage(ref) + sigRepo, err := TargetRepositoryForImage(img) if err != nil { return err } @@ -357,7 +359,7 @@ func SignCmd(ctx context.Context, ko KeyOpts, annotations map[string]interface{} } // Check if the image is public (no auth in Get) - uploadTLog, err := shouldUploadToTlog(ref, force, ko.RekorURL) + uploadTLog, err := shouldUploadToTlog(img, force, ko.RekorURL) if err != nil { return err } diff --git a/cmd/cosign/cli/sign_test.go b/cmd/cosign/cli/sign_test.go index 06a37e15c7f..fbe10140c34 100644 --- a/cmd/cosign/cli/sign_test.go +++ b/cmd/cosign/cli/sign_test.go @@ -34,7 +34,7 @@ func TestSignCmdLocalKeyAndSk(t *testing.T) { Sk: true, }, } { - err := SignCmd(ctx, ko, nil, "", "", false, "", false, false, "") + err := SignCmd(ctx, ko, nil, nil, "", false, "", false, false, "") if (errors.Is(err, &KeyParseError{}) == false) { t.Fatal("expected KeyParseError") } From 7e30323db191714e78bc4c5a53242fe2824545b3 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Tue, 14 Sep 2021 12:29:12 -0700 Subject: [PATCH 2/2] Update the e2e tests as well Signed-off-by: Matt Moore --- test/e2e_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/e2e_test.go b/test/e2e_test.go index 7276d48c203..a6b42f768f2 100644 --- a/test/e2e_test.go +++ b/test/e2e_test.go @@ -97,7 +97,7 @@ func TestSignVerify(t *testing.T) { // Now sign the image ko := cli.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc} - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // Now verify and download should work! must(verify(pubKeyPath, imgName, true, nil, ""), t) @@ -108,7 +108,7 @@ func TestSignVerify(t *testing.T) { // Sign the image with an annotation annotations := map[string]interface{}{"foo": "bar"} - must(cli.SignCmd(ctx, ko, annotations, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, annotations, []string{imgName}, "", true, "", false, false, ""), t) // It should match this time. must(verify(pubKeyPath, imgName, true, map[string]interface{}{"foo": "bar"}, ""), t) @@ -132,7 +132,7 @@ func TestSignVerifyClean(t *testing.T) { // Now sign the image ko := cli.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc} - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // Now verify and download should work! must(verify(pubKeyPath, imgName, true, nil, ""), t) @@ -206,7 +206,7 @@ func TestBundle(t *testing.T) { } // Sign the image - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // Make sure verify works must(verify(pubKeyPath, imgName, true, nil, ""), t) @@ -236,14 +236,14 @@ func TestDuplicateSign(t *testing.T) { // Now sign the image ko := cli.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc} - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // Now verify and download should work! must(verify(pubKeyPath, imgName, true, nil, ""), t) must(download.SignatureCmd(ctx, imgName), t) // Signing again should work just fine... - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // but a duplicate signature should not be a uploaded sigRepo, err := cli.TargetRepositoryForImage(ref) if err != nil { @@ -337,14 +337,14 @@ func TestMultipleSignatures(t *testing.T) { // Now sign the image with one key ko := cli.KeyOpts{KeyRef: priv1, PassFunc: passFunc} - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // Now verify should work with that one, but not the other must(verify(pub1, imgName, true, nil, ""), t) mustErr(verify(pub2, imgName, true, nil, ""), t) // Now sign with the other key too ko.KeyRef = priv2 - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // Now verify should work with both must(verify(pub1, imgName, true, nil, ""), t) @@ -626,7 +626,7 @@ func TestAttachSBOM(t *testing.T) { // Now sign the sbom with one key ko1 := cli.KeyOpts{KeyRef: privKeyPath1, PassFunc: passFunc} - must(cli.SignCmd(ctx, ko1, nil, imgName, "", true, "", false, false, "sbom"), t) + must(cli.SignCmd(ctx, ko1, nil, []string{imgName}, "", true, "", false, false, "sbom"), t) // Now verify should work with that one, but not the other must(verify(pubKeyPath1, imgName, true, nil, "sbom"), t) @@ -664,7 +664,7 @@ func TestTlog(t *testing.T) { PassFunc: passFunc, RekorURL: rekorURL, } - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // Now verify should work! must(verify(pubKeyPath, imgName, true, nil, ""), t) @@ -676,7 +676,7 @@ func TestTlog(t *testing.T) { mustErr(verify(pubKeyPath, imgName, true, nil, ""), t) // Sign again with the tlog env var on - must(cli.SignCmd(ctx, ko, nil, imgName, "", true, "", false, false, ""), t) + must(cli.SignCmd(ctx, ko, nil, []string{imgName}, "", true, "", false, false, ""), t) // And now verify works! must(verify(pubKeyPath, imgName, true, nil, ""), t) }