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

@@ -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)
 	}

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)