Don't fail on Rekor entry verification for untrusted entries (#47)

Like #45, as long as the threshold of expected timestamps is met, then verification should succeed. Otherwise, entries without trust root material should be skipped. One benefit of having the log key ID be used to look up the correct trust root material is that we can still error out if the signature is invalid. Ref: #43 Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
sigstore · Jan 3, 2024 · 69a0d81 · 69a0d81
1 parent f7518a6
commit 69a0d81
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 16 deletions.
diff --git a/pkg/tlog/entry.go b/pkg/tlog/entry.go
@@ -230,6 +230,10 @@ func (entry *Entry) LogIndex() int64 {
 	return *entry.logEntryAnon.LogIndex
 }
 
+func (entry *Entry) Body() any {
+	return entry.logEntryAnon.Body
+}
+
 func (entry *Entry) HasInclusionPromise() bool {
 	return entry.signedEntryTimestamp != nil
 }

diff --git a/pkg/verify/sct.go b/pkg/verify/sct.go
@@ -29,6 +29,7 @@ import (
 // leaf certificate, will extract SCTs from the leaf certificate and verify the
 // timestamps using the TrustedMaterial's FulcioCertificateAuthorities() and
 // CTlogAuthorities()
+// TODO(issue#46): Add unit tests
 func VerifySignedCertificateTimestamp(leafCert *x509.Certificate, threshold int, trustedMaterial root.TrustedMaterial) error { // nolint: revive
 	ctlogs := trustedMaterial.CTlogAuthorities()
 	fulcioCerts := trustedMaterial.FulcioCertificateAuthorities()
@@ -48,7 +49,8 @@ func VerifySignedCertificateTimestamp(leafCert *x509.Certificate, threshold int,
 		encodedKeyID := hex.EncodeToString(sct.LogID.KeyID[:])
 		key, ok := ctlogs[encodedKeyID]
 		if !ok {
-			return fmt.Errorf("unable to find ctlogs key for %s", encodedKeyID)
+			// skip entries the trust root cannot verify
+			continue
 		}
 
 		for _, fulcioCa := range fulcioCerts {

diff --git a/pkg/verify/tlog.go b/pkg/verify/tlog.go
@@ -55,9 +55,6 @@ func VerifyArtifactTransparencyLog(entity SignedEntity, trustedMaterial root.Tru
 			}
 		}
 	}
-	if len(entries) < threshold {
-		return nil, fmt.Errorf("not enough transparency log entries: %d < %d", len(entries), threshold)
-	}
 
 	sigContent, err := entity.SignatureContent()
 	if err != nil {
@@ -80,21 +77,23 @@ func VerifyArtifactTransparencyLog(entity SignedEntity, trustedMaterial root.Tru
 		}
 
 		if !online {
-			var inclusionVerified bool
-			// TODO: do we validate that an entry has EITHER a promise OR a proof?
+			if !entry.HasInclusionPromise() && !entry.HasInclusionProof() {
+				return nil, fmt.Errorf("entry must contain an inclusion proof and/or promise")
+			}
 			if entry.HasInclusionPromise() {
 				err = tlog.VerifySET(entry, trustedMaterial.TlogAuthorities())
 				if err != nil {
-					return nil, err
+					// skip entries the trust root cannot verify
+					continue
 				}
-				inclusionVerified = true
 			}
 			if entity.HasInclusionProof() {
 				keyID := entry.LogKeyID()
 				hex64Key := hex.EncodeToString([]byte(keyID))
 				tlogVerifier, ok := trustedMaterial.TlogAuthorities()[hex64Key]
 				if !ok {
-					return nil, fmt.Errorf("unable to find tlog information for key %s", hex64Key)
+					// skip entries the trust root cannot verify
+					continue
 				}
 
 				verifier, err := getVerifier(tlogVerifier.PublicKey, tlogVerifier.SignatureHashFunc)
@@ -106,19 +105,15 @@ func VerifyArtifactTransparencyLog(entity SignedEntity, trustedMaterial root.Tru
 				if err != nil {
 					return nil, err
 				}
-
-				inclusionVerified = true
-			}
-
-			if inclusionVerified {
-				verifiedTimestamps = append(verifiedTimestamps, entry.IntegratedTime())
 			}
+			verifiedTimestamps = append(verifiedTimestamps, entry.IntegratedTime())
 		} else {
 			keyID := entry.LogKeyID()
 			hex64Key := hex.EncodeToString([]byte(keyID))
 			tlogVerifier, ok := trustedMaterial.TlogAuthorities()[hex64Key]
 			if !ok {
-				return nil, fmt.Errorf("unable to find tlog information for key %s", hex64Key)
+				// skip entries the trust root cannot verify
+				continue
 			}
 
 			client, err := getRekorClient(tlogVerifier.BaseURL)
@@ -178,6 +173,10 @@ func VerifyArtifactTransparencyLog(entity SignedEntity, trustedMaterial root.Tru
 		}
 	}
 
+	if len(verifiedTimestamps) < threshold {
+		return nil, fmt.Errorf("not enough verified timestamps from transparency log entries: %d < %d", len(verifiedTimestamps), threshold)
+	}
+
 	return verifiedTimestamps, nil
 }
 

diff --git a/pkg/verify/tlog_test.go b/pkg/verify/tlog_test.go
@@ -15,6 +15,8 @@
 package verify_test
 
 import (
+	"encoding/base64"
+	"strings"
 	"testing"
 	"time"
 
@@ -52,6 +54,112 @@ func TestTlogVerifier(t *testing.T) {
 	assert.Error(t, err)
 }
 
+type oneTrustedOneUntrustedLogEntry struct {
+	*ca.TestEntity
+	UntrustedTestEntity *ca.TestEntity
+}
+
+func (e *oneTrustedOneUntrustedLogEntry) TlogEntries() ([]*tlog.Entry, error) {
+	entries, err := e.TestEntity.TlogEntries()
+	if err != nil {
+		return nil, err
+	}
+
+	otherEntries, err := e.UntrustedTestEntity.TlogEntries()
+	if err != nil {
+		return nil, err
+	}
+
+	return append(entries, otherEntries...), nil
+}
+
+func TestIgnoredTLogEntries(t *testing.T) {
+	statement := []byte(`{"_type":"https://in-toto.io/Statement/v0.1","predicateType":"customFoo","subject":[{"name":"subject","digest":{"sha256":"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"}}],"predicate":{}}`)
+
+	virtualSigstore, err := ca.NewVirtualSigstore()
+	assert.NoError(t, err)
+	entity, err := virtualSigstore.Attest("foo@fighters.com", "issuer", statement)
+	assert.NoError(t, err)
+
+	untrustedSigstore, err := ca.NewVirtualSigstore()
+	assert.NoError(t, err)
+	untrustedEntity, err := untrustedSigstore.Attest("foo@fighters.com", "issuer", statement)
+	assert.NoError(t, err)
+
+	// success: entry that cannot be verified is ignored
+	_, err = verify.VerifyArtifactTransparencyLog(&oneTrustedOneUntrustedLogEntry{entity, untrustedEntity}, virtualSigstore, 1, false)
+	assert.NoError(t, err)
+
+	// failure: threshold of 2 is not met since 1 untrusted entry is ignored
+	_, err = verify.VerifyArtifactTransparencyLog(&oneTrustedOneUntrustedLogEntry{entity, untrustedEntity}, virtualSigstore, 2, false)
+	assert.Error(t, err)
+}
+
+// invalidTLogEntity constructs a bundle with a Rekor response, but without an inclusion proof or promise
+type invalidTLogEntity struct {
+	*ca.TestEntity
+}
+
+func (e *invalidTLogEntity) TlogEntries() ([]*tlog.Entry, error) {
+	entries, err := e.TestEntity.TlogEntries()
+	if err != nil {
+		return nil, err
+	}
+	var invalidEntries []*tlog.Entry
+	for _, entry := range entries {
+		body, err := base64.StdEncoding.DecodeString(entry.Body().(string))
+		if err != nil {
+			return nil, err
+		}
+		invalidEntry, err := tlog.NewEntry(body, entry.IntegratedTime().Unix(), entry.LogIndex(), []byte(entry.LogKeyID()), nil, nil)
+		if err != nil {
+			return nil, err
+		}
+		invalidEntries = append(invalidEntries, invalidEntry)
+	}
+	return invalidEntries, nil
+}
+
+func TestInvalidTLogEntries(t *testing.T) {
+	statement := []byte(`{"_type":"https://in-toto.io/Statement/v0.1","predicateType":"customFoo","subject":[{"name":"subject","digest":{"sha256":"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"}}],"predicate":{}}`)
+
+	virtualSigstore, err := ca.NewVirtualSigstore()
+	assert.NoError(t, err)
+	entity, err := virtualSigstore.Attest("foo@fighters.com", "issuer", statement)
+	assert.NoError(t, err)
+
+	// failure: threshold of 1 is not met with invalid entry
+	_, err = verify.VerifyArtifactTransparencyLog(&invalidTLogEntity{entity}, virtualSigstore, 1, false)
+	assert.Error(t, err)
+	if err.Error() != "entry must contain an inclusion proof and/or promise" {
+		t.Errorf("expected error with missing proof/promises, got: %v", err.Error())
+	}
+}
+
+type noTLogEntity struct {
+	*ca.TestEntity
+}
+
+func (e *noTLogEntity) TlogEntries() ([]*tlog.Entry, error) {
+	return []*tlog.Entry{}, nil
+}
+
+func TestNoTLogEntries(t *testing.T) {
+	statement := []byte(`{"_type":"https://in-toto.io/Statement/v0.1","predicateType":"customFoo","subject":[{"name":"subject","digest":{"sha256":"deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"}}],"predicate":{}}`)
+
+	virtualSigstore, err := ca.NewVirtualSigstore()
+	assert.NoError(t, err)
+	entity, err := virtualSigstore.Attest("foo@fighters.com", "issuer", statement)
+	assert.NoError(t, err)
+
+	// failure: threshold of 1 is not met with no entries
+	_, err = verify.VerifyArtifactTransparencyLog(&noTLogEntity{entity}, virtualSigstore, 1, false)
+	assert.Error(t, err)
+	if !strings.Contains(err.Error(), "not enough verified timestamps from transparency log") {
+		t.Errorf("expected error with timestamp threshold, got: %v", err.Error())
+	}
+}
+
 type dupTlogEntity struct {
 	*ca.TestEntity
 }