PM-3562: Validate a checkpointing certificate. (#67)

* PM-3562: Validate a checkpointing certificate.

* PM-3562: Test that a random certificate is rejected.

* PM-3562: Test that a valid certificate is accepted.

* PM-3562: Test that invalid certificates are rejected.

* PM-3562: Validate the leaf index in the Merkle tree proof.

* PM-3562: Add general filter to make sure the certificate was changed.

* PM-3562: Explain buildChain.

* PM-3562: Another sanity check in construct.

* PM-3562: Add comment about max leaf index check.

* PM-3562: Reverse the order of the error message and the condition.

metronome/checkpointing/models/props/src/io/iohk/metronome/checkpointing/models/CheckpointCertificateProps.scala

@@ -0,0 +1,212 @@
+package io.iohk.metronome.checkpointing.models
+
+import cats.data.NonEmptyList
+import io.iohk.metronome.checkpointing.CheckpointingAgreement
+import io.iohk.metronome.crypto.ECKeyPair
+import io.iohk.ethereum.rlp
+import io.iohk.metronome.hotstuff.consensus.{
+  LeaderSelection,
+  Federation,
+  ViewNumber
+}
+import io.iohk.metronome.hotstuff.consensus.basic.{
+  Secp256k1Signing,
+  Phase,
+  QuorumCertificate
+}
+import java.security.SecureRandom
+import org.scalacheck._
+import org.scalacheck.Prop.{forAll, forAllNoShrink, propBoolean}
+import scodec.bits.ByteVector
+import scala.util.Random
+
+object CheckpointCertificateProps extends Properties("CheckpointCertificate") {
+  import ArbitraryInstances._
+  import Arbitrary.arbitrary
+  import RLPCodecs._
+
+  // Testing with real signatures is rather slow.
+  override def overrideParameters(p: Test.Parameters): Test.Parameters =
+    p.withMinSuccessfulTests(25)
+
+  implicit val genFederation: Gen[
+    (
+        Federation[CheckpointingAgreement.PKey],
+        Vector[CheckpointingAgreement.SKey]
+    )
+  ] =
+    for {
+      seed <- arbitrary[Array[Byte]]
+      rnd = new SecureRandom(seed)
+      fedSize <- Gen.choose(3, 10)
+      keys = Vector.fill(fedSize)(ECKeyPair.generate(rnd))
+      fed  = Federation(keys.map(_.pub))(LeaderSelection.RoundRobin)
+    } yield fed.fold(sys.error, identity) -> keys.map(_.prv)
+
+  implicit val arbFederation =
+    Arbitrary(genFederation.map(_._1))
+
+  implicit val signing =
+    new Secp256k1Signing[CheckpointingAgreement](
+      (phase, viewNumber, blockHash) =>
+        List(
+          rlp.encode(phase),
+          rlp.encode(viewNumber),
+          rlp.encode(blockHash)
+        ).map(ByteVector(_)).reduce(_ ++ _)
+    )
+
+  /** Extends a chain of blocks `n` more times.
+    *
+    * The `ancestors` are ordered so that the `head` is the most recent block,
+    * the one which should be used as the parent for the extension.
+    *
+    * The return value is reversed so the oldest block is the `head` and the tip is the `last` item.
+    * This can be used to put into the `CheckpointCertificate` in the same order.
+    */
+  def buildChain(
+      n: Int,
+      parentLedger: Ledger,
+      ancestors: NonEmptyList[Block.Header]
+  ): Gen[NonEmptyList[Block.Header]] =
+    if (n == 0) {
+      Gen.const(ancestors.reverse)
+    } else {
+      arbitrary[Vector[Transaction]].flatMap { txns =>
+        val ledger = parentLedger.update(txns)
+        val block  = Block.make(ancestors.head, ledger.hash, Block.Body(txns))
+        buildChain(n - 1, ledger, block.header :: ancestors)
+      }
+    }
+
+  val validCertificate = for {
+    candidate <- arbitrary[Transaction.CheckpointCandidate]
+    txnsPre   <- arbitrary[Vector[Transaction]]
+    txnsPost  <- arbitrary[Vector[Transaction.ProposerBlock]]
+    txns = txnsPre ++ Vector(candidate) ++ txnsPost
+
+    parent <- arbitrary[Block.Header]
+    ledger <- arbitrary[Ledger].map(_ update txns)
+    block = Block.make(parent, ledger.hash, Block.Body(txns))
+
+    chainLength <- Gen.choose(0, 5)
+    chain       <- buildChain(chainLength, ledger, NonEmptyList.one(block.header))
+
+    (federation, privateKeys) <- genFederation
+    signers                   <- Gen.pick(federation.quorumSize, privateKeys)
+    viewNumber                <- arbitrary[ViewNumber]
+    signatures = signers.toList.map(
+      signing.sign(_, Phase.Commit, viewNumber, chain.last.hash)
+    )
+    signature = signing.combine(signatures)
+
+    commitQC = QuorumCertificate[CheckpointingAgreement, Phase.Commit](
+      Phase.Commit,
+      viewNumber,
+      chain.last.hash,
+      signature = signature
+    )
+
+    certificate = CheckpointCertificate.construct(block, chain, commitQC).get
+
+  } yield (federation, certificate)
+
+  def invalidateCertificate(
+      c: CheckpointCertificate
+  ): Gen[(String, CheckpointCertificate)] =
+    Gen
+      .oneOf(
+        arbitrary[Block.Hash].map { h =>
+          "Invalid block hash" -> c.copy(commitQC =
+            c.commitQC.copy[CheckpointingAgreement, Phase.Commit](blockHash = h)
+          )
+        },
+        arbitrary[ViewNumber].map { vn =>
+          "Invalid view number" -> c.copy(commitQC =
+            c.commitQC
+              .copy[CheckpointingAgreement, Phase.Commit](viewNumber = vn)
+          )
+        },
+        Gen.oneOf(c.commitQC.signature.sig).map { s =>
+          "Removed signature" -> c.copy(commitQC =
+            c.commitQC.copy[CheckpointingAgreement, Phase.Commit](signature =
+              c.commitQC.signature.copy(
+                sig = c.commitQC.signature.sig.filterNot(_ == s)
+              )
+            )
+          )
+        },
+        arbitrary[CheckpointingAgreement.PSig].map { s =>
+          "Replaced signature" -> c.copy(commitQC =
+            c.commitQC.copy[CheckpointingAgreement, Phase.Commit](signature =
+              c.commitQC.signature.copy(
+                sig = s +: c.commitQC.signature.sig.tail
+              )
+            )
+          )
+        },
+        arbitrary[Transaction.CheckpointCandidate].map { cc =>
+          "Invalid candidate" -> c.copy(checkpoint = cc)
+        },
+        arbitrary[CheckpointCertificate].map { cc =>
+          "Invalid proof" -> c.copy(proof = cc.proof)
+        },
+        Gen.nonEmptyListOf(arbitrary[Block.Header]).map { bs =>
+          "Prefixed headers" ->
+            c.copy(headers =
+              NonEmptyList.fromListUnsafe(bs ++ c.headers.toList)
+            )
+        },
+        Gen.nonEmptyListOf(arbitrary[Block.Header]).map { bs =>
+          "Postfixed headers" ->
+            c.copy(headers =
+              NonEmptyList.fromListUnsafe(c.headers.toList ++ bs)
+            )
+        },
+        for {
+          hs0 <- Gen.const(c.headers.toList)
+          if hs0.size > 1
+          seed <- arbitrary[Int]
+          hs1 = new Random(seed).shuffle(hs0)
+          if hs1 != hs0
+        } yield "Shuffled headers" -> c
+          .copy(headers = NonEmptyList.fromListUnsafe(hs1))
+      )
+      .suchThat({ case (_, c1) => c1 != c })
+
+  property("validate - reject random") = forAll {
+    (
+        federation: Federation[CheckpointingAgreement.PKey],
+        certificate: CheckpointCertificate
+    ) =>
+      CheckpointCertificate.validate(certificate, federation).isLeft
+  }
+
+  property("validate - accept valid") = forAll(validCertificate) {
+    case (federation, certificate) =>
+      CheckpointCertificate
+        .validate(certificate, federation)
+        .fold(
+          _ |: false,
+          _ => "ok" |: true
+        )
+  }
+
+  property("validate - reject invalid") = forAllNoShrink(
+    for {
+      (federation, valid) <- validCertificate
+      (hint, invalid)     <- invalidateCertificate(valid)
+    } yield (federation, hint, invalid)
+  ) { case (federation, hint, certificate) =>
+    hint |: CheckpointCertificate.validate(certificate, federation).isLeft
+  }
+
+  property("validate - reject foreign") = forAll(
+    for {
+      (_, valid) <- validCertificate
+      federation <- arbitrary[Federation[CheckpointingAgreement.PKey]]
+    } yield (federation, valid)
+  ) { case (federation, certificate) =>
+    CheckpointCertificate.validate(certificate, federation).isLeft
+  }
+}

metronome/checkpointing/models/src/io/iohk/metronome/checkpointing/models/CheckpointCertificate.scala

@@ -4,6 +4,9 @@ import cats.data.NonEmptyList
 import io.iohk.metronome.hotstuff.consensus.basic.{QuorumCertificate, Phase}
 import io.iohk.metronome.checkpointing.CheckpointingAgreement
 import io.iohk.metronome.checkpointing.models.Transaction.CheckpointCandidate
+import io.iohk.metronome.hotstuff.consensus.basic.Signing
+import io.iohk.metronome.hotstuff.consensus.Federation
+import io.iohk.metronome.core.Validated
 
 /** The Checkpoint Certificate is a proof of the BFT agreement
   * over a given Checkpoint Candidate.
@@ -31,14 +34,57 @@ case class CheckpointCertificate(
 )
 
 object CheckpointCertificate {
+
+  /** Create a `CheckpointCertificate` from a `Block` that last had a `CheckpointCandidate`
+    * and a list of `Block.Header`s leading up to the `QuorumCertifictate` that proves the
+    * BFT agreement over the contents.
+    */
   def construct(
       block: Block,
       headers: NonEmptyList[Block.Header],
       commitQC: QuorumCertificate[CheckpointingAgreement, Phase.Commit]
-  ): Option[CheckpointCertificate] =
+  ): Option[CheckpointCertificate] = {
+    assert(block.hash == headers.head.hash)
+    assert(commitQC.blockHash == headers.last.hash)
+
     constructProof(block).map { case (proof, cp) =>
       CheckpointCertificate(headers, cp, proof, commitQC)
     }
+  }
+
+  /** Validate a `CheckpointCertificate` by checking that:
+    * - the chain of block headers is valid
+    * - the quorum certificate is valid
+    * - the Merkle proof of the candidate is valid
+    */
+  def validate(
+      certificate: CheckpointCertificate,
+      federation: Federation[CheckpointingAgreement.PKey]
+  )(implicit
+      signing: Signing[CheckpointingAgreement]
+  ): Either[String, Validated[Transaction.CheckpointCandidate]] = {
+    val hs = certificate.headers
+    for {
+      _ <- hs.toList.zip(hs.tail).forall { case (parent, child) =>
+        parent.hash == child.parentHash
+      } orError
+        "The headers do not correspond to a chain of parent-child blocks."
+
+      _ <- (certificate.commitQC.blockHash == hs.last.hash) orError
+        "The Commit Q.C. is not about the last block in the chain."
+
+      _ <- signing.validate(federation, certificate.commitQC) orError
+        "The Commit Q.C. is invalid."
+
+      _ <- MerkleTree.verifyProof(
+        certificate.proof,
+        root = hs.head.contentMerkleRoot,
+        leaf = MerkleTree.Hash(certificate.checkpoint.hash)
+      ) orError
+        "The Merkle proof is invalid."
+
+    } yield Validated[Transaction.CheckpointCandidate](certificate.checkpoint)
+  }
 
   private def constructProof(
       block: Block
@@ -51,4 +97,9 @@ object CheckpointCertificate {
         val cpHash = MerkleTree.Hash(cp.hash)
         MerkleTree.generateProofFromHash(tree, cpHash).map(_ -> cp)
     }.flatten
+
+  private implicit class BoolOps(val test: Boolean) extends AnyVal {
+    def orError(error: String): Either[String, Unit] =
+      Either.cond(test, (), error)
+  }
 }

metronome/checkpointing/models/src/io/iohk/metronome/checkpointing/models/MerkleTree.scala

@@ -117,6 +117,12 @@ object MerkleTree {
       }
     }
 
+    // This is required so for example on a single element tree a `Proof(4, Nil)` is not accepted, only `Proof(0, Nil)`,
+    // otherwise it would make the `CheckpointCertificate` malleable by adding arbitrarily large leaf indexes,
+    // as long as the LSB side that the verification uses is correct.
+    val maxLeafIndex = (1 << proof.siblingPath.length) - 1
+
+    proof.leafIndex <= maxLeafIndex &&
     verify(leaf, 1, proof.siblingPath) == root
   }