[FAB-4252] Check duplicated identities in VSCC

Malicious users might try to duplicate proposal response endorsed
by the same identity to pass endorsement policies. Each peer need
to check such duplicated identities before policy evaluation phase.
This patchset is to fix such potential issue in VSCC

Change-Id: I05b29fafa818d7090373f0dd79b79be04974678d
Signed-off-by: Xi Xue Jia <[email protected]>

Author: xixuejia

core/scc/vscc/validator_onevalidsignature.go

@@ -33,12 +33,17 @@ import (
 	mspmgmt "github.com/hyperledger/fabric/msp/mgmt"
 	"github.com/hyperledger/fabric/protos/common"
 	"github.com/hyperledger/fabric/protos/ledger/rwset/kvrwset"
+	"github.com/hyperledger/fabric/protos/msp"
 	pb "github.com/hyperledger/fabric/protos/peer"
 	"github.com/hyperledger/fabric/protos/utils"
 )
 
 var logger = flogging.MustGetLogger("vscc")
 
+const (
+	DUPLICATED_IDENTITY_ERROR = "Endorsement policy evaluation failure might be caused by duplicated identities"
+)
+
 // ValidatorOneValidSignature implements the default transaction validation policy,
 // which is to check the correctness of the read-write set and the endorsement
 // signatures
@@ -62,7 +67,7 @@ func (vscc *ValidatorOneValidSignature) Init(stub shim.ChaincodeStubInterface) p
 // chaincodes to provide more sophisticated policy processing such as enabling
 // policy specification to be coded as a transaction of the chaincode and the client
 // selecting which policy to use for validation using parameter function
-// @return serialized Block of valid and invalid transactions indentified
+// @return serialized Block of valid and invalid transactions identified
 // Note that Peer calls this function with 3 arguments, where args[0] is the
 // function name, args[1] is the Envelope and args[2] is the validation policy
 func (vscc *ValidatorOneValidSignature) Invoke(stub shim.ChaincodeStubInterface) pb.Response {
@@ -134,27 +139,19 @@ func (vscc *ValidatorOneValidSignature) Invoke(stub shim.ChaincodeStubInterface)
 			return shim.Error(err.Error())
 		}
 
-		// this is the first part of the signed message
-		prespBytes := cap.Action.ProposalResponsePayload
-
-		// build the signature set for the evaluation
-		signatureSet := make([]*common.SignedData, len(cap.Action.Endorsements))
-
-		// loop through each of the endorsements and build the signature set
-		for i, endorsement := range cap.Action.Endorsements {
-			signatureSet[i] = &common.SignedData{
-				// set the data that is signed; concatenation of proposal response bytes and endorser ID
-				Data: append(prespBytes, endorsement.Endorser...),
-				// set the identity that signs the message: it's the endorser
-				Identity: endorsement.Endorser,
-				// set the signature
-				Signature: endorsement.Signature,
-			}
+		signatureSet, err := vscc.deduplicateIdentity(cap)
+		if err != nil {
+			return shim.Error(err.Error())
 		}
 
 		// evaluate the signature set against the policy
 		err = policy.Evaluate(signatureSet)
 		if err != nil {
+			logger.Warningf("Endorsement policy failure for transaction txid=%s, err: %s", chdr.GetTxId(), err.Error())
+			if len(signatureSet) < len(cap.Action.Endorsements) {
+				// Warning: duplicated identities exist, endorsement failure might be cause by this reason
+				return shim.Error(DUPLICATED_IDENTITY_ERROR)
+			}
 			return shim.Error(fmt.Sprintf("VSCC error: policy evaluation failed, err %s", err))
 		}
 
@@ -425,3 +422,38 @@ func (vscc *ValidatorOneValidSignature) getInstantiatedCC(chid, ccid string) (cd
 	exists = true
 	return
 }
+
+func (vscc *ValidatorOneValidSignature) deduplicateIdentity(cap *pb.ChaincodeActionPayload) ([]*common.SignedData, error) {
+	// this is the first part of the signed message
+	prespBytes := cap.Action.ProposalResponsePayload
+
+	// build the signature set for the evaluation
+	signatureSet := []*common.SignedData{}
+	signatureMap := make(map[string]struct{})
+	// loop through each of the endorsements and build the signature set
+	for _, endorsement := range cap.Action.Endorsements {
+		//unmarshal endorser bytes
+		serializedIdentity := &msp.SerializedIdentity{}
+		if err := proto.Unmarshal(endorsement.Endorser, serializedIdentity); err != nil {
+			logger.Errorf("Unmarshal endorser error: %s", err)
+			return nil, fmt.Errorf("Unmarshal endorser error: %s", err)
+		}
+		identity := serializedIdentity.Mspid + string(serializedIdentity.IdBytes)
+		if _, ok := signatureMap[identity]; ok {
+			// Endorsement with the same identity has already been added
+			logger.Warningf("Ignoring duplicated identity, Mspid: %s, pem:\n%s", serializedIdentity.Mspid, serializedIdentity.IdBytes)
+			continue
+		}
+		signatureSet = append(signatureSet, &common.SignedData{
+			// set the data that is signed; concatenation of proposal response bytes and endorser ID
+			Data: append(prespBytes, endorsement.Endorser...),
+			// set the identity that signs the message: it's the endorser
+			Identity: endorsement.Endorser,
+			// set the signature
+			Signature: endorsement.Signature})
+		signatureMap[identity] = struct{}{}
+	}
+
+	logger.Debugf("Signature set is of size %d out of %d endorsement(s)", len(signatureSet), len(cap.Action.Endorsements))
+	return signatureSet, nil
+}

core/scc/vscc/validator_onevalidsignature_test.go

@@ -44,31 +44,36 @@ import (
 	mspmgmt "github.com/hyperledger/fabric/msp/mgmt"
 	"github.com/hyperledger/fabric/msp/mgmt/testtools"
 	"github.com/hyperledger/fabric/protos/common"
+	mspproto "github.com/hyperledger/fabric/protos/msp"
 	"github.com/hyperledger/fabric/protos/peer"
 	"github.com/hyperledger/fabric/protos/utils"
 	"github.com/stretchr/testify/assert"
 )
 
-func createTx() (*common.Envelope, *peer.ProposalResponse, error) {
+func createTx(endorsedByDuplicatedIdentity bool) (*common.Envelope, error) {
 	ccid := &peer.ChaincodeID{Name: "foo", Version: "v1"}
 	cis := &peer.ChaincodeInvocationSpec{ChaincodeSpec: &peer.ChaincodeSpec{ChaincodeId: ccid}}
 
 	prop, _, err := utils.CreateProposalFromCIS(common.HeaderType_ENDORSER_TRANSACTION, util.GetTestChainID(), cis, sid)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	presp, err := utils.CreateProposalResponse(prop.Header, prop.Payload, &peer.Response{Status: 200}, []byte("res"), nil, ccid, nil, id)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
-	env, err := utils.CreateSignedTx(prop, id, presp)
+	var env *common.Envelope
+	if endorsedByDuplicatedIdentity {
+		env, err = utils.CreateSignedTx(prop, id, presp, presp)
+	} else {
+		env, err = utils.CreateSignedTx(prop, id, presp)
+	}
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
-
-	return env, presp, err
+	return env, err
 }
 
 func processSignedCDS(cds *peer.ChaincodeDeploymentSpec, policy *common.SignaturePolicyEnvelope) ([]byte, error) {
@@ -217,6 +222,24 @@ func getSignedByMSPMemberPolicy(mspID string) ([]byte, error) {
 	return b, err
 }
 
+func getSignedByOneMemberTwicePolicy(mspID string) ([]byte, error) {
+	principal := &mspproto.MSPPrincipal{
+		PrincipalClassification: mspproto.MSPPrincipal_ROLE,
+		Principal:               utils.MarshalOrPanic(&mspproto.MSPRole{Role: mspproto.MSPRole_MEMBER, MspIdentifier: mspID})}
+
+	p := &common.SignaturePolicyEnvelope{
+		Version:    0,
+		Rule:       cauthdsl.NOutOf(2, []*common.SignaturePolicy{cauthdsl.SignedBy(0), cauthdsl.SignedBy(0)}),
+		Identities: []*mspproto.MSPPrincipal{principal},
+	}
+	b, err := utils.Marshal(p)
+	if err != nil {
+		return nil, fmt.Errorf("Could not marshal policy, err %s", err)
+	}
+
+	return b, err
+}
+
 func getSignedByMSPAdminPolicy(mspID string) ([]byte, error) {
 	p := cauthdsl.SignedByMspAdmin(mspID)
 
@@ -276,7 +299,7 @@ func TestInvoke(t *testing.T) {
 		t.Fatalf("vscc invoke should have failed")
 	}
 
-	tx, _, err := createTx()
+	tx, err := createTx(false)
 	if err != nil {
 		t.Fatalf("createTx returned err %s", err)
 	}
@@ -328,6 +351,24 @@ func TestInvoke(t *testing.T) {
 	if res := stub.MockInvoke("1", args); res.Status == shim.OK {
 		t.Fatalf("vscc invoke should have failed")
 	}
+
+	// bad path: signed by duplicated MSP identity
+	policy, err = getSignedByOneMemberTwicePolicy(mspid)
+	if err != nil {
+		t.Fatalf("failed getting policy, err %s", err)
+	}
+	tx, err = createTx(true)
+	if err != nil {
+		t.Fatalf("createTx returned err %s", err)
+	}
+	envBytes, err = utils.GetBytesEnvelope(tx)
+	if err != nil {
+		t.Fatalf("GetBytesEnvelope returned err %s", err)
+	}
+	args = [][]byte{[]byte("dv"), envBytes, policy}
+	if res := stub.MockInvoke("1", args); res.Status == shim.OK || res.Message != DUPLICATED_IDENTITY_ERROR {
+		t.Fatalf("vscc invoke should have failed due to policy evaluation failure caused by duplicated identity")
+	}
 }
 
 func TestInvalidFunction(t *testing.T) {