[FAB-4479] Fix theoretical orderer crashes

Although the orderer makes every effort to validate all user provided
data, especially when processing configuration updates, there is a very
large attack surface where a specially crafted message might be able to
trigger a programming error like a nil pointer derference, crashing the
orderer.

Because the client threads for Broadcast and Deliver never have any
direct affect on world state (ie writing to the ledger, or updating
configuration), they may safely panic at any point without worry of
corrupting the orderer state.

Since these threads may panic without adversely affecting the orderer,
these panics should be recovered and logged, instead of being allowed to
crash the whole ordering process.

This CR adds a recover to the top level Broadcast/Deliver functions
which logs the stack trace at a Critical level, before closing the
connection.

Change-Id: I0ece5b4749648dd9a478ba814c081049bde55c6d
Signed-off-by: Jason Yellick <[email protected]>

Author: Jason Yellick

orderer/server.go

@@ -23,6 +23,8 @@ import (
 	"github.com/hyperledger/fabric/orderer/configupdate"
 	"github.com/hyperledger/fabric/orderer/multichain"
 	ab "github.com/hyperledger/fabric/protos/orderer"
+
+	"runtime/debug"
 )
 
 type configUpdateSupport struct {
@@ -70,11 +72,23 @@ func NewServer(ml multichain.Manager, signer crypto.LocalSigner) ab.AtomicBroadc
 // Broadcast receives a stream of messages from a client for ordering
 func (s *server) Broadcast(srv ab.AtomicBroadcast_BroadcastServer) error {
 	logger.Debugf("Starting new Broadcast handler")
+	defer func() {
+		if r := recover(); r != nil {
+			logger.Criticalf("Broadcast client triggered panic: %s\n%s", r, debug.Stack())
+		}
+		logger.Debugf("Closing Broadcast stream")
+	}()
 	return s.bh.Handle(srv)
 }
 
 // Deliver sends a stream of blocks to a client after ordering
 func (s *server) Deliver(srv ab.AtomicBroadcast_DeliverServer) error {
 	logger.Debugf("Starting new Deliver handler")
+	defer func() {
+		if r := recover(); r != nil {
+			logger.Criticalf("Deliver client triggered panic: %s\n%s", r, debug.Stack())
+		}
+		logger.Debugf("Closing Deliver stream")
+	}()
 	return s.dh.Handle(srv)
 }

orderer/server_test.go

@@ -0,0 +1,31 @@
+/*
+Copyright IBM Corp. 2017 All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+                 http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"testing"
+)
+
+func TestBroadcastNoPanic(t *testing.T) {
+	// Defer recovers from the panic
+	_ = (&server{}).Broadcast(nil)
+}
+
+func TestDeliverNoPanic(t *testing.T) {
+	// Defer recovers from the panic
+	_ = (&server{}).Deliver(nil)
+}