diff --git a/cmd/cosign/cli/verify/verify.go b/cmd/cosign/cli/verify/verify.go index 8c589efb337f..6f974c7ef2c2 100644 --- a/cmd/cosign/cli/verify/verify.go +++ b/cmd/cosign/cli/verify/verify.go @@ -146,13 +146,20 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) { } co.SigVerifier = pubKey + // NB: There are only 2 kinds of verification right now: + // 1. You gave us the public key explicitly to verify against so co.SigVerifier is non-nil or, + // 2. We're going to find an x509 certificate on the signature and verify against Fulcio root trust + // TODO(nsmith5): Refactor this verification logic to pass back _how_ verification + // was performed so we don't need to use this fragile logic here. + fulcioVerified := (co.SigVerifier == nil) + for _, img := range images { if c.LocalImage { verified, bundleVerified, err := cosign.VerifyLocalImageSignatures(ctx, img, co) if err != nil { return err } - PrintVerificationHeader(img, co, bundleVerified) + PrintVerificationHeader(img, co, bundleVerified, fulcioVerified) PrintVerification(img, verified, c.Output) } else { ref, err := name.ParseReference(img) @@ -169,7 +176,7 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) { return err } - PrintVerificationHeader(ref.Name(), co, bundleVerified) + PrintVerificationHeader(ref.Name(), co, bundleVerified, fulcioVerified) PrintVerification(ref.Name(), verified, c.Output) } } @@ -177,7 +184,7 @@ func (c *VerifyCommand) Exec(ctx context.Context, images []string) (err error) { return nil } -func PrintVerificationHeader(imgRef string, co *cosign.CheckOpts, bundleVerified bool) { +func PrintVerificationHeader(imgRef string, co *cosign.CheckOpts, bundleVerified, fulcioVerified bool) { fmt.Fprintf(os.Stderr, "\nVerification for %s --\n", imgRef) fmt.Fprintln(os.Stderr, "The following checks were performed on each of these signatures:") if co.ClaimVerifier != nil { @@ -195,7 +202,9 @@ func PrintVerificationHeader(imgRef string, co *cosign.CheckOpts, bundleVerified if co.SigVerifier != nil { fmt.Fprintln(os.Stderr, " - The signatures were verified against the specified public key") } - fmt.Fprintln(os.Stderr, " - Any certificates were verified against the Fulcio roots.") + if fulcioVerified { + fmt.Fprintln(os.Stderr, " - Any certificates were verified against the Fulcio roots.") + } } // PrintVerification logs details about the verification to stdout diff --git a/cmd/cosign/cli/verify/verify_attestation.go b/cmd/cosign/cli/verify/verify_attestation.go index a734218dd16f..8d414df03639 100644 --- a/cmd/cosign/cli/verify/verify_attestation.go +++ b/cmd/cosign/cli/verify/verify_attestation.go @@ -127,6 +127,13 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e } } + // NB: There are only 2 kinds of verification right now: + // 1. You gave us the public key explicitly to verify against so co.SigVerifier is non-nil or, + // 2. We're going to find an x509 certificate on the signature and verify against Fulcio root trust + // TODO(nsmith5): Refactor this verification logic to pass back _how_ verification + // was performed so we don't need to use this fragile logic here. + fulcioVerified := (co.SigVerifier == nil) + for _, imageRef := range images { var verified []oci.Signature var bundleVerified bool @@ -267,7 +274,7 @@ func (c *VerifyAttestationCommand) Exec(ctx context.Context, images []string) (e } // TODO: add CUE validation report to `PrintVerificationHeader`. - PrintVerificationHeader(imageRef, co, bundleVerified) + PrintVerificationHeader(imageRef, co, bundleVerified, fulcioVerified) // The attestations are always JSON, so use the raw "text" mode for outputting them instead of conversion PrintVerification(imageRef, verified, "text") } diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go index 40ea2136d120..70846fd3af6d 100644 --- a/pkg/cosign/verify.go +++ b/pkg/cosign/verify.go @@ -586,6 +586,10 @@ func VerifyBundle(ctx context.Context, sig oci.Signature) (bool, error) { return false, nil } + if err := compareSigs(bundle.Payload.Body.(string), sig); err != nil { + return false, err + } + publicKeys, err := GetRekorPubs(ctx) if err != nil { return false, errors.Wrap(err, "retrieving rekor public key") @@ -637,6 +641,31 @@ func VerifyBundle(ctx context.Context, sig oci.Signature) (bool, error) { return true, nil } +// compare bundle signature to the signature we are verifying +func compareSigs(bundleBody string, sig oci.Signature) error { + // TODO(nsmith5): modify function signature to make it more clear _why_ + // we've returned nil (there are several reasons possible here). + actualSig, err := sig.Base64Signature() + if err != nil { + return errors.Wrap(err, "base64 signature") + } + if actualSig == "" { + // NB: empty sig means this is an attestation + return nil + } + bundleSignature, err := bundleSig(bundleBody) + if err != nil { + return errors.Wrap(err, "failed to extract signature from bundle") + } + if bundleSignature == "" { + return nil + } + if bundleSignature != actualSig { + return fmt.Errorf("signature in bundle does not match signature being verified") + } + return nil +} + func bundleHash(bundleBody, signature string) (string, string, error) { var toto models.Intoto var rekord models.Rekord @@ -698,6 +727,44 @@ func bundleHash(bundleBody, signature string) (string, string, error) { return *hrekordObj.Data.Hash.Algorithm, *hrekordObj.Data.Hash.Value, nil } +// bundleSig extracts the signature from the rekor bundle body +func bundleSig(bundleBody string) (string, error) { + var rekord models.Rekord + var hrekord models.Hashedrekord + var rekordObj models.RekordV001Schema + var hrekordObj models.HashedrekordV001Schema + + bodyDecoded, err := base64.StdEncoding.DecodeString(bundleBody) + if err != nil { + return "", errors.Wrap(err, "decoding bundleBody") + } + + // Try Rekord + if err := json.Unmarshal(bodyDecoded, &rekord); err == nil { + specMarshal, err := json.Marshal(rekord.Spec) + if err != nil { + return "", err + } + if err := json.Unmarshal(specMarshal, &rekordObj); err != nil { + return "", err + } + return rekordObj.Signature.Content.String(), nil + } + + // Try hashedRekordObj + if err := json.Unmarshal(bodyDecoded, &hrekord); err != nil { + return "", err + } + specMarshal, err := json.Marshal(hrekord.Spec) + if err != nil { + return "", err + } + if err := json.Unmarshal(specMarshal, &hrekordObj); err != nil { + return "", err + } + return hrekordObj.Signature.Content.String(), nil +} + func VerifySET(bundlePayload cbundle.RekorPayload, signature []byte, pub *ecdsa.PublicKey) error { contents, err := json.Marshal(bundlePayload) if err != nil { diff --git a/pkg/cosign/verify_test.go b/pkg/cosign/verify_test.go index adf4999fda21..3839a35a1d81 100644 --- a/pkg/cosign/verify_test.go +++ b/pkg/cosign/verify_test.go @@ -26,6 +26,7 @@ import ( "github.com/in-toto/in-toto-golang/in_toto" "github.com/pkg/errors" "github.com/secure-systems-lab/go-securesystemslib/dsse" + "github.com/sigstore/cosign/pkg/oci/static" "github.com/sigstore/cosign/pkg/types" "github.com/sigstore/cosign/test" "github.com/sigstore/sigstore/pkg/signature" @@ -196,3 +197,40 @@ func TestValidateAndUnpackCertInvalidEmail(t *testing.T) { _, err := ValidateAndUnpackCert(leafCert, co) require.Contains(t, err.Error(), "expected email not found in certificate") } + +func TestCompareSigs(t *testing.T) { + //TODO(nsmith5): Add test cases for invalid signature, missing signature etc + tests := []struct { + description string + b64sig string + bundleBody string + shouldErr bool + }{ + { + description: "sigs match", + b64sig: "MEQCIDO3XHbLovPWK+bk8ItCig2cwlr/8MXbLvz3UFzxMGIMAiA1lqdM9IqqUvCUqzOjufTq3sKU3qSn7R5tPqPz0ddNwQ==", + bundleBody: `eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoiaGFzaGVkcmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiIzODE1MmQxZGQzMjZhZjQwNWY4OTlkYmNjMmNlMzUwYjVmMTZkNDVkZjdmMjNjNDg4ZjQ4NTBhZmExY2Q4NmQxIn19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FUUNJRE8zWEhiTG92UFdLK2JrOEl0Q2lnMmN3bHIvOE1YYkx2ejNVRnp4TUdJTUFpQTFscWRNOUlxcVV2Q1Vxek9qdWZUcTNzS1UzcVNuN1I1dFBxUHowZGROd1E9PSIsInB1YmxpY0tleSI6eyJjb250ZW50IjoiTFMwdExTMUNSVWRKVGlCUVZVSk1TVU1nUzBWWkxTMHRMUzBLVFVacmQwVjNXVWhMYjFwSmVtb3dRMEZSV1VsTGIxcEplbW93UkVGUlkwUlJaMEZGVUN0RVIyb3ZXWFV4VG5vd01XVjVSV2hVZDNRMlQya3hXV3BGWXdwSloxRldjRlZTTjB0bUwwSm1hVk16Y1ZReFVHd3dkbGh3ZUZwNVMyWkpSMHMyZWxoQ04ybE5aV3RFVTA1M1dHWldPSEpKYUdaMmRrOW5QVDBLTFMwdExTMUZUa1FnVUZWQ1RFbERJRXRGV1MwdExTMHRDZz09In19fX0=`, + }, + { + description: "sigs don't match", + b64sig: "bm9wZQo=", + bundleBody: `eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoiaGFzaGVkcmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiIzODE1MmQxZGQzMjZhZjQwNWY4OTlkYmNjMmNlMzUwYjVmMTZkNDVkZjdmMjNjNDg4ZjQ4NTBhZmExY2Q4NmQxIn19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FUUNJRE8zWEhiTG92UFdLK2JrOEl0Q2lnMmN3bHIvOE1YYkx2ejNVRnp4TUdJTUFpQTFscWRNOUlxcVV2Q1Vxek9qdWZUcTNzS1UzcVNuN1I1dFBxUHowZGROd1E9PSIsInB1YmxpY0tleSI6eyJjb250ZW50IjoiTFMwdExTMUNSVWRKVGlCUVZVSk1TVU1nUzBWWkxTMHRMUzBLVFVacmQwVjNXVWhMYjFwSmVtb3dRMEZSV1VsTGIxcEplbW93UkVGUlkwUlJaMEZGVUN0RVIyb3ZXWFV4VG5vd01XVjVSV2hVZDNRMlQya3hXV3BGWXdwSloxRldjRlZTTjB0bUwwSm1hVk16Y1ZReFVHd3dkbGh3ZUZwNVMyWkpSMHMyZWxoQ04ybE5aV3RFVTA1M1dHWldPSEpKYUdaMmRrOW5QVDBLTFMwdExTMUZUa1FnVUZWQ1RFbERJRXRGV1MwdExTMHRDZz09In19fX0=`, + shouldErr: true, + }, + } + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + sig, err := static.NewSignature([]byte("payload"), test.b64sig) + if err != nil { + t.Fatalf("failed to create static signature: %v", err) + } + err = compareSigs(test.bundleBody, sig) + if err == nil && test.shouldErr { + t.Fatal("test should have errored") + } + if err != nil && !test.shouldErr { + t.Fatal(err) + } + }) + } +} diff --git a/pkg/sget/sget.go b/pkg/sget/sget.go index 9b4566143e2f..f61e23b80807 100644 --- a/pkg/sget/sget.go +++ b/pkg/sget/sget.go @@ -83,13 +83,20 @@ func (sg *SecureGet) Do(ctx context.Context) error { } if co.SigVerifier != nil || options.EnableExperimental() { + // NB: There are only 2 kinds of verification right now: + // 1. You gave us the public key explicitly to verify against so co.SigVerifier is non-nil or, + // 2. We're going to find an x509 certificate on the signature and verify against Fulcio root trust + // TODO(nsmith5): Refactor this verification logic to pass back _how_ verification + // was performed so we don't need to use this fragile logic here. + fulcioVerified := (co.SigVerifier == nil) + co.RootCerts = fulcio.GetRoots() sp, bundleVerified, err := cosign.VerifyImageSignatures(ctx, ref, co) if err != nil { return err } - verify.PrintVerificationHeader(sg.ImageRef, co, bundleVerified) + verify.PrintVerificationHeader(sg.ImageRef, co, bundleVerified, fulcioVerified) verify.PrintVerification(sg.ImageRef, sp, "text") } diff --git a/test/e2e_test.go b/test/e2e_test.go index 55fd29b93ff8..cabe048e38c4 100644 --- a/test/e2e_test.go +++ b/test/e2e_test.go @@ -55,6 +55,7 @@ import ( "github.com/sigstore/cosign/pkg/cosign" "github.com/sigstore/cosign/pkg/cosign/kubernetes" cremote "github.com/sigstore/cosign/pkg/cosign/remote" + "github.com/sigstore/cosign/pkg/oci/mutate" ociremote "github.com/sigstore/cosign/pkg/oci/remote" "github.com/sigstore/cosign/pkg/sget" sigs "github.com/sigstore/cosign/pkg/signature" @@ -1156,3 +1157,94 @@ func registryClientOpts(ctx context.Context) []remote.Option { remote.WithContext(ctx), } } + +// If a signature has a bundle, but *not for that signature*, cosign verification should fail +// This test is pretty long, so here are the basic points: +// 1. Sign image1 with a keypair, store entry in rekor +// 2. Sign image2 with keypair, DO NOT store entry in rekor +// 3. Take the bundle from image1 and store it on the signature in image2 +// 4. Verification of image2 should now fail, since the bundle is for a different signature +func TestInvalidBundle(t *testing.T) { + regName, stop := reg(t) + defer stop() + td := t.TempDir() + + img1 := path.Join(regName, "cosign-e2e") + + imgRef, _, cleanup := mkimage(t, img1) + defer cleanup() + + _, privKeyPath, pubKeyPath := keypair(t, td) + + ctx := context.Background() + + // Sign image1 and store the entry in rekor + // (we're just using it for its bundle) + defer setenv(t, options.ExperimentalEnv, "1")() + remoteOpts := ociremote.WithRemoteOptions(registryClientOpts(ctx)...) + ko := sign.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc, RekorURL: rekorURL} + regOpts := options.RegistryOptions{} + + must(sign.SignCmd(ro, ko, regOpts, nil, []string{img1}, "", true, "", "", "", true, false, ""), t) + // verify image1 + must(verify(pubKeyPath, img1, true, nil, ""), t) + // extract the bundle from image1 + si, err := ociremote.SignedImage(imgRef, remoteOpts) + must(err, t) + imgSigs, err := si.Signatures() + must(err, t) + sigs, err := imgSigs.Get() + must(err, t) + if l := len(sigs); l != 1 { + t.Error("expected one signature") + } + bund, err := sigs[0].Bundle() + must(err, t) + if bund == nil { + t.Fail() + } + + // Now, we move on to image2 + // Sign image2 and DO NOT store the entry in rekor + defer setenv(t, options.ExperimentalEnv, "0")() + img2 := path.Join(regName, "unrelated") + imgRef2, _, cleanup := mkimage(t, img2) + defer cleanup() + must(sign.SignCmd(ro, ko, regOpts, nil, []string{img2}, "", true, "", "", "", false, false, ""), t) + must(verify(pubKeyPath, img2, true, nil, ""), t) + + si2, err := ociremote.SignedEntity(imgRef2, remoteOpts) + must(err, t) + sigs2, err := si2.Signatures() + must(err, t) + gottenSigs2, err := sigs2.Get() + must(err, t) + if len(gottenSigs2) != 1 { + t.Fatal("there should be one signature") + } + sigsTag, err := ociremote.SignatureTag(imgRef2) + if err != nil { + t.Fatal(err) + } + + // At this point, we would mutate the signature to add the bundle annotation + // since we don't have a function for it at the moment, mock this by deleting the signature + // and pushing a new signature with the additional bundle annotation + if err := remote.Delete(sigsTag); err != nil { + t.Fatal(err) + } + mustErr(verify(pubKeyPath, img2, true, nil, ""), t) + + newSig, err := mutate.Signature(gottenSigs2[0], mutate.WithBundle(bund)) + must(err, t) + si2, err = ociremote.SignedEntity(imgRef2, remoteOpts) + must(err, t) + newImage, err := mutate.AttachSignatureToEntity(si2, newSig) + must(err, t) + if err := ociremote.WriteSignatures(sigsTag.Repository, newImage); err != nil { + t.Fatal(err) + } + + // veriyfing image2 now should fail + mustErr(verify(pubKeyPath, img2, true, nil, ""), t) +}