Validate tlog entry when verifying signature via public key. (#1833)

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/mock_rekor_client.go

@@ -0,0 +1,97 @@
+// Copyright 2022 The Sigstore Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+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))
+}
+
+// EntriesClient is a client that implements entries.ClientService for Rekor
+// To use:
+// var mClient client.Rekor
+// mClient.entries = &EntriesClient{}
+type EntriesClient struct {
+	Entries models.LogEntry
+}
+
+func (m *EntriesClient) CreateLogEntry(params *entries.CreateLogEntryParams, opts ...entries.ClientOption) (*entries.CreateLogEntryCreated, error) {
+	return &entries.CreateLogEntryCreated{
+		ETag:     "",
+		Location: "",
+		Payload:  data,
+	}, nil
+}
+
+func (m *EntriesClient) GetLogEntryByIndex(params *entries.GetLogEntryByIndexParams, opts ...entries.ClientOption) (*entries.GetLogEntryByIndexOK, error) {
+	return &entries.GetLogEntryByIndexOK{
+		Payload: data,
+	}, nil
+}
+
+func (m *EntriesClient) GetLogEntryByUUID(params *entries.GetLogEntryByUUIDParams, opts ...entries.ClientOption) (*entries.GetLogEntryByUUIDOK, error) {
+	return &entries.GetLogEntryByUUIDOK{
+		Payload: data,
+	}, nil
+}
+
+func (m *EntriesClient) SearchLogQuery(params *entries.SearchLogQueryParams, opts ...entries.ClientOption) (*entries.SearchLogQueryOK, error) {
+	return &entries.SearchLogQueryOK{
+		Payload: []models.LogEntry{data},
+	}, nil
+}
+
+// TODO: Implement mock
+func (m *EntriesClient) SetTransport(transport runtime.ClientTransport) {
+}

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.EntriesClient{}
 
 	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.EntriesClient{}
+
+	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"