From dc5fc647613bae6ce65ba3a8344c5b4d25f09db7 Mon Sep 17 00:00:00 2001
From: Marko Vukolic <mvu@zurich.ibm.com>
Date: Wed, 21 Dec 2016 10:20:21 +0100
Subject: [PATCH] fix sbft consensus violation after attack

A correct replica would blindly put together a checkpointed batch with
its locally preprepared one for the same sequence number. However, even
correct replicas need to check if the payloads are the same, as Byzantine
primary might had previously polluted a correct replica with a different
preprepare.

Added a test that was causing consensus violation in the previous code
(now passing).

Change-Id: I5417730cc95d05c619f445178fa93ecfe5043a70
Signed-off-by: Marko Vukolic <mvu@zurich.ibm.com>
---
 orderer/sbft/simplebft/newview.go        | 10 ++++-
 orderer/sbft/simplebft/simplebft_test.go | 52 ++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/orderer/sbft/simplebft/newview.go b/orderer/sbft/simplebft/newview.go
index 319a08cba08..98103392bec 100644
--- a/orderer/sbft/simplebft/newview.go
+++ b/orderer/sbft/simplebft/newview.go
@@ -146,11 +146,17 @@ func (s *SBFT) handleNewView(nv *NewView, src uint64) {
 	s.view = nv.View
 	s.discardBacklog(s.primaryID())
 
-	// maybe deliver using xset
+	// maybe deliver previous batch
 	if s.sys.LastBatch().DecodeHeader().Seq < prevBatch.DecodeHeader().Seq {
 		if prevBatch.DecodeHeader().Seq == s.cur.subject.Seq.Seq {
 			// we just received a signature set for a request which we preprepared, but never delivered.
-			prevBatch.Payloads = s.cur.preprep.Batch.Payloads
+			// check first if the locally preprepared request matches the signature set
+			if !reflect.DeepEqual(prevBatch.DecodeHeader().DataHash, s.cur.preprep.Batch.DecodeHeader().DataHash) {
+				log.Warningf("replica %d: [seq %d] request checkpointed in a previous view does not match locally preprepared one, delivering batch without payload", s.id, s.cur.subject.Seq.Seq)
+			} else {
+				log.Debugf("replica %d: [seq %d] request checkpointed in a previous view with matching preprepare, completing and delivering the batch with payload", s.id, s.cur.subject.Seq.Seq)
+				prevBatch.Payloads = s.cur.preprep.Batch.Payloads
+			}
 		}
 		s.deliverBatch(prevBatch)
 	}
diff --git a/orderer/sbft/simplebft/simplebft_test.go b/orderer/sbft/simplebft/simplebft_test.go
index d6f1567cbe2..ea410dd9a0d 100644
--- a/orderer/sbft/simplebft/simplebft_test.go
+++ b/orderer/sbft/simplebft/simplebft_test.go
@@ -266,6 +266,58 @@ func TestByzPrimary(t *testing.T) {
 	}
 }
 
+func TestNewPrimaryHandlingViewChange(t *testing.T) {
+	skipInShortMode(t)
+	N := uint64(7)
+	sys := newTestSystem(N)
+	var repls []*SBFT
+	var adapters []*testSystemAdapter
+	for i := uint64(0); i < N; i++ {
+		a := sys.NewAdapter(i)
+		s, err := New(i, &Config{N: N, F: 2, BatchDurationNsec: 2000000000, BatchSizeBytes: 1, RequestTimeoutNsec: 20000000000}, a)
+		if err != nil {
+			t.Fatal(err)
+		}
+		repls = append(repls, s)
+		adapters = append(adapters, a)
+	}
+
+	r1 := []byte{1, 2, 3}
+	r2 := []byte{5, 6, 7}
+
+	// change preprepare to 2-6
+	sys.filterFn = func(e testElem) (testElem, bool) {
+		if msg, ok := e.ev.(*testMsgEvent); ok {
+			if pp := msg.msg.GetPreprepare(); pp != nil && msg.src == 0 && msg.dst >= 2 {
+				pp := *pp
+				batch := *pp.Batch
+				batch.Payloads = [][]byte{r2}
+				pp.Batch = &batch
+				h := merkleHashData(batch.Payloads)
+				bh := &BatchHeader{}
+				proto.Unmarshal(pp.Batch.Header, bh)
+				bh.DataHash = h
+				bhraw, _ := proto.Marshal(bh)
+				pp.Batch.Header = bhraw
+				msg.msg = &Msg{&Msg_Preprepare{&pp}}
+			}
+		}
+		return e, true
+	}
+
+	connectAll(sys)
+	repls[0].Request(r1)
+	sys.Run()
+	for _, a := range adapters {
+		if len(a.batches) < 1 {
+			t.Fatal("expected execution of at least one batch")
+		}
+		if a.batches[0].Payloads != nil && !reflect.DeepEqual(adapters[2].batches[0].Payloads, a.batches[0].Payloads) {
+			t.Error("consensus violated on first batch at replica", a.id)
+		}
+	}
+}
+
 func TestByzPrimaryBullyingSingleReplica(t *testing.T) {
 	skipInShortMode(t)
 	N := uint64(10)