FABGW-19: Secure CommitStatus service (#2570)

- Receive a SignedCommitStatusRequest message.
- Verify signature and check included identity has CHANNELREADERS permission.
- Add integration tests for success and failure scenarios.

Signed-off-by: Mark S. Lewis <[email protected]>

core/aclmgmt/defaultaclprovider.go

@@ -108,6 +108,9 @@ func newDefaultACLProvider(policyChecker policy.PolicyChecker) defaultACLProvide
 	d.cResourcePolicyMap[resources.Event_Block] = CHANNELREADERS
 	d.cResourcePolicyMap[resources.Event_FilteredBlock] = CHANNELREADERS
 
+	// Gateway resources
+	d.cResourcePolicyMap[resources.Gateway_CommitStatus] = CHANNELREADERS
+
 	return d
 }
 
@@ -140,6 +143,8 @@ func (d *defaultACLProviderImpl) CheckACL(resName string, channelID string, idin
 			return err
 		}
 		return d.policyChecker.CheckPolicyBySignedData(channelID, policy, sd)
+	case *protoutil.SignedData:
+		return d.policyChecker.CheckPolicyBySignedData(channelID, policy, []*protoutil.SignedData{typedData})
 	case []*protoutil.SignedData:
 		return d.policyChecker.CheckPolicyBySignedData(channelID, policy, typedData)
 	default:

core/aclmgmt/resourceprovider.go

@@ -126,6 +126,9 @@ func (rp *aclmgmtPolicyProviderImpl) CheckACL(polName string, idinfo interface{}
 			return err
 		}
 
+	case *protoutil.SignedData:
+		sd = []*protoutil.SignedData{idinfo}
+
 	default:
 		return InvalidIdInfo(polName)
 	}

core/aclmgmt/resourceprovider_test.go

@@ -57,17 +57,32 @@ type signerSerializer interface {
 //go:generate counterfeiter -o mocks/defaultaclprovider.go --fake-name DefaultACLProvider . defaultACLProvider
 
 func TestPolicyBase(t *testing.T) {
-	peval := &mockPolicyEvaluatorImpl{pmap: map[string]string{"res": "pol"}, peval: map[string]error{"pol": nil}}
-	pprov := newPolicyProvider(peval)
-	sProp, _ := protoutil.MockSignedEndorserProposalOrPanic("A", &peer.ChaincodeSpec{}, []byte("Alice"), []byte("msg1"))
-	err := pprov.CheckACL("pol", sProp)
-	require.NoError(t, err)
-
-	signer := &mocks.SignerSerializer{}
-	env, err := protoutil.CreateSignedEnvelope(common.HeaderType_CONFIG, "myc", signer, &common.ConfigEnvelope{}, 0, 0)
-	require.NoError(t, err)
-	err = pprov.CheckACL("pol", env)
-	require.NoError(t, err)
+	evaluator := &mockPolicyEvaluatorImpl{pmap: map[string]string{"res": "pol"}, peval: map[string]error{"pol": nil}}
+	provider := newPolicyProvider(evaluator)
+
+	t.Run("SignedProposal", func(t *testing.T) {
+		proposal, _ := protoutil.MockSignedEndorserProposalOrPanic("A", &peer.ChaincodeSpec{}, []byte("Alice"), []byte("msg1"))
+		err := provider.CheckACL("pol", proposal)
+		require.NoError(t, err)
+	})
+
+	t.Run("Envelope", func(t *testing.T) {
+		signer := &mocks.SignerSerializer{}
+		envelope, err := protoutil.CreateSignedEnvelope(common.HeaderType_CONFIG, "myc", signer, &common.ConfigEnvelope{}, 0, 0)
+		require.NoError(t, err)
+		err = provider.CheckACL("pol", envelope)
+		require.NoError(t, err)
+	})
+
+	t.Run("SignedData", func(t *testing.T) {
+		data := &protoutil.SignedData{
+			Data:      []byte("DATA"),
+			Identity:  []byte("IDENTITY"),
+			Signature: []byte("SIGNATURE"),
+		}
+		err := provider.CheckACL("pol", data)
+		require.NoError(t, err)
+	})
 }
 
 func TestPolicyBad(t *testing.T) {

core/aclmgmt/resources/resources.go

@@ -61,4 +61,7 @@ const (
 	// Events
 	Event_Block         = "event/Block"
 	Event_FilteredBlock = "event/FilteredBlock"
+
+	// Gateway resources
+	Gateway_CommitStatus = "gateway/CommitStatus"
 )

go.mod

@@ -30,7 +30,7 @@ require (
 	github.com/hyperledger/fabric-chaincode-go v0.0.0-20201119163726-f8ef75b17719
 	github.com/hyperledger/fabric-config v0.1.0
 	github.com/hyperledger/fabric-lib-go v1.0.0
-	github.com/hyperledger/fabric-protos-go v0.0.0-20210311171918-e08edaab0493
+	github.com/hyperledger/fabric-protos-go v0.0.0-20210422135545-37e930696e2a
 	github.com/kr/pretty v0.2.1
 	github.com/magiconair/properties v1.8.1 // indirect
 	github.com/mattn/go-runewidth v0.0.4 // indirect

go.sum

@@ -161,8 +161,8 @@ github.com/hyperledger/fabric-lib-go v1.0.0 h1:UL1w7c9LvHZUSkIvHTDGklxFv2kTeva1Q
 github.com/hyperledger/fabric-lib-go v1.0.0/go.mod h1:H362nMlunurmHwkYqR5uHL2UDWbQdbfz74n8kbCFsqc=
 github.com/hyperledger/fabric-protos-go v0.0.0-20190919234611-2a87503ac7c9/go.mod h1:xVYTjK4DtZRBxZ2D9aE4y6AbLaPwue2o/criQyQbVD0=
 github.com/hyperledger/fabric-protos-go v0.0.0-20200424173316-dd554ba3746e/go.mod h1:xVYTjK4DtZRBxZ2D9aE4y6AbLaPwue2o/criQyQbVD0=
-github.com/hyperledger/fabric-protos-go v0.0.0-20210311171918-e08edaab0493 h1:jj4S9FWy0o0Re0XhMAPaL3d/ho3pqWcNH8duMcRxVNw=
-github.com/hyperledger/fabric-protos-go v0.0.0-20210311171918-e08edaab0493/go.mod h1:xVYTjK4DtZRBxZ2D9aE4y6AbLaPwue2o/criQyQbVD0=
+github.com/hyperledger/fabric-protos-go v0.0.0-20210422135545-37e930696e2a h1:sJwXhCxTQ7euBj+4w7pomn0CzOgdHkLlUOqtDgsI8IA=
+github.com/hyperledger/fabric-protos-go v0.0.0-20210422135545-37e930696e2a/go.mod h1:xVYTjK4DtZRBxZ2D9aE4y6AbLaPwue2o/criQyQbVD0=
 github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=
 github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
 github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=

integration/gateway/gateway_test.go

@@ -21,6 +21,9 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/tedsuo/ifrit"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 var _ = Describe("GatewayService", func() {
@@ -170,15 +173,22 @@ var _ = Describe("GatewayService", func() {
 	})
 
 	Describe("CommitStatus", func() {
-		It("should respond with status of submitted transaction", func() {
-			conn := network.PeerClientConn(org1Peer0)
-			defer conn.Close()
-			gatewayClient := gateway.NewGatewayClient(conn)
-			ctx, cancel := context.WithTimeout(context.Background(), network.EventuallyTimeout)
-			defer cancel()
-
-			signingIdentity := network.PeerUserSigner(org1Peer0, "User1")
-			proposedTransaction, transactionID := NewProposedTransaction(signingIdentity, "testchannel", "gatewaycc", "respond", []byte("200"), []byte("conga message"), []byte("conga payload"))
+		var conn *grpc.ClientConn
+		var gatewayClient gateway.GatewayClient
+		var ctx context.Context
+		var cancel context.CancelFunc
+		var signingIdentity *nwo.SigningIdentity
+		var transactionID string
+		var identity []byte
+
+		BeforeEach(func() {
+			conn = network.PeerClientConn(org1Peer0)
+			gatewayClient = gateway.NewGatewayClient(conn)
+			ctx, cancel = context.WithTimeout(context.Background(), network.EventuallyTimeout)
+
+			signingIdentity = network.PeerUserSigner(org1Peer0, "User1")
+			var proposedTransaction *peer.SignedProposal
+			proposedTransaction, transactionID = NewProposedTransaction(signingIdentity, "testchannel", "gatewaycc", "respond", []byte("200"), []byte("conga message"), []byte("conga payload"))
 
 			endorseRequest := &gateway.EndorseRequest{
 				TransactionId:       transactionID,
@@ -201,17 +211,93 @@ var _ = Describe("GatewayService", func() {
 			_, err = gatewayClient.Submit(ctx, submitRequest)
 			Expect(err).NotTo(HaveOccurred())
 
+			identity, err = signingIdentity.Serialize()
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			conn.Close()
+			cancel()
+		})
+
+		It("should respond with status of submitted transaction", func() {
 			statusRequest := &gateway.CommitStatusRequest{
 				ChannelId:     "testchannel",
+				Identity:      identity,
 				TransactionId: transactionID,
 			}
-			actualStatus, err := gatewayClient.CommitStatus(ctx, statusRequest)
+
+			statusRequestBytes, err := proto.Marshal(statusRequest)
+			Expect(err).NotTo(HaveOccurred())
+
+			signature, err := signingIdentity.Sign(statusRequestBytes)
+			Expect(err).NotTo(HaveOccurred())
+
+			signedStatusRequest := &gateway.SignedCommitStatusRequest{
+				Request:   statusRequestBytes,
+				Signature: signature,
+			}
+
+			actualStatus, err := gatewayClient.CommitStatus(ctx, signedStatusRequest)
 			Expect(err).NotTo(HaveOccurred())
 
 			expectedStatus := &gateway.CommitStatusResponse{
 				Result: peer.TxValidationCode_VALID,
 			}
 			Expect(proto.Equal(actualStatus, expectedStatus)).To(BeTrue(), "Expected\n\t%#v\nto proto.Equal\n\t%#v", actualStatus, expectedStatus)
 		})
+
+		It("should fail on unauthorized identity", func() {
+			badSigningIdentity := network.OrdererUserSigner(network.Orderer("orderer"), "Admin")
+			badIdentity, err := badSigningIdentity.Serialize()
+			Expect(err).NotTo(HaveOccurred())
+
+			statusRequest := &gateway.CommitStatusRequest{
+				ChannelId:     "testchannel",
+				Identity:      badIdentity,
+				TransactionId: transactionID,
+			}
+			statusRequestBytes, err := proto.Marshal(statusRequest)
+			Expect(err).NotTo(HaveOccurred())
+
+			signature, err := badSigningIdentity.Sign(statusRequestBytes)
+			Expect(err).NotTo(HaveOccurred())
+
+			signedStatusRequest := &gateway.SignedCommitStatusRequest{
+				Request:   statusRequestBytes,
+				Signature: signature,
+			}
+
+			_, err = gatewayClient.CommitStatus(ctx, signedStatusRequest)
+			Expect(err).To(HaveOccurred())
+
+			grpcErr, _ := status.FromError(err)
+			Expect(grpcErr.Code()).To(Equal(codes.PermissionDenied))
+		})
+
+		It("should fail on bad signature", func() {
+			statusRequest := &gateway.CommitStatusRequest{
+				ChannelId:     "testchannel",
+				Identity:      identity,
+				TransactionId: transactionID,
+			}
+
+			statusRequestBytes, err := proto.Marshal(statusRequest)
+			Expect(err).NotTo(HaveOccurred())
+
+			signature, err := signingIdentity.Sign([]byte("WRONG"))
+			Expect(err).NotTo(HaveOccurred())
+
+			signedStatusRequest := &gateway.SignedCommitStatusRequest{
+				Request:   statusRequestBytes,
+				Signature: signature,
+			}
+
+			_, err = gatewayClient.CommitStatus(ctx, signedStatusRequest)
+			Expect(err).To(HaveOccurred())
+
+			grpcErr, _ := status.FromError(err)
+			Expect(grpcErr.Code()).To(Equal(codes.PermissionDenied))
+		})
 	})
 })

internal/peer/node/start.go

@@ -834,6 +834,7 @@ func serve(args []string) error {
 						Query:    peerAdapter,
 						Notifier: commit.NewNotifier(peerAdapter),
 					},
+					aclProvider,
 					peerInstance.GossipService.SelfMembershipInfo().Endpoint,
 					coreConfig.LocalMSPID,
 					coreConfig.GatewayOptions,

internal/pkg/gateway/api.go

@@ -15,6 +15,7 @@ import (
 	"github.com/hyperledger/fabric-protos-go/common"
 	gp "github.com/hyperledger/fabric-protos-go/gateway"
 	"github.com/hyperledger/fabric-protos-go/peer"
+	"github.com/hyperledger/fabric/core/aclmgmt/resources"
 	"github.com/hyperledger/fabric/protoutil"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -226,11 +227,25 @@ func (gs *Server) Submit(ctx context.Context, request *gp.SubmitRequest) (*gp.Su
 //
 // If the transaction commit status cannot be returned, for example if the specified channel does not exist, a
 // FailedPrecondition error will be returned.
-func (gs *Server) CommitStatus(ctx context.Context, request *gp.CommitStatusRequest) (*gp.CommitStatusResponse, error) {
-	if request == nil {
+func (gs *Server) CommitStatus(ctx context.Context, signedRequest *gp.SignedCommitStatusRequest) (*gp.CommitStatusResponse, error) {
+	if signedRequest == nil {
 		return nil, status.Error(codes.InvalidArgument, "a commit status request is required")
 	}
 
+	request := &gp.CommitStatusRequest{}
+	if err := proto.Unmarshal(signedRequest.Request, request); err != nil {
+		return nil, status.Error(codes.InvalidArgument, "invalid status request")
+	}
+
+	signedData := &protoutil.SignedData{
+		Data:      signedRequest.Request,
+		Identity:  request.Identity,
+		Signature: signedRequest.Signature,
+	}
+	if err := gs.policy.CheckACL(resources.Gateway_CommitStatus, request.ChannelId, signedData); err != nil {
+		return nil, status.Error(codes.PermissionDenied, err.Error())
+	}
+
 	txStatus, err := gs.commitFinder.TransactionStatus(ctx, request.ChannelId, request.TransactionId)
 	if err != nil {
 		return nil, status.Error(codes.FailedPrecondition, err.Error())