[FAB-12287] handle successful decode of bad data

LSCC handles CDSPackage and SignedCDSPackage instances in pretty much
the same way. The current pattern seems to be to try to decode data from
lifecycle into CDSPackage and, if it works, use it; if it doesn't try to
decode it into a SignedCDSPackage.

That would be great if protobuf were guaranteed to fail decoding the
wrong object but it doesn't.

In preparation for an upgrade to the proto package, the CDSPackage
validation needs to be extended to catch more scenarios where bad data
has been "successfully" unmarshaled into an object. This needs to be
done in a way that increases confidence in the decoding process without
duplicating validation that lives in lifecycle.

Change-Id: Ib53ddf1260a08e2f2178ce0f8862da5429aff87b
Signed-off-by: Matthew Sykes <sykesmat@us.ibm.com>

Author: sykesm

core/common/ccprovider/ccprovider.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"unicode"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/common/chaincode"
@@ -83,6 +84,16 @@ func GetChaincodePackage(ccname string, ccversion string) ([]byte, error) {
 	return GetChaincodePackageFromPath(ccname, ccversion, chaincodeInstallPath)
 }
 
+// isPrintable is used by CDSPackage and SignedCDSPackage validation to
+// detect garbage strings in unmarshaled proto fields where printable
+// characters are expected.
+func isPrintable(name string) bool {
+	notASCII := func(r rune) bool {
+		return !unicode.IsPrint(r)
+	}
+	return strings.IndexFunc(name, notASCII) == -1
+}
+
 // GetChaincodePackage returns the chaincode package from the file system
 func GetChaincodePackageFromPath(ccname string, ccversion string, ccInstallPath string) ([]byte, error) {
 	path := fmt.Sprintf("%s/%s.%s", ccInstallPath, ccname, ccversion)
@@ -306,7 +317,7 @@ func GetCCPackage(buf []byte) (CCPackage, error) {
 		return scds, nil
 	}
 
-	return nil, errors.New("could not unmarshaled chaincode package to CDS or SignedCDS")
+	return nil, errors.New("could not unmarshal chaincode package to CDS or SignedCDS")
 }
 
 // GetInstalledChaincodes returns a map whose key is the chaincode id and

core/common/ccprovider/ccprovider_test.go

@@ -7,13 +7,12 @@ SPDX-License-Identifier: Apache-2.0
 package ccprovider_test
 
 import (
-	"fmt"
+	"crypto/sha256"
 	"io/ioutil"
-	"math/rand"
 	"os"
 	"path"
+	"strings"
 	"testing"
-	"time"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/common/chaincode"
@@ -24,7 +23,7 @@ import (
 )
 
 func TestInstalledCCs(t *testing.T) {
-	tmpDir := setupDirectoryStructure(t)
+	tmpDir, hashes := setupDirectoryStructure(t)
 	defer func() {
 		os.RemoveAll(tmpDir)
 	}()
@@ -44,12 +43,12 @@ func TestInstalledCCs(t *testing.T) {
 				{
 					Name:    "example02",
 					Version: "1.0",
-					Id:      []byte{45, 186, 93, 188, 51, 158, 115, 22, 174, 162, 104, 63, 175, 131, 156, 27, 123, 30, 226, 49, 61, 183, 146, 17, 37, 136, 17, 141, 240, 102, 170, 53},
+					Id:      hashes["example02.1.0"],
 				},
 				{
 					Name:    "example04",
 					Version: "1",
-					Id:      []byte{45, 186, 93, 188, 51, 158, 115, 22, 174, 162, 104, 63, 175, 131, 156, 27, 123, 30, 226, 49, 61, 183, 146, 17, 37, 136, 17, 141, 240, 102, 170, 53},
+					Id:      hashes["example04.1"],
 				},
 			},
 			directory: "nonempty",
@@ -106,15 +105,15 @@ func TestInstalledCCs(t *testing.T) {
 	}
 }
 
-func setupDirectoryStructure(t *testing.T) string {
+func setupDirectoryStructure(t *testing.T) (string, map[string][]byte) {
 	files := []string{
 		"example02.1.0", // Version contains the delimiter '.' is a valid case
 		"example03",     // No version specified
 		"example04.1",   // Version doesn't contain the '.' delimiter
 	}
-	rand.Seed(time.Now().UnixNano())
-	tmp := path.Join(os.TempDir(), fmt.Sprintf("%d", rand.Int()))
-	assert.NoError(t, os.Mkdir(tmp, 0755))
+	hashes := map[string][]byte{}
+	tmp, err := ioutil.TempDir("", "test-installed-cc")
+	assert.NoError(t, err)
 	dir := path.Join(tmp, "empty")
 	assert.NoError(t, os.Mkdir(dir, 0755))
 	dir = path.Join(tmp, "nonempty")
@@ -124,22 +123,42 @@ func setupDirectoryStructure(t *testing.T) string {
 	dir = path.Join(tmp, "nopermissionforfiles")
 	assert.NoError(t, os.Mkdir(dir, 0755))
 	noPermissionFile := path.Join(tmp, "nopermissionforfiles", "nopermission.1")
-	_, err := os.Create(noPermissionFile)
+	_, err = os.Create(noPermissionFile)
 	assert.NoError(t, err)
 	dir = path.Join(tmp, "nonempty")
 	assert.NoError(t, os.Mkdir(path.Join(tmp, "nonempty", "directory"), 0755))
 	for _, f := range files {
+		var name, ver string
+		parts := strings.SplitN(f, ".", 2)
+		name = parts[0]
+		if len(parts) > 1 {
+			ver = parts[1]
+		}
 		file, err := os.Create(path.Join(dir, f))
 		assert.NoError(t, err)
 		cds := &peer.ChaincodeDeploymentSpec{
 			ChaincodeSpec: &peer.ChaincodeSpec{
-				ChaincodeId: &peer.ChaincodeID{},
+				ChaincodeId: &peer.ChaincodeID{Name: name, Version: ver},
 			},
 		}
+
+		codehash := sha256.New()
+		codehash.Write(cds.CodePackage)
+
+		metahash := sha256.New()
+		metahash.Write([]byte(name))
+		metahash.Write([]byte(ver))
+
+		hash := sha256.New()
+		hash.Write(codehash.Sum(nil))
+		hash.Write(metahash.Sum(nil))
+
+		hashes[f] = hash.Sum(nil)
+
 		b, _ := proto.Marshal(cds)
 		file.Write(b)
 		file.Close()
 	}
 
-	return tmp
+	return tmp, hashes
 }

core/common/ccprovider/cdspackage.go

@@ -184,6 +184,16 @@ func (ccpack *CDSPackage) ValidateCC(ccdata *ChaincodeData) error {
 		return fmt.Errorf("nil data")
 	}
 
+	// This is a hack. LSCC expects a specific LSCC error when names are invalid so it
+	// has its own validation code. We can't use that error because of import cycles.
+	// Unfortunately, we also need to check if what have makes some sort of sense as
+	// protobuf will gladly deserialize garbage and there are paths where we assume that
+	// a successful unmarshal means everything works but, if it fails, we try to unmarshal
+	// into something different.
+	if !isPrintable(ccdata.Name) {
+		return fmt.Errorf("invalid chaincode name: %q", ccdata.Name)
+	}
+
 	if ccdata.Name != ccpack.depSpec.ChaincodeSpec.ChaincodeId.Name || ccdata.Version != ccpack.depSpec.ChaincodeSpec.ChaincodeId.Version {
 		return fmt.Errorf("invalid chaincode data %v (%v)", ccdata, ccpack.depSpec.ChaincodeSpec.ChaincodeId)
 	}
@@ -236,7 +246,12 @@ func (ccpack *CDSPackage) InitFromPath(ccname string, ccversion string, path str
 		return nil, nil, err
 	}
 
-	if _, err = ccpack.InitFromBuffer(buf); err != nil {
+	ccdata, err := ccpack.InitFromBuffer(buf)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	if err := ccpack.ValidateCC(ccdata); err != nil {
 		return nil, nil, err
 	}
 

core/common/ccprovider/cdspackage_test.go

@@ -212,9 +212,57 @@ func TestValidateCCErrorPaths(t *testing.T) {
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "uninitialized package", "Unexpected error returned")
 
-	cpack.depSpec = &pb.ChaincodeDeploymentSpec{ChaincodeSpec: &pb.ChaincodeSpec{Type: 1, ChaincodeId: &pb.ChaincodeID{Name: "testcc", Version: "0"},
-		Input: &pb.ChaincodeInput{Args: [][]byte{[]byte("")}}}, CodePackage: []byte("code")}
+	cpack.depSpec = &pb.ChaincodeDeploymentSpec{
+		CodePackage: []byte("code"),
+		ChaincodeSpec: &pb.ChaincodeSpec{
+			Type:        1,
+			ChaincodeId: &pb.ChaincodeID{Name: "testcc", Version: "0"},
+			Input:       &pb.ChaincodeInput{Args: [][]byte{[]byte("")}},
+		},
+	}
 	err = cpack.ValidateCC(ccdata)
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "nil data", "Unexpected error returned")
+
+	// invalid encoded name
+	cpack = &CDSPackage{}
+	ccdata = &ChaincodeData{Name: "\027"}
+	cpack.depSpec = &pb.ChaincodeDeploymentSpec{
+		CodePackage: []byte("code"),
+		ChaincodeSpec: &pb.ChaincodeSpec{
+			ChaincodeId: &pb.ChaincodeID{Name: ccdata.Name, Version: "0"},
+		},
+	}
+	cpack.data = &CDSData{}
+	err = cpack.ValidateCC(ccdata)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), `invalid chaincode name: "\x17"`)
+
+	// mismatched names
+	cpack = &CDSPackage{}
+	ccdata = &ChaincodeData{Name: "Tom"}
+	cpack.depSpec = &pb.ChaincodeDeploymentSpec{
+		CodePackage: []byte("code"),
+		ChaincodeSpec: &pb.ChaincodeSpec{
+			ChaincodeId: &pb.ChaincodeID{Name: "Jerry", Version: "0"},
+		},
+	}
+	cpack.data = &CDSData{}
+	err = cpack.ValidateCC(ccdata)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), `invalid chaincode data name:"Tom"  (name:"Jerry" version:"0" )`)
+
+	// mismatched versions
+	cpack = &CDSPackage{}
+	ccdata = &ChaincodeData{Name: "Tom", Version: "1"}
+	cpack.depSpec = &pb.ChaincodeDeploymentSpec{
+		CodePackage: []byte("code"),
+		ChaincodeSpec: &pb.ChaincodeSpec{
+			ChaincodeId: &pb.ChaincodeID{Name: ccdata.Name, Version: "0"},
+		},
+	}
+	cpack.data = &CDSData{}
+	err = cpack.ValidateCC(ccdata)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), `invalid chaincode data name:"Tom" version:"1"  (name:"Tom" version:"0" )`)
 }

core/common/ccprovider/sigcdspackage.go

@@ -229,6 +229,16 @@ func (ccpack *SignedCDSPackage) ValidateCC(ccdata *ChaincodeData) error {
 		return fmt.Errorf("chaincode deployment spec cannot be nil in a package")
 	}
 
+	// This is a hack. LSCC expects a specific LSCC error when names are invalid so it
+	// has its own validation code. We can't use that error because of import cycles.
+	// Unfortunately, we also need to check if what have makes some sort of sense as
+	// protobuf will gladly deserialize garbage and there are paths where we assume that
+	// a successful unmarshal means everything works but, if it fails, we try to unmarshal
+	// into something different.
+	if !isPrintable(ccdata.Name) {
+		return fmt.Errorf("invalid chaincode name: %q", ccdata.Name)
+	}
+
 	if ccdata.Name != ccpack.depSpec.ChaincodeSpec.ChaincodeId.Name || ccdata.Version != ccpack.depSpec.ChaincodeSpec.ChaincodeId.Version {
 		return fmt.Errorf("invalid chaincode data %v (%v)", ccdata, ccpack.depSpec.ChaincodeSpec.ChaincodeId)
 	}

core/common/ccprovider/sigcdspackage_test.go

@@ -258,6 +258,11 @@ func TestValidateSignedCCErrorPaths(t *testing.T) {
 	assert.Error(t, err)
 	assert.Contains(t, err.Error(), "chaincode deployment spec cannot be nil in a package", "Unexpected error validating package")
 	ccpack.depSpec = depspec
+
+	cd = &ChaincodeData{Name: "\027", Version: "0"}
+	err = ccpack.ValidateCC(cd)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), `invalid chaincode name: "\x17"`)
 }
 
 func TestSigCDSGetCCPackage(t *testing.T) {