From 06c253a0eac83f5e577e00e5947872a6a0928bac Mon Sep 17 00:00:00 2001
From: Andrew Ayer <andrew@sslmate.com>
Date: Mon, 10 Apr 2017 13:54:49 -0700
Subject: [PATCH] Continue processing a log even if an STH failed to verify

It may still be possible to audit other STHs, and to scan new entries
up to the latest verified STH.  This allows Cert Spotter to continue
to make forward progress even if a log is persistently skewed (as the
DigiCert has been lately).

Also, rework some code to be simpler and less redundant.
---
 cmd/common.go | 36 ++++++++++++++++--------------------
 scanner.go    | 27 +++++++++++++--------------
 2 files changed, 29 insertions(+), 34 deletions(-)

diff --git a/cmd/common.go b/cmd/common.go
index a96d3c4..8f86fba 100644
--- a/cmd/common.go
+++ b/cmd/common.go
@@ -175,6 +175,17 @@ func (ctlog *logHandle) refresh() error {
 	return nil
 }
 
+func (ctlog *logHandle) verifySTH(sth *ct.SignedTreeHead) error {
+	isValid, err := ctlog.scanner.CheckConsistency(ctlog.verifiedSTH, sth)
+	if err != nil {
+		return fmt.Errorf("Error fetching consistency proof: %s", err)
+	}
+	if !isValid {
+		return fmt.Errorf("Consistency proof between %d and %d is invalid", ctlog.verifiedSTH.TreeSize, sth.TreeSize)
+	}
+	return nil
+}
+
 func (ctlog *logHandle) audit() error {
 	sths, err := ctlog.state.GetUnverifiedSTHs()
 	if err != nil {
@@ -183,16 +194,13 @@ func (ctlog *logHandle) audit() error {
 
 	for _, sth := range sths {
 		if *verbose {
-			log.Printf("Verifying consistency between STH %d (%x) and STH %d (%x)", sth.TreeSize, sth.SHA256RootHash, ctlog.verifiedSTH.TreeSize, ctlog.verifiedSTH.SHA256RootHash)
+			log.Printf("Verifying consistency of STH %d (%x) with previously-verified STH %d (%x)", sth.TreeSize, sth.SHA256RootHash, ctlog.verifiedSTH.TreeSize, ctlog.verifiedSTH.SHA256RootHash)
+		}
+		if err := ctlog.verifySTH(sth); err != nil {
+			log.Printf("Unable to verify consistency of STH %d (%s) (if this error persists, it should be construed as misbehavior by the log): %s", sth.TreeSize, ctlog.state.UnverifiedSTHFilename(sth), err)
+			continue
 		}
 		if sth.TreeSize > ctlog.verifiedSTH.TreeSize {
-			isValid, err := ctlog.scanner.CheckConsistency(ctlog.verifiedSTH, sth)
-			if err != nil {
-				return fmt.Errorf("Error fetching consistency proof between %d and %d (if this error persists, it should be construed as misbehavior by the log): %s", ctlog.verifiedSTH.TreeSize, sth.TreeSize, err)
-			}
-			if !isValid {
-				return fmt.Errorf("Log has misbehaved: STH in '%s' is not consistent with STH in '%s'", ctlog.state.VerifiedSTHFilename(), ctlog.state.UnverifiedSTHFilename(sth))
-			}
 			if *verbose {
 				log.Printf("STH %d (%x) is now the latest verified STH", sth.TreeSize, sth.SHA256RootHash)
 			}
@@ -200,18 +208,6 @@ func (ctlog *logHandle) audit() error {
 			if err := ctlog.state.StoreVerifiedSTH(ctlog.verifiedSTH); err != nil {
 				return fmt.Errorf("Error storing verified STH: %s", err)
 			}
-		} else if sth.TreeSize < ctlog.verifiedSTH.TreeSize {
-			isValid, err := ctlog.scanner.CheckConsistency(sth, ctlog.verifiedSTH)
-			if err != nil {
-				return fmt.Errorf("Error fetching consistency proof between %d and %d (if this error persists, it should be construed as misbehavior by the log): %s", ctlog.verifiedSTH.TreeSize, sth.TreeSize, err)
-			}
-			if !isValid {
-				return fmt.Errorf("Log has misbehaved: STH in '%s' is not consistent with STH in '%s'", ctlog.state.VerifiedSTHFilename(), ctlog.state.UnverifiedSTHFilename(sth))
-			}
-		} else {
-			if !bytes.Equal(sth.SHA256RootHash[:], ctlog.verifiedSTH.SHA256RootHash[:]) {
-				return fmt.Errorf("Log has misbehaved: STH in '%s' is not consistent with STH in '%s'", ctlog.state.VerifiedSTHFilename(), ctlog.state.UnverifiedSTHFilename(sth))
-			}
 		}
 		if err := ctlog.state.RemoveUnverifiedSTH(sth); err != nil {
 			return fmt.Errorf("Error removing redundant STH: %s", err)
diff --git a/scanner.go b/scanner.go
index 019f8c5..9e55c88 100644
--- a/scanner.go
+++ b/scanner.go
@@ -214,24 +214,23 @@ func (s *Scanner) GetSTH() (*ct.SignedTreeHead, error) {
 }
 
 func (s *Scanner) CheckConsistency(first *ct.SignedTreeHead, second *ct.SignedTreeHead) (bool, error) {
-	var proof ct.ConsistencyProof
-
-	if first.TreeSize > second.TreeSize {
-		// No way this can be valid
-		return false, nil
-	} else if first.TreeSize == second.TreeSize {
-		// The proof *should* be empty, so don't bother contacting the server.
-		// This is necessary because the digicert server returns a 400 error if first==second.
-		proof = []ct.MerkleTreeNode{}
-	} else {
-		var err error
-		proof, err = s.logClient.GetConsistencyProof(int64(first.TreeSize), int64(second.TreeSize))
+	if first.TreeSize < second.TreeSize {
+		proof, err := s.logClient.GetConsistencyProof(int64(first.TreeSize), int64(second.TreeSize))
 		if err != nil {
 			return false, err
 		}
+		return VerifyConsistencyProof(proof, first, second), nil
+	} else if first.TreeSize > second.TreeSize {
+		proof, err := s.logClient.GetConsistencyProof(int64(second.TreeSize), int64(first.TreeSize))
+		if err != nil {
+			return false, err
+		}
+		return VerifyConsistencyProof(proof, second, first), nil
+	} else {
+		// There is no need to ask the server for a consistency proof if the trees
+		// are the same size, and the DigiCert log returns a 400 error if we try.
+		return bytes.Equal(first.SHA256RootHash[:], second.SHA256RootHash[:]), nil
 	}
-
-	return VerifyConsistencyProof(proof, first, second), nil
 }
 
 func (s *Scanner) MakeCollapsedMerkleTree(sth *ct.SignedTreeHead) (*CollapsedMerkleTree, error) {