From 32f80c0d7e39133cd958e7e2ef4767b0da316d3d Mon Sep 17 00:00:00 2001
From: Matt Moore <mattmoor@chainguard.dev>
Date: Tue, 27 Sep 2022 03:17:32 -0700
Subject: [PATCH] Fix: Remove an extra registry request from verification path.
 (#2285)

:bug: Remove an extra registry request when supplied with a digest.

The current logic calls `getSignedEntity` which must determine the media type of the referenced entity in order to produce an appropriately types `SignedEntity`.  However, we are only interested in the attached signatures (or
attestations) and not the underlying structure, so when we are passed a digest, this roundtrip is entirely superfluous.

This changes this logic to explicitly resolve the digest (since verification wants to know the `v1.Hash`), which is free for digests.  Then we explicitly invoke `ociremote.Signatures()` on one of
`ociremote.{Signature,Attestation}Tag`, which is also free (since we always pass the digest).

/kind bug

Signed-off-by: Matt Moore <mattmoor@chainguard.dev>
---
 pkg/cosign/verify.go | 49 +++++++++++++++++++++++---------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go
index ddba36dbdf8..b063957eff8 100644
--- a/pkg/cosign/verify.go
+++ b/pkg/cosign/verify.go
@@ -118,19 +118,6 @@ type CheckOpts struct {
 	Identities []Identity
 }
 
-func getSignedEntity(signedImgRef name.Reference, regClientOpts []ociremote.Option) (oci.SignedEntity, v1.Hash, error) {
-	se, err := ociremote.SignedEntity(signedImgRef, regClientOpts...)
-	if err != nil {
-		return nil, v1.Hash{}, err
-	}
-	// Both of the SignedEntity types implement Digest()
-	h, err := se.(interface{ Digest() (v1.Hash, error) }).Digest()
-	if err != nil {
-		return nil, v1.Hash{}, err
-	}
-	return se, h, nil
-}
-
 func verifyOCISignature(ctx context.Context, verifier signature.Verifier, sig oci.Signature) error {
 	b64sig, err := sig.Base64Signature()
 	if err != nil {
@@ -427,9 +414,13 @@ func VerifyImageSignatures(ctx context.Context, signedImgRef name.Reference, co
 		return nil, false, errors.New("one of verifier or root certs is required")
 	}
 
-	// TODO(mattmoor): We could implement recursive verification if we just wrapped
-	// most of the logic below here in a call to mutate.Map
-	se, h, err := getSignedEntity(signedImgRef, co.RegistryClientOpts)
+	// This is a carefully optimized sequence for fetching the signatures of the
+	// entity that minimizes registry requests when supplied with a digest input
+	digest, err := ociremote.ResolveDigest(signedImgRef, co.RegistryClientOpts...)
+	if err != nil {
+		return nil, false, err
+	}
+	h, err := v1.NewHash(digest.Identifier())
 	if err != nil {
 		return nil, false, err
 	}
@@ -437,7 +428,11 @@ func VerifyImageSignatures(ctx context.Context, signedImgRef name.Reference, co
 	var sigs oci.Signatures
 	sigRef := co.SignatureRef
 	if sigRef == "" {
-		sigs, err = se.Signatures()
+		st, err := ociremote.SignatureTag(digest, co.RegistryClientOpts...)
+		if err != nil {
+			return nil, false, err
+		}
+		sigs, err = ociremote.Signatures(st, co.RegistryClientOpts...)
 		if err != nil {
 			return nil, false, err
 		}
@@ -625,7 +620,7 @@ func loadSignatureFromFile(sigRef string, signedImgRef name.Reference, co *Check
 	}, nil
 }
 
-// VerifyAttestations does all the main cosign checks in a loop, returning the verified attestations.
+// VerifyImageAttestations does all the main cosign checks in a loop, returning the verified attestations.
 // If there were no valid attestations, we return an error.
 func VerifyImageAttestations(ctx context.Context, signedImgRef name.Reference, co *CheckOpts) (checkedAttestations []oci.Signature, bundleVerified bool, err error) {
 	// Enforce this up front.
@@ -633,14 +628,22 @@ func VerifyImageAttestations(ctx context.Context, signedImgRef name.Reference, c
 		return nil, false, errors.New("one of verifier or root certs is required")
 	}
 
-	// TODO(mattmoor): We could implement recursive verification if we just wrapped
-	// most of the logic below here in a call to mutate.Map
-
-	se, h, err := getSignedEntity(signedImgRef, co.RegistryClientOpts)
+	// This is a carefully optimized sequence for fetching the attestations of
+	// the entity that minimizes registry requests when supplied with a digest
+	// input.
+	digest, err := ociremote.ResolveDigest(signedImgRef, co.RegistryClientOpts...)
 	if err != nil {
 		return nil, false, err
 	}
-	atts, err := se.Attestations()
+	h, err := v1.NewHash(digest.Identifier())
+	if err != nil {
+		return nil, false, err
+	}
+	st, err := ociremote.AttestationTag(digest, co.RegistryClientOpts...)
+	if err != nil {
+		return nil, false, err
+	}
+	atts, err := ociremote.Signatures(st, co.RegistryClientOpts...)
 	if err != nil {
 		return nil, false, err
 	}