Enhance validation of chaincode names/versions in LCCC

This CR ensures chaincode names and versions only consist
of the allowable characters. Chaincode names should only consist of
alphanumerics, '_', and '-' while versions can also include '.' . It
also fixes the GetInstalledChaincodes query logic to account for the
fact that '.' is allowed in versions. Finally, this CR reworks some
of the test case logic for install, deploy, and upgrade to allow
for easier addition of standard test cases for those functions.

https://jira.hyperledger.org/browse/FAB-2922

Change-Id: I4ac29ed619eb91d2a04f4896546443ae81da432c
Signed-off-by: Will Lahti <[email protected]>

Author: wlahti

core/common/ccprovider/ccprovider.go

@@ -122,7 +122,9 @@ func GetInstalledChaincodes() (*pb.ChaincodeQueryResponse, error) {
 	var ccInfoArray []*pb.ChaincodeInfo
 
 	for _, file := range files {
-		fileNameArray := strings.Split(file.Name(), ".")
+		// split at first period as chaincode versions can contain periods while
+		// chaincode names cannot
+		fileNameArray := strings.SplitN(file.Name(), ".", 2)
 
 		// check that length is 2 as expected, otherwise skip to next cc file
 		if len(fileNameArray) == 2 {

core/scc/lccc/lccc.go

@@ -18,9 +18,8 @@ package lccc
 
 import (
 	"fmt"
+	"regexp"
 	"sort"
-	"strconv"
-	"strings"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/common/cauthdsl"
@@ -74,8 +73,8 @@ const (
 	//GETINSTALLEDCHAINCODES gets the installed chaincodes on a peer
 	GETINSTALLEDCHAINCODES = "getinstalledchaincodes"
 
-	//characters used in chaincodenamespace
-	specialChars = "/:[]${}"
+	allowedCharsChaincodeName = "[A-Za-z0-9_-]+"
+	allowedCharsVersion       = "[A-Za-z0-9_.-]+"
 )
 
 //---------- the LCCC -----------------
@@ -164,37 +163,44 @@ func (f InvalidChainNameErr) Error() string {
 type InvalidChaincodeNameErr string
 
 func (f InvalidChaincodeNameErr) Error() string {
-	return fmt.Sprintf("invalid chain code name %s", string(f))
+	return fmt.Sprintf("invalid chaincode name %s", string(f))
 }
 
-//MarshallErr error marshaling/unmarshalling
-type MarshallErr string
+//EmptyChaincodeNameErr trying to upgrade to same version of Chaincode
+type EmptyChaincodeNameErr string
 
-func (m MarshallErr) Error() string {
-	return fmt.Sprintf("error while marshalling %s", string(m))
+func (f EmptyChaincodeNameErr) Error() string {
+	return fmt.Sprint("chaincode name not provided")
 }
 
-//IdenticalVersionErr trying to upgrade to same version of Chaincode
-type IdenticalVersionErr string
-
-func (f IdenticalVersionErr) Error() string {
-	return fmt.Sprintf("chain code with the same version exists %s", string(f))
-}
-
-//InvalidVersionErr trying to upgrade to same version of Chaincode
+//InvalidVersionErr invalid version error
 type InvalidVersionErr string
 
 func (f InvalidVersionErr) Error() string {
-	return fmt.Sprintf("invalid version %s", string(f))
+	return fmt.Sprintf("invalid chaincode version %s", string(f))
 }
 
-//EmptyVersionErr trying to upgrade to same version of Chaincode
+//EmptyVersionErr empty version error
 type EmptyVersionErr string
 
 func (f EmptyVersionErr) Error() string {
 	return fmt.Sprintf("version not provided for chaincode %s", string(f))
 }
 
+//MarshallErr error marshaling/unmarshalling
+type MarshallErr string
+
+func (m MarshallErr) Error() string {
+	return fmt.Sprintf("error while marshalling %s", string(m))
+}
+
+//IdenticalVersionErr trying to upgrade to same version of Chaincode
+type IdenticalVersionErr string
+
+func (f IdenticalVersionErr) Error() string {
+	return fmt.Sprintf("chaincode with the same version exists %s", string(f))
+}
+
 //-------------- helper functions ------------------
 //create the chaincode on the given chain
 func (lccc *LifeCycleSysCC) createChaincode(stub shim.ChaincodeStubInterface, chainname string, ccname string, version string, cccode []byte, policy []byte, escc []byte, vscc []byte) (*ccprovider.ChaincodeData, error) {
@@ -338,35 +344,60 @@ func (lccc *LifeCycleSysCC) isValidChainName(chainname string) bool {
 	return true
 }
 
-//check validity of chaincode name
-func (lccc *LifeCycleSysCC) isValidChaincodeName(chaincodename string) bool {
-	//TODO we probably need more checks
-	if chaincodename == "" {
-		return false
+// isValidChaincodeName checks the validity of chaincode name. Chaincode names
+// should never be blank and should only consist of alphanumerics, '_', and '-'
+func (lccc *LifeCycleSysCC) isValidChaincodeName(chaincodeName string) error {
+	if chaincodeName == "" {
+		return EmptyChaincodeNameErr("")
+	}
+
+	if !isValidCCNameOrVersion(chaincodeName, allowedCharsChaincodeName) {
+		return InvalidChaincodeNameErr(chaincodeName)
+	}
+
+	return nil
+}
+
+// isValidChaincodeVersion checks the validity of chaincode version. Versions
+// should never be blank and should only consist of alphanumerics, '_',  '-',
+// and '.'
+func (lccc *LifeCycleSysCC) isValidChaincodeVersion(chaincodeName string, version string) error {
+	if version == "" {
+		return EmptyVersionErr(chaincodeName)
+	}
+
+	if !isValidCCNameOrVersion(version, allowedCharsVersion) {
+		return InvalidVersionErr(version)
 	}
 
-	//do not allow special characters in chaincode name
-	if strings.ContainsAny(chaincodename, specialChars) {
+	return nil
+}
+
+func isValidCCNameOrVersion(ccNameOrVersion string, regExp string) bool {
+	re, _ := regexp.Compile(regExp)
+
+	matched := re.FindString(ccNameOrVersion)
+	if len(matched) != len(ccNameOrVersion) {
 		return false
 	}
 
 	return true
 }
 
-//this implements "install" Invoke transaction
+// executeInstall implements the "install" Invoke transaction
 func (lccc *LifeCycleSysCC) executeInstall(stub shim.ChaincodeStubInterface, depSpec []byte) error {
 	cds, err := utils.GetChaincodeDeploymentSpec(depSpec)
 
 	if err != nil {
 		return err
 	}
 
-	if !lccc.isValidChaincodeName(cds.ChaincodeSpec.ChaincodeId.Name) {
-		return InvalidChaincodeNameErr(cds.ChaincodeSpec.ChaincodeId.Name)
+	if err = lccc.isValidChaincodeName(cds.ChaincodeSpec.ChaincodeId.Name); err != nil {
+		return err
 	}
 
-	if cds.ChaincodeSpec.ChaincodeId.Version == "" {
-		return EmptyVersionErr(cds.ChaincodeSpec.ChaincodeId.Name)
+	if err = lccc.isValidChaincodeVersion(cds.ChaincodeSpec.ChaincodeId.Name, cds.ChaincodeSpec.ChaincodeId.Version); err != nil {
+		return err
 	}
 
 	if err = ccprovider.PutChaincodeIntoFS(cds); err != nil {
@@ -376,16 +407,20 @@ func (lccc *LifeCycleSysCC) executeInstall(stub shim.ChaincodeStubInterface, dep
 	return err
 }
 
-//this implements "deploy" Invoke transaction
+// executeDeploy implements the "instantiate" Invoke transaction
 func (lccc *LifeCycleSysCC) executeDeploy(stub shim.ChaincodeStubInterface, chainname string, depSpec []byte, policy []byte, escc []byte, vscc []byte) error {
 	cds, err := utils.GetChaincodeDeploymentSpec(depSpec)
 
 	if err != nil {
 		return err
 	}
 
-	if !lccc.isValidChaincodeName(cds.ChaincodeSpec.ChaincodeId.Name) {
-		return InvalidChaincodeNameErr(cds.ChaincodeSpec.ChaincodeId.Name)
+	if err = lccc.isValidChaincodeName(cds.ChaincodeSpec.ChaincodeId.Name); err != nil {
+		return err
+	}
+
+	if err = lccc.isValidChaincodeVersion(cds.ChaincodeSpec.ChaincodeId.Name, cds.ChaincodeSpec.ChaincodeId.Version); err != nil {
+		return err
 	}
 
 	if err = lccc.acl(stub, chainname, cds); err != nil {
@@ -397,40 +432,12 @@ func (lccc *LifeCycleSysCC) executeDeploy(stub shim.ChaincodeStubInterface, chai
 		return ExistsErr(cds.ChaincodeSpec.ChaincodeId.Name)
 	}
 
-	if cds.ChaincodeSpec.ChaincodeId.Version == "" {
-		return EmptyVersionErr(cds.ChaincodeSpec.ChaincodeId.Name)
-	}
-
 	_, err = lccc.createChaincode(stub, chainname, cds.ChaincodeSpec.ChaincodeId.Name, cds.ChaincodeSpec.ChaincodeId.Version, depSpec, policy, escc, vscc)
 
 	return err
 }
 
-func (lccc *LifeCycleSysCC) getUpgradeVersion(cd *ccprovider.ChaincodeData, cds *pb.ChaincodeDeploymentSpec) (string, error) {
-	if cd.Version == cds.ChaincodeSpec.ChaincodeId.Version {
-		return "", IdenticalVersionErr(cds.ChaincodeSpec.ChaincodeId.Name)
-	}
-
-	if cds.ChaincodeSpec.ChaincodeId.Version != "" {
-		return cds.ChaincodeSpec.ChaincodeId.Version, nil
-	}
-
-	//user did not specifcy Version. the previous version better be a number
-	v, err := strconv.ParseInt(cd.Version, 10, 32)
-
-	//This should never happen as long we don't expose version as version is computed internally
-	//so panic till we find a need to relax
-	if err != nil {
-		return "", InvalidVersionErr(cd.Version)
-	}
-
-	// replace the ChaincodeDeploymentSpec using the next version
-	newVersion := fmt.Sprintf("%d", (v + 1))
-
-	return newVersion, nil
-}
-
-//this implements "upgrade" Invoke transaction
+// executeUpgrade implements the "upgrade" Invoke transaction.
 func (lccc *LifeCycleSysCC) executeUpgrade(stub shim.ChaincodeStubInterface, chainName string, depSpec []byte, policy []byte, escc []byte, vscc []byte) ([]byte, error) {
 	cds, err := utils.GetChaincodeDeploymentSpec(depSpec)
 	if err != nil {
@@ -442,8 +449,12 @@ func (lccc *LifeCycleSysCC) executeUpgrade(stub shim.ChaincodeStubInterface, cha
 	}
 
 	chaincodeName := cds.ChaincodeSpec.ChaincodeId.Name
-	if !lccc.isValidChaincodeName(chaincodeName) {
-		return nil, InvalidChaincodeNameErr(chaincodeName)
+	if err = lccc.isValidChaincodeName(chaincodeName); err != nil {
+		return nil, err
+	}
+
+	if err = lccc.isValidChaincodeVersion(chaincodeName, cds.ChaincodeSpec.ChaincodeId.Version); err != nil {
+		return nil, err
 	}
 
 	// check for existence of chaincode
@@ -452,11 +463,12 @@ func (lccc *LifeCycleSysCC) executeUpgrade(stub shim.ChaincodeStubInterface, cha
 		return nil, NotFoundErr(chainName)
 	}
 
-	ver, err := lccc.getUpgradeVersion(cd, cds)
-	if err != nil {
-		return nil, err
+	if cd.Version == cds.ChaincodeSpec.ChaincodeId.Version {
+		return nil, IdenticalVersionErr(cds.ChaincodeSpec.ChaincodeId.Name)
 	}
 
+	ver := cds.ChaincodeSpec.ChaincodeId.Version
+
 	newCD, err := lccc.upgradeChaincode(stub, chainName, chaincodeName, ver, depSpec, policy, escc, vscc)
 	if err != nil {
 		return nil, err