Validate tlog entry when verifying signature via public key.

Previously, when signatures were validated via public key, we only
checked for the existence of the tlog, but didn't actually validate the
entry. This refactors the code to ensure both cert + public paths go
through the same tlog entry validation.

Signed-off-by: Billy Lynch <billy@chainguard.dev>

internal/pkg/cosign/rekor/mock_rekor_client.go → internal/pkg/cosign/rekor/mock/mock_rekor_client.go

@@ -12,51 +12,84 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package rekor
+package mock
 
 import (
+	"encoding/base64"
+	"encoding/hex"
+
 	"github.com/go-openapi/runtime"
+	"github.com/google/trillian/merkle/rfc6962"
 	"github.com/sigstore/rekor/pkg/generated/client/entries"
 	"github.com/sigstore/rekor/pkg/generated/models"
 )
 
+var (
+	lea = models.LogEntryAnon{
+		Attestation:    &models.LogEntryAnonAttestation{},
+		Body:           base64.StdEncoding.EncodeToString([]byte("asdf")),
+		IntegratedTime: new(int64),
+		LogID:          new(string),
+		LogIndex:       new(int64),
+		Verification: &models.LogEntryAnonVerification{
+			InclusionProof: &models.InclusionProof{
+				RootHash: new(string),
+				TreeSize: new(int64),
+				LogIndex: new(int64),
+			},
+		},
+	}
+	data = models.LogEntry{
+		uuid(lea): lea,
+	}
+)
+
+// uuid generates the UUID for the given LogEntry.
+// This is effectively a reimplementation of
+// pkg/cosign/tlog.go -> verifyUUID / ComputeLeafHash, but separated
+// to avoid a circular dependency.
+// TODO?: Perhaps we should refactor the tlog libraries into a separate
+// package?
+func uuid(e models.LogEntryAnon) string {
+	entryBytes, err := base64.StdEncoding.DecodeString(e.Body.(string))
+	if err != nil {
+		panic(err)
+	}
+	return hex.EncodeToString(rfc6962.DefaultHasher.HashLeaf(entryBytes))
+}
+
 // Client that implements entries.ClientService for Rekor
 // To use:
 // var mClient client.Rekor
 // mClient.entries = &MockEntriesClient{}
 type MockEntriesClient struct {
+	Entries models.LogEntry
 }
 
 func (m *MockEntriesClient) CreateLogEntry(params *entries.CreateLogEntryParams, opts ...entries.ClientOption) (*entries.CreateLogEntryCreated, error) {
 	return &entries.CreateLogEntryCreated{
 		ETag:     "",
 		Location: "",
-		Payload: map[string]models.LogEntryAnon{
-			"sdf": {
-				Attestation:    &models.LogEntryAnonAttestation{},
-				Body:           nil,
-				IntegratedTime: new(int64),
-				LogID:          new(string),
-				LogIndex:       new(int64),
-				Verification:   &models.LogEntryAnonVerification{},
-			},
-		},
+		Payload:  data,
 	}, nil
 }
 
-// TODO: Implement mock
 func (m *MockEntriesClient) GetLogEntryByIndex(params *entries.GetLogEntryByIndexParams, opts ...entries.ClientOption) (*entries.GetLogEntryByIndexOK, error) {
-	return nil, nil
+	return &entries.GetLogEntryByIndexOK{
+		Payload: data,
+	}, nil
 }
 
-// TODO: Implement mock
 func (m *MockEntriesClient) GetLogEntryByUUID(params *entries.GetLogEntryByUUIDParams, opts ...entries.ClientOption) (*entries.GetLogEntryByUUIDOK, error) {
-	return nil, nil
+	return &entries.GetLogEntryByUUIDOK{
+		Payload: data,
+	}, nil
 }
 
-// TODO: Implement mock
 func (m *MockEntriesClient) SearchLogQuery(params *entries.SearchLogQueryParams, opts ...entries.ClientOption) (*entries.SearchLogQueryOK, error) {
-	return nil, nil
+	return &entries.SearchLogQueryOK{
+		Payload: []models.LogEntry{data},
+	}, nil
 }
 
 // TODO: Implement mock

internal/pkg/cosign/rekor/signer_test.go

@@ -23,6 +23,7 @@ import (
 	"testing"
 
 	"github.com/sigstore/cosign/internal/pkg/cosign/payload"
+	"github.com/sigstore/cosign/internal/pkg/cosign/rekor/mock"
 	"github.com/sigstore/cosign/pkg/cosign"
 	"github.com/sigstore/rekor/pkg/generated/client"
 	"github.com/sigstore/sigstore/pkg/signature"
@@ -47,7 +48,7 @@ func TestSigner(t *testing.T) {
 
 	// Mock out Rekor client
 	var mClient client.Rekor
-	mClient.Entries = &MockEntriesClient{}
+	mClient.Entries = &mock.MockEntriesClient{}
 
 	testSigner := NewSigner(payloadSigner, &mClient)
 

pkg/cosign/verify.go

@@ -303,15 +303,7 @@ func tlogValidatePublicKey(ctx context.Context, rekorClient *client.Rekor, pub c
 	if err != nil {
 		return err
 	}
-	b64sig, err := sig.Base64Signature()
-	if err != nil {
-		return err
-	}
-	payload, err := sig.Payload()
-	if err != nil {
-		return err
-	}
-	_, err = FindTlogEntry(ctx, rekorClient, b64sig, payload, pemBytes)
+	_, err = tlogValidateEntry(ctx, rekorClient, sig, pemBytes)
 	return err
 }
 
@@ -324,23 +316,28 @@ func tlogValidateCertificate(ctx context.Context, rekorClient *client.Rekor, sig
 	if err != nil {
 		return err
 	}
-	b64sig, err := sig.Base64Signature()
+	e, err := tlogValidateEntry(ctx, rekorClient, sig, pemBytes)
 	if err != nil {
 		return err
 	}
-	payload, err := sig.Payload()
+	// if we have a cert, we should check expiry
+	return CheckExpiry(cert, time.Unix(*e.IntegratedTime, 0))
+}
+
+func tlogValidateEntry(ctx context.Context, client *client.Rekor, sig oci.Signature, pem []byte) (*models.LogEntryAnon, error) {
+	b64sig, err := sig.Base64Signature()
 	if err != nil {
-		return err
+		return nil, err
 	}
-	e, err := FindTlogEntry(ctx, rekorClient, b64sig, payload, pemBytes)
+	payload, err := sig.Payload()
 	if err != nil {
-		return err
+		return nil, err
 	}
-	if err := VerifyTLogEntry(ctx, rekorClient, e); err != nil {
-		return err
+	e, err := FindTlogEntry(ctx, client, b64sig, payload, pem)
+	if err != nil {
+		return nil, err
 	}
-	// if we have a cert, we should check expiry
-	return CheckExpiry(cert, time.Unix(*e.IntegratedTime, 0))
+	return e, VerifyTLogEntry(ctx, client, e)
 }
 
 type fakeOCISignatures struct {

pkg/cosign/verify_test.go

@@ -40,11 +40,13 @@ 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/internal/pkg/cosign/rekor/mock"
 	"github.com/sigstore/cosign/pkg/cosign/bundle"
 	ctuf "github.com/sigstore/cosign/pkg/cosign/tuf"
 	"github.com/sigstore/cosign/pkg/oci/static"
 	"github.com/sigstore/cosign/pkg/types"
 	"github.com/sigstore/cosign/test"
+	"github.com/sigstore/rekor/pkg/generated/client"
 	rtypes "github.com/sigstore/rekor/pkg/types"
 	"github.com/sigstore/sigstore/pkg/cryptoutils"
 	"github.com/sigstore/sigstore/pkg/signature"
@@ -345,6 +347,41 @@ func TestVerifyImageSignatureWithExistingSub(t *testing.T) {
 	}
 }
 
+// This test ensures that image signature validation fails properly if we are
+// using a SigVerifier with Rekor.
+// See https://github.com/sigstore/cosign/issues/1816 for more details.
+func TestVerifyImageSignatureWithSigVerifierAndRekor(t *testing.T) {
+	sv, privKey, err := signature.NewDefaultECDSASignerVerifier()
+	if err != nil {
+		t.Fatalf("error generating verifier: %v", err)
+	}
+
+	payload := []byte{1, 2, 3, 4}
+	h := sha256.Sum256(payload)
+	sig, _ := privKey.Sign(rand.Reader, h[:], crypto.SHA256)
+	ociSig, _ := static.NewSignature(payload, base64.StdEncoding.EncodeToString(sig))
+
+	// Add a fake rekor client - this makes it look like there's a matching
+	// tlog entry for the signature during validation (even though it does not
+	// match the underlying data / key)
+	mClient := new(client.Rekor)
+	mClient.Entries = &mock.MockEntriesClient{}
+
+	if _, err := VerifyImageSignature(context.TODO(), ociSig, v1.Hash{}, &CheckOpts{
+		SigVerifier: sv,
+		RekorClient: mClient,
+	}); err == nil || !strings.Contains(err.Error(), "verifying inclusion proof") {
+		// TODO(wlynch): This is a weak test, since this is really failing because
+		// there is no inclusion proof for the Rekor entry rather than failing to
+		// validate the Rekor public key itself. At the very least this ensures
+		// that we're hitting tlog validation during signature checking,
+		// but we should look into improving this once there is an in-memory
+		// Rekor client that is capable of performing inclusion proof validation
+		// in unit tests.
+		t.Fatal("expected error while verifying signature")
+	}
+}
+
 func TestValidateAndUnpackCertSuccess(t *testing.T) {
 	subject := "email@email"
 	oidcIssuer := "https://accounts.google.com"