fix: make tlog entry lookups for online verification shard-aware (#2297)

Signed-off-by: Asra Ali <asraa@google.com> fix tests Signed-off-by: Asra Ali <asraa@google.com> lint Signed-off-by: Asra Ali <asraa@google.com> fix online lookup Signed-off-by: Asra Ali <asraa@google.com> Signed-off-by: Asra Ali <asraa@google.com>
sigstore · Oct 1, 2022 · 5d36342 · 5d36342
1 parent 983c364
commit 5d36342
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 40 deletions.
diff --git a/cmd/cosign/cli/verify/verify_blob.go b/cmd/cosign/cli/verify/verify_blob.go
@@ -410,7 +410,26 @@ func tlogFindCertificate(ctx context.Context, rekorClient *client.Rekor,
 func tlogFindEntry(ctx context.Context, client *client.Rekor,
 	blobBytes []byte, sig string, pem []byte) (*models.LogEntryAnon, error) {
 	b64sig := base64.StdEncoding.EncodeToString([]byte(sig))
-	return cosign.FindTlogEntry(ctx, client, b64sig, blobBytes, pem)
+	tlogEntries, err := cosign.FindTlogEntry(ctx, client, b64sig, blobBytes, pem)
+	if err != nil {
+		return nil, err
+	}
+	if len(tlogEntries) == 0 {
+		return nil, fmt.Errorf("no valid tlog entries found with proposed entry")
+	}
+	// Always return the earliest integrated entry. That
+	// always suffices for verification of signature time.
+	var earliestLogEntry models.LogEntryAnon
+	var earliestLogEntryTime *time.Time
+	// We'll always return a tlog entry because there's at least one entry in the log.
+	for _, entry := range tlogEntries {
+		entryTime := time.Unix(*entry.IntegratedTime, 0)
+		if earliestLogEntryTime == nil || entryTime.Before(*earliestLogEntryTime) {
+			earliestLogEntryTime = &entryTime
+			earliestLogEntry = entry
+		}
+	}
+	return &earliestLogEntry, nil
 }
 
 // signatures returns the raw signature

diff --git a/cmd/cosign/cli/verify/verify_blob_test.go b/cmd/cosign/cli/verify/verify_blob_test.go
@@ -248,7 +248,7 @@ func TestVerifyBlob(t *testing.T) {
 		// If online lookups to Rekor are enabled
 		experimental bool
 		// The rekor entry response when Rekor is enabled
-		rekorEntry *models.LogEntry
+		rekorEntry []*models.LogEntry
 		shouldErr  bool
 	}{
 		{
@@ -274,8 +274,8 @@ func TestVerifyBlob(t *testing.T) {
 			signature:    blobSignature,
 			sigVerifier:  signer,
 			experimental: true,
-			rekorEntry: makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
-				pubKeyBytes, true),
+			rekorEntry: []*models.LogEntry{makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
+				pubKeyBytes, true)},
 			shouldErr: false,
 		},
 		{
@@ -411,19 +411,18 @@ func TestVerifyBlob(t *testing.T) {
 			cert:         unexpiredLeafCert,
 			sigVerifier:  signer,
 			experimental: true,
-			rekorEntry: makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
-				unexpiredCertPem, true),
+			rekorEntry: []*models.LogEntry{makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
+				unexpiredCertPem, true)},
 			shouldErr: false,
 		},
-
 		{
 			name:         "valid signature with unexpired certificate - experimental & rekor entry found",
 			blob:         blobBytes,
 			signature:    blobSignature,
 			cert:         unexpiredLeafCert,
 			experimental: true,
-			rekorEntry: makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
-				unexpiredCertPem, true),
+			rekorEntry: []*models.LogEntry{makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
+				unexpiredCertPem, true)},
 			shouldErr: false,
 		},
 		{
@@ -442,8 +441,20 @@ func TestVerifyBlob(t *testing.T) {
 			sigVerifier:  signer,
 			cert:         expiredLeafCert,
 			experimental: true,
-			rekorEntry: makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
-				expiredLeafPem, true),
+			rekorEntry: []*models.LogEntry{makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
+				expiredLeafPem, true)},
+			shouldErr: false,
+		},
+		{
+			name:         "valid signature with expired certificate - experimental multiple rekor entries",
+			blob:         blobBytes,
+			signature:    blobSignature,
+			sigVerifier:  signer,
+			cert:         expiredLeafCert,
+			experimental: true,
+			rekorEntry: []*models.LogEntry{makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
+				expiredLeafPem, true), makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
+				expiredLeafPem, false)},
 			shouldErr: false,
 		},
 		{
@@ -453,8 +464,8 @@ func TestVerifyBlob(t *testing.T) {
 			cert:         expiredLeafCert,
 			sigVerifier:  signer,
 			experimental: true,
-			rekorEntry: makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
-				expiredLeafPem, false),
+			rekorEntry: []*models.LogEntry{makeRekorEntry(t, *rekorSigner, blobBytes, []byte(blobSignature),
+				expiredLeafPem, false)},
 			shouldErr: true,
 		},
 
@@ -521,8 +532,8 @@ func TestVerifyBlob(t *testing.T) {
 			cert:         expiredLeafCert,
 			experimental: true,
 			// This is the wrong signer for the SET!
-			rekorEntry: makeRekorEntry(t, *signer, blobBytes, []byte(blobSignature),
-				expiredLeafPem, true),
+			rekorEntry: []*models.LogEntry{makeRekorEntry(t, *signer, blobBytes, []byte(blobSignature),
+				expiredLeafPem, true)},
 			shouldErr: true,
 		},
 	}

diff --git a/internal/pkg/cosign/rekor/mock/mock_rekor_client.go b/internal/pkg/cosign/rekor/mock/mock_rekor_client.go
@@ -28,15 +28,15 @@ import (
 // var mClient client.Rekor
 // mClient.Entries = &logEntry
 type EntriesClient struct {
-	Entries *models.LogEntry
+	Entries []*models.LogEntry
 }
 
 func (m *EntriesClient) CreateLogEntry(params *entries.CreateLogEntryParams, opts ...entries.ClientOption) (*entries.CreateLogEntryCreated, error) {
 	if m.Entries != nil {
 		return &entries.CreateLogEntryCreated{
 			ETag:     "",
 			Location: "",
-			Payload:  *m.Entries,
+			Payload:  *m.Entries[0],
 		}, nil
 	}
 	return nil, errors.New("entry not provided")
@@ -45,7 +45,7 @@ func (m *EntriesClient) CreateLogEntry(params *entries.CreateLogEntryParams, opt
 func (m *EntriesClient) GetLogEntryByIndex(params *entries.GetLogEntryByIndexParams, opts ...entries.ClientOption) (*entries.GetLogEntryByIndexOK, error) {
 	if m.Entries != nil {
 		return &entries.GetLogEntryByIndexOK{
-			Payload: *m.Entries,
+			Payload: *m.Entries[0],
 		}, nil
 	}
 	return nil, errors.New("entry not provided")
@@ -54,7 +54,7 @@ func (m *EntriesClient) GetLogEntryByIndex(params *entries.GetLogEntryByIndexPar
 func (m *EntriesClient) GetLogEntryByUUID(params *entries.GetLogEntryByUUIDParams, opts ...entries.ClientOption) (*entries.GetLogEntryByUUIDOK, error) {
 	if m.Entries != nil {
 		return &entries.GetLogEntryByUUIDOK{
-			Payload: *m.Entries,
+			Payload: *m.Entries[0],
 		}, nil
 	}
 	return nil, errors.New("entry not provided")
@@ -63,7 +63,9 @@ func (m *EntriesClient) GetLogEntryByUUID(params *entries.GetLogEntryByUUIDParam
 func (m *EntriesClient) SearchLogQuery(params *entries.SearchLogQueryParams, opts ...entries.ClientOption) (*entries.SearchLogQueryOK, error) {
 	resp := []models.LogEntry{}
 	if m.Entries != nil {
-		resp = append(resp, *m.Entries)
+		for _, entry := range m.Entries {
+			resp = append(resp, *entry)
+		}
 	}
 	return &entries.SearchLogQueryOK{
 		Payload: resp,

diff --git a/internal/pkg/cosign/rekor/signer_test.go b/internal/pkg/cosign/rekor/signer_test.go
@@ -52,9 +52,9 @@ func TestSigner(t *testing.T) {
 	var mClient client.Rekor
 
 	mClient.Entries = &mock.EntriesClient{
-		Entries: &models.LogEntry{"123": models.LogEntryAnon{
+		Entries: []*models.LogEntry{{"123": models.LogEntryAnon{
 			LogIndex: swag.Int64(123),
-		}},
+		}}},
 	}
 
 	testSigner := NewSigner(payloadSigner, &mClient)

diff --git a/pkg/cosign/tlog.go b/pkg/cosign/tlog.go
@@ -389,7 +389,8 @@ func proposedEntry(b64Sig string, payload, pubKey []byte) ([]models.ProposedEntr
 	return proposedEntry, nil
 }
 
-func FindTlogEntry(ctx context.Context, rekorClient *client.Rekor, b64Sig string, payload, pubKey []byte) (entry *models.LogEntryAnon, err error) {
+func FindTlogEntry(ctx context.Context, rekorClient *client.Rekor,
+	b64Sig string, payload, pubKey []byte) ([]models.LogEntryAnon, error) {
 	searchParams := entries.NewSearchLogQueryParamsWithContext(ctx)
 	searchLogQuery := models.SearchLogQuery{}
 	proposedEntry, err := proposedEntry(b64Sig, payload, pubKey)
@@ -406,23 +407,21 @@ func FindTlogEntry(ctx context.Context, rekorClient *client.Rekor, b64Sig string
 	}
 	if len(resp.Payload) == 0 {
 		return nil, errors.New("signature not found in transparency log")
-	} else if len(resp.Payload) > 1 {
-		return nil, errors.New("multiple entries returned; this should not happen")
-	}
-	logEntry := resp.Payload[0]
-	if len(logEntry) != 1 {
-		return nil, errors.New("UUID value can not be extracted")
 	}
 
-	var tlogEntry models.LogEntryAnon
-	for k, e := range logEntry {
-		// Check body hash matches uuid
-		if err := verifyUUID(k, e); err != nil {
-			return nil, err
+	// This may accumulate multiple entries on multiple tree IDs.
+	results := make([]models.LogEntryAnon, 0)
+	for _, logEntry := range resp.GetPayload() {
+		for k, e := range logEntry {
+			// Check body hash matches uuid
+			if err := verifyUUID(k, e); err != nil {
+				continue
+			}
+			results = append(results, e)
 		}
-		tlogEntry = e
 	}
-	return &tlogEntry, nil
+
+	return results, nil
 }
 
 func FindTLogEntriesByPayload(ctx context.Context, rekorClient *client.Rekor, payload []byte) (uuids []string, err error) {

diff --git a/pkg/cosign/verify.go b/pkg/cosign/verify.go
@@ -410,11 +410,34 @@ func tlogValidateEntry(ctx context.Context, client *client.Rekor, sig oci.Signat
 	if err != nil {
 		return nil, err
 	}
-	e, err := FindTlogEntry(ctx, client, b64sig, payload, pem)
+	tlogEntries, err := FindTlogEntry(ctx, client, b64sig, payload, pem)
 	if err != nil {
 		return nil, err
 	}
-	return e, VerifyTLogEntry(ctx, client, e)
+	if len(tlogEntries) == 0 {
+		return nil, fmt.Errorf("no valid tlog entries found with proposed entry")
+	}
+	// Always return the earliest integrated entry. That
+	// always suffices for verification of signature time.
+	var earliestLogEntry models.LogEntryAnon
+	var earliestLogEntryTime *time.Time
+	entryVerificationErrs := make([]string, 0)
+	for _, e := range tlogEntries {
+		entry := e
+		if err := VerifyTLogEntry(ctx, client, &entry); err != nil {
+			entryVerificationErrs = append(entryVerificationErrs, err.Error())
+			continue
+		}
+		entryTime := time.Unix(*entry.IntegratedTime, 0)
+		if earliestLogEntryTime == nil || entryTime.Before(*earliestLogEntryTime) {
+			earliestLogEntryTime = &entryTime
+			earliestLogEntry = entry
+		}
+	}
+	if earliestLogEntryTime == nil {
+		return nil, fmt.Errorf("no valid tlog entries found %s", strings.Join(entryVerificationErrs, ", "))
+	}
+	return &earliestLogEntry, nil
 }
 
 type fakeOCISignatures struct {

diff --git a/pkg/cosign/verify_test.go b/pkg/cosign/verify_test.go
@@ -400,7 +400,7 @@ func TestVerifyImageSignatureWithSigVerifierAndRekor(t *testing.T) {
 	// match the underlying data / key)
 	mClient := new(client.Rekor)
 	mClient.Entries = &mock.EntriesClient{
-		Entries: &data,
+		Entries: []*models.LogEntry{&data},
 	}
 
 	if _, err := VerifyImageSignature(context.TODO(), ociSig, v1.Hash{}, &CheckOpts{
@@ -414,7 +414,7 @@ func TestVerifyImageSignatureWithSigVerifierAndRekor(t *testing.T) {
 		// 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")
+		t.Fatalf("expected error while verifying signature, got %s", err)
 	}
 }