[FAB-4751] disallow external CC from registering

Externals chaincodes are allowed to register even when
not in dev mode. There is no reason to.

This CR does two things
. dissalows external chaincode from registering when
  not in dev mode
. tightens the window between launch and registration

patch 2
. fixed docker UT
. added some tests

patch 3
. fixed a typo in a log
. removed WIP

Change-Id: I733b058ac8ec5f7922110e691cc48fa2c6d3d04c
Signed-off-by: Srinivasan Muralidharan <[email protected]>

Author: Srinivasan Muralidharan

core/chaincode/chaincode_support.go

@@ -102,8 +102,9 @@ func GetChain() *ChaincodeSupport {
 	return theChaincodeSupport
 }
 
-//call this under lock
 func (chaincodeSupport *ChaincodeSupport) preLaunchSetup(chaincode string) chan bool {
+	chaincodeSupport.runningChaincodes.Lock()
+	defer chaincodeSupport.runningChaincodes.Unlock()
 	//register placeholder Handler. This will be transferred in registerHandler
 	//NOTE: from this point, existence of handler for this chaincode means the chaincode
 	//is in the process of getting started (or has been started)
@@ -261,6 +262,11 @@ func (chaincodeSupport *ChaincodeSupport) registerHandler(chaincodehandler *Hand
 		chaincodehandler.readyNotify = chrte2.handler.readyNotify
 		chrte2.handler = chaincodehandler
 	} else {
+		if chaincodeSupport.userRunsCC == false {
+			//this chaincode was not launched by the peer and is attempting
+			//to register. Don't allow this.
+			return fmt.Errorf("peer will not accepting external chaincode connection %v (except in dev mode)", chaincodehandler.ChaincodeID)
+		}
 		chaincodeSupport.runningChaincodes.chaincodeMap[key] = &chaincodeRTEnv{handler: chaincodehandler}
 	}
 
@@ -405,8 +411,6 @@ func (chaincodeSupport *ChaincodeSupport) launchAndWaitForRegister(ctxt context.
 		return fmt.Errorf("Error chaincode is being launched: %s", canName)
 	}
 
-	//chaincodeHasBeenLaunch false... its not in the map, add it and proceed to launch
-	notfy := chaincodeSupport.preLaunchSetup(canName)
 	chaincodeSupport.runningChaincodes.Unlock()
 
 	//launch the chaincode
@@ -422,7 +426,16 @@ func (chaincodeSupport *ChaincodeSupport) launchAndWaitForRegister(ctxt context.
 
 	vmtype, _ := chaincodeSupport.getVMType(cds)
 
-	sir := container.StartImageReq{CCID: ccintf.CCID{ChaincodeSpec: cds.ChaincodeSpec, NetworkID: chaincodeSupport.peerNetworkID, PeerID: chaincodeSupport.peerID, Version: cccid.Version}, Builder: builder, Args: args, Env: env}
+	//set up the shadow handler JIT before container launch to
+	//reduce window of when an external chaincode can sneak in
+	//and use the launching context and make it its own
+	var notfy chan bool
+	preLaunchFunc := func() error {
+		notfy = chaincodeSupport.preLaunchSetup(canName)
+		return nil
+	}
+
+	sir := container.StartImageReq{CCID: ccintf.CCID{ChaincodeSpec: cds.ChaincodeSpec, NetworkID: chaincodeSupport.peerNetworkID, PeerID: chaincodeSupport.peerID, Version: cccid.Version}, Builder: builder, Args: args, Env: env, PrelaunchFunc: preLaunchFunc}
 
 	ipcCtxt := context.WithValue(ctxt, ccintf.GetCCHandlerKey(), chaincodeSupport)
 

core/container/api/core.go

@@ -25,11 +25,12 @@ import (
 )
 
 type BuildSpecFactory func() (io.Reader, error)
+type PrelaunchFunc func() error
 
 //abstract virtual image for supporting arbitrary virual machines
 type VM interface {
 	Deploy(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, reader io.Reader) error
-	Start(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, builder BuildSpecFactory) error
+	Start(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, builder BuildSpecFactory, preLaunchFunc PrelaunchFunc) error
 	Stop(ctxt context.Context, ccid ccintf.CCID, timeout uint, dontkill bool, dontremove bool) error
 	Destroy(ctxt context.Context, ccid ccintf.CCID, force bool, noprune bool) error
 	GetVMName(ccID ccintf.CCID) (string, error)

core/container/controller.go

@@ -153,15 +153,16 @@ func (bp CreateImageReq) getCCID() ccintf.CCID {
 //StartImageReq - properties for starting a container.
 type StartImageReq struct {
 	ccintf.CCID
-	Builder api.BuildSpecFactory
-	Args    []string
-	Env     []string
+	Builder       api.BuildSpecFactory
+	Args          []string
+	Env           []string
+	PrelaunchFunc api.PrelaunchFunc
 }
 
 func (si StartImageReq) do(ctxt context.Context, v api.VM) VMCResp {
 	var resp VMCResp
 
-	if err := v.Start(ctxt, si.CCID, si.Args, si.Env, si.Builder); err != nil {
+	if err := v.Start(ctxt, si.CCID, si.Args, si.Env, si.Builder, si.PrelaunchFunc); err != nil {
 		resp = VMCResp{Err: err}
 	} else {
 		resp = VMCResp{}

core/container/dockercontroller/dockercontroller.go

@@ -207,7 +207,7 @@ func (vm *DockerVM) Deploy(ctxt context.Context, ccid ccintf.CCID,
 
 //Start starts a container using a previously created docker image
 func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
-	args []string, env []string, builder container.BuildSpecFactory) error {
+	args []string, env []string, builder container.BuildSpecFactory, prelaunchFunc container.PrelaunchFunc) error {
 	imageID, err := vm.GetVMName(ccid)
 	if err != nil {
 		return err
@@ -330,6 +330,12 @@ func (vm *DockerVM) Start(ctxt context.Context, ccid ccintf.CCID,
 		}()
 	}
 
+	if prelaunchFunc != nil {
+		if err = prelaunchFunc(); err != nil {
+			return err
+		}
+	}
+
 	// start container with HostConfig was deprecated since v1.10 and removed in v1.2
 	err = client.StartContainer(containerID, nil)
 	if err != nil {

core/container/dockercontroller/dockercontroller_test.go

@@ -106,19 +106,19 @@ func Test_Start(t *testing.T) {
 	// case 1: getMockClient returns error
 	dvm.getClientFnc = getMockClient
 	getClientErr = true
-	err := dvm.Start(ctx, ccid, args, env, nil)
+	err := dvm.Start(ctx, ccid, args, env, nil, nil)
 	testerr(t, err, false)
 	getClientErr = false
 
 	// case 2: dockerClient.CreateContainer returns error
 	createErr = true
-	err = dvm.Start(ctx, ccid, args, env, nil)
+	err = dvm.Start(ctx, ccid, args, env, nil, nil)
 	testerr(t, err, false)
 	createErr = false
 
 	// case 3: dockerClient.CreateContainer returns docker.noSuchImgErr
 	noSuchImgErr = true
-	err = dvm.Start(ctx, ccid, args, env, nil)
+	err = dvm.Start(ctx, ccid, args, env, nil, nil)
 	testerr(t, err, false)
 
 	chaincodePath := "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example01"
@@ -136,35 +136,53 @@ func Test_Start(t *testing.T) {
 	// docker.noSuchImgErr and dockerClient.Start returns error
 	viper.Set("vm.docker.attachStdout", true)
 	startErr = true
-	err = dvm.Start(ctx, ccid, args, env, bldr)
+	err = dvm.Start(ctx, ccid, args, env, bldr, nil)
 	testerr(t, err, false)
 	startErr = false
 
 	// Success cases
-	err = dvm.Start(ctx, ccid, args, env, bldr)
+	err = dvm.Start(ctx, ccid, args, env, bldr, nil)
 	testerr(t, err, true)
 	noSuchImgErr = false
 
 	// dockerClient.StopContainer returns error
 	stopErr = true
-	err = dvm.Start(ctx, ccid, args, env, nil)
+	err = dvm.Start(ctx, ccid, args, env, nil, nil)
 	testerr(t, err, true)
 	stopErr = false
 
 	// dockerClient.KillContainer returns error
 	killErr = true
-	err = dvm.Start(ctx, ccid, args, env, nil)
+	err = dvm.Start(ctx, ccid, args, env, nil, nil)
 	testerr(t, err, true)
 	killErr = false
 
 	// dockerClient.RemoveContainer returns error
 	removeErr = true
-	err = dvm.Start(ctx, ccid, args, env, nil)
+	err = dvm.Start(ctx, ccid, args, env, nil, nil)
 	testerr(t, err, true)
 	removeErr = false
 
-	err = dvm.Start(ctx, ccid, args, env, nil)
+	err = dvm.Start(ctx, ccid, args, env, nil, nil)
 	testerr(t, err, true)
+
+	//test preLaunchFunc works correctly
+	preLaunchStr := "notset"
+	preLaunchFunc := func() error {
+		preLaunchStr = "set"
+		return nil
+	}
+
+	err = dvm.Start(ctx, ccid, args, env, nil, preLaunchFunc)
+	testerr(t, err, true)
+	assert.Equal(t, preLaunchStr, "set")
+
+	preLaunchFunc = func() error {
+		return fmt.Errorf("testing error path")
+	}
+
+	err = dvm.Start(ctx, ccid, args, env, nil, preLaunchFunc)
+	testerr(t, err, false)
 }
 
 func Test_Stop(t *testing.T) {

core/container/inproccontroller/inproccontroller.go

@@ -154,7 +154,7 @@ func (ipc *inprocContainer) launchInProc(ctxt context.Context, id string, args [
 }
 
 //Start starts a previously registered system codechain
-func (vm *InprocVM) Start(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, builder container.BuildSpecFactory) error {
+func (vm *InprocVM) Start(ctxt context.Context, ccid ccintf.CCID, args []string, env []string, builder container.BuildSpecFactory, prelaunchFunc container.PrelaunchFunc) error {
 	path := ccid.ChaincodeSpec.ChaincodeId.Path
 
 	ipctemplate := typeRegistry[path]
@@ -182,6 +182,12 @@ func (vm *InprocVM) Start(ctxt context.Context, ccid ccintf.CCID, args []string,
 		return fmt.Errorf("in-process communication generator not supplied")
 	}
 
+	if prelaunchFunc != nil {
+		if err = prelaunchFunc(); err != nil {
+			return err
+		}
+	}
+
 	ipc.running = true
 
 	go func() {