[FAB-5361] Properly return FORBIDDEN on broadcast

For messages which fail signature verification, the orderer currently
returns BAD_REQUEST, as it is registered simply as an error in
processing the message.  However, this is very poor from a
serviceability perspective, and a more precise status should be returned
when possible.

This CR creates a new error type in the message processor which may be
returned for signature validation failures.

This CR also allows the broadcast error processing to use the errors
package to retrieve the cause of an error, so that additional
descriptive information beyond the basic error definition may be
included as well.

Change-Id: I7c02ca1b456bb4c052492b7979c0e75954f2b75a
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

orderer/common/broadcast/broadcast.go

@@ -12,7 +12,9 @@ import (
 	"github.com/hyperledger/fabric/orderer/common/msgprocessor"
 	cb "github.com/hyperledger/fabric/protos/common"
 	ab "github.com/hyperledger/fabric/protos/orderer"
+
 	"github.com/op/go-logging"
+	"github.com/pkg/errors"
 )
 
 var logger = logging.MustGetLogger("orderer/common/broadcast")
@@ -123,9 +125,11 @@ func (bh *handlerImpl) Handle(srv ab.AtomicBroadcast_BroadcastServer) error {
 
 // ClassifyError converts an error type into a status code.
 func ClassifyError(err error) cb.Status {
-	switch err {
+	switch errors.Cause(err) {
 	case msgprocessor.ErrChannelDoesNotExist:
 		return cb.Status_NOT_FOUND
+	case msgprocessor.ErrPermissionDenied:
+		return cb.Status_FORBIDDEN
 	default:
 		return cb.Status_BAD_REQUEST
 	}

orderer/common/broadcast/broadcast_test.go

@@ -1,17 +1,7 @@
 /*
-Copyright IBM Corp. 2016 All Rights Reserved.
+Copyright IBM Corp. 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.
+SPDX-License-Identifier: Apache-2.0
 */
 
 package broadcast
@@ -27,6 +17,7 @@ import (
 	ab "github.com/hyperledger/fabric/protos/orderer"
 
 	logging "github.com/op/go-logging"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 	"google.golang.org/grpc"
 )
@@ -171,6 +162,21 @@ func TestEnqueueFailure(t *testing.T) {
 	}
 }
 
+func TestClassifyError(t *testing.T) {
+	t.Run("NotFound", func(t *testing.T) {
+		assert.Equal(t, cb.Status_NOT_FOUND, ClassifyError(msgprocessor.ErrChannelDoesNotExist))
+	})
+	t.Run("Forbidden", func(t *testing.T) {
+		assert.Equal(t, cb.Status_FORBIDDEN, ClassifyError(msgprocessor.ErrPermissionDenied))
+	})
+	t.Run("WrappedErr", func(t *testing.T) {
+		assert.Equal(t, cb.Status_NOT_FOUND, ClassifyError(errors.Wrap(msgprocessor.ErrChannelDoesNotExist, "A wrapped error")))
+	})
+	t.Run("DefaultBadReq", func(t *testing.T) {
+		assert.Equal(t, cb.Status_BAD_REQUEST, ClassifyError(fmt.Errorf("Foo")))
+	})
+}
+
 func TestBadChannelId(t *testing.T) {
 	mm := getMockSupportManager()
 	mm.MsgProcessorVal = &mockSupport{ProcessErr: msgprocessor.ErrChannelDoesNotExist}

orderer/common/msgprocessor/msgprocessor.go

@@ -27,6 +27,10 @@ const (
 // are not for the system channel ID and are not attempting to create a new channel
 var ErrChannelDoesNotExist = errors.New("channel does not exist")
 
+// ErrPermissionDenied is returned by errors which are caused by transactions
+// which are not permitted due to an authorization failure.
+var ErrPermissionDenied = errors.New("permission denied")
+
 // Classification represents the possible message types for the system.
 type Classification int
 

orderer/common/msgprocessor/sigfilter.go

@@ -11,6 +11,8 @@ import (
 
 	"github.com/hyperledger/fabric/common/policies"
 	cb "github.com/hyperledger/fabric/protos/common"
+
+	"github.com/pkg/errors"
 )
 
 type sigFilter struct {
@@ -40,5 +42,9 @@ func (sf *sigFilter) Apply(message *cb.Envelope) error {
 		return fmt.Errorf("could not find policy %s", sf.policyName)
 	}
 
-	return policy.Evaluate(signedData)
+	err = policy.Evaluate(signedData)
+	if err != nil {
+		return errors.Wrap(errors.WithStack(ErrPermissionDenied), err.Error())
+	}
+	return nil
 }

orderer/common/msgprocessor/sigfilter_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/hyperledger/fabric/protos/utils"
 
 	"github.com/op/go-logging"
+	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -55,5 +56,5 @@ func TestErrorOnPolicy(t *testing.T) {
 	mpm := &mockpolicies.Manager{Policy: &mockpolicies.Policy{Err: fmt.Errorf("Error")}}
 	err := NewSigFilter("foo", mpm).Apply(makeEnvelope())
 	assert.NotNil(t, err)
-	assert.Equal(t, mpm.Policy.Err, err)
+	assert.Equal(t, ErrPermissionDenied, errors.Cause(err))
 }