Skip to content

Commit

Permalink
Add an instantiation policy cache layer
Browse files Browse the repository at this point in the history
This commit adds a simple cache in front of the instantiation policy
checks performed by LSCC.  Once the chaincode data for a chaincode ID
has been retrieved from the filesystem, that data is stored into a map,
to be used against future instantiation policy checks.

Signed-off-by: Jason Yellick <jyellick@us.ibm.com>
  • Loading branch information
Jason Yellick authored and denyeart committed Dec 11, 2019
1 parent c2c5183 commit cce46ac
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 16 deletions.
58 changes: 43 additions & 15 deletions core/scc/lscc/lscc.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"bytes"
"fmt"
"regexp"
"sync"

"github.com/golang/protobuf/proto"
"github.com/hyperledger/fabric-chaincode-go/shim"
Expand Down Expand Up @@ -165,6 +166,8 @@ type SCC struct {

// BCCSP instance
BCCSP bccsp.BCCSP

PackageCache PackageCache
}

// PeerShim adapts the peer instance for use with LSCC by providing methods
Expand Down Expand Up @@ -201,26 +204,50 @@ func (p *PeerShim) PolicyManager(channelID string) (policies.Manager, bool) {
func (lscc *SCC) Name() string { return "lscc" }
func (lscc *SCC) Chaincode() shim.Chaincode { return lscc }

type PackageCache struct {
Mutex sync.RWMutex
ValidatedPackages map[string]*ccprovider.ChaincodeData
}

type LegacySecurity struct {
Support FilesystemSupport
Support FilesystemSupport
PackageCache *PackageCache
}

func (ls *LegacySecurity) SecurityCheckLegacyChaincode(cd *ccprovider.ChaincodeData) error {
ccpack, err := ls.Support.GetChaincodeFromLocalStorage(cd.ChaincodeID())
if err != nil {
return InvalidDeploymentSpecErr(err.Error())
}
ccid := cd.ChaincodeID()

ls.PackageCache.Mutex.RLock()
fsData, ok := ls.PackageCache.ValidatedPackages[ccid]
ls.PackageCache.Mutex.RUnlock()

if !ok {
ls.PackageCache.Mutex.Lock()
defer ls.PackageCache.Mutex.Unlock()
fsData, ok = ls.PackageCache.ValidatedPackages[ccid]
if !ok {
ccpack, err := ls.Support.GetChaincodeFromLocalStorage(cd.ChaincodeID())
if err != nil {
return InvalidDeploymentSpecErr(err.Error())
}

// This is 'the big security check', though it's no clear what's being accomplished
// here. Basically, it seems to try to verify that the chaincode defintion matches
// what's on the filesystem, which, might include instanatiation policy, but it's
// not obvious from the code, and was being checked separately, so we check it
// explicitly below.
if err = ccpack.ValidateCC(cd); err != nil {
return InvalidCCOnFSError(err.Error())
}
// This is 'the big security check', though it's no clear what's being accomplished
// here. Basically, it seems to try to verify that the chaincode defintion matches
// what's on the filesystem, which, might include instanatiation policy, but it's
// not obvious from the code, and was being checked separately, so we check it
// explicitly below.
if err = ccpack.ValidateCC(cd); err != nil {
return InvalidCCOnFSError(err.Error())
}

fsData := ccpack.GetChaincodeData()
if ls.PackageCache.ValidatedPackages == nil {
ls.PackageCache.ValidatedPackages = map[string]*ccprovider.ChaincodeData{}
}

fsData = ccpack.GetChaincodeData()
ls.PackageCache.ValidatedPackages[ccid] = fsData
}
}

// we have the info from the fs, check that the policy
// matches the one on the file system if one was specified;
Expand Down Expand Up @@ -260,7 +287,8 @@ func (lscc *SCC) ChaincodeEndorsementInfo(channelID, chaincodeName string, qe le
}

ls := &LegacySecurity{
Support: lscc.Support,
Support: lscc.Support,
PackageCache: &lscc.PackageCache,
}

err = ls.SecurityCheckLegacyChaincode(chaincodeData)
Expand Down
19 changes: 18 additions & 1 deletion core/scc/lscc/lscc_noncc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ var _ = Describe("LSCC", func() {
var (
fakeCCPackage *mock.CCPackage
legacySecurity *lscc.LegacySecurity
packageCache *lscc.PackageCache
)

BeforeEach(func() {
Expand All @@ -82,8 +83,11 @@ var _ = Describe("LSCC", func() {

fakeSupport.GetChaincodeFromLocalStorageReturns(fakeCCPackage, nil)

packageCache = &lscc.PackageCache{}

legacySecurity = &lscc.LegacySecurity{
Support: fakeSupport,
Support: fakeSupport,
PackageCache: packageCache,
}
})

Expand All @@ -95,6 +99,13 @@ var _ = Describe("LSCC", func() {
Expect(fakeCCPackage.ValidateCCArgsForCall(0)).To(Equal(ccData))
})

It("caches the result of the package validation", func() {
err := legacySecurity.SecurityCheckLegacyChaincode(ccData)
Expect(err).NotTo(HaveOccurred())

Expect(packageCache.ValidatedPackages).To(HaveKey("chaincode-data-name:version"))
})

Context("when cc package validation fails", func() {
BeforeEach(func() {
fakeCCPackage.ValidateCCReturns(errors.New("fake-validation-error"))
Expand All @@ -104,6 +115,12 @@ var _ = Describe("LSCC", func() {
err := legacySecurity.SecurityCheckLegacyChaincode(ccData)
Expect(err).To(MatchError(lscc.InvalidCCOnFSError("fake-validation-error")))
})

It("does not cache the result of the package validation", func() {
legacySecurity.SecurityCheckLegacyChaincode(ccData)

Expect(packageCache.ValidatedPackages).NotTo(HaveKey("chaincode-data-name:version"))
})
})

Context("when the instantiation policy doesn't match", func() {
Expand Down

0 comments on commit cce46ac

Please sign in to comment.