[FAB-16574] Cache results of msp.Validate function

As part of the policy rework, we need to cache the result of the call to msp.Validate() over an identity so that identity validation may be performed in parallel prior to the policy evaluation. This way, the policy evaluation step may be performed sequentially without any significant performance hit: indeed policy evaluation calls msp.SatisfiesPrincipal which internally calls Validate again. Caching will ensure that the latter call simply returns the validation status that was previoulsy determined. This change is safe since MSPs are immutable and identities belong to a specific MSP instance. Change-Id: Ifce11a0fd25cf25ff52982b3480a8a5589d0a679 Signed-off-by: Alessandro Sorniotti <ale.linux@sopit.net>
hyperledger · Nov 20, 2019 · f747ff1 · f747ff1
1 parent 5c49542
commit f747ff1
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 12 deletions.
diff --git a/msp/identities.go b/msp/identities.go
@@ -12,6 +12,7 @@ import (
 	"crypto/x509"
 	"encoding/hex"
 	"encoding/pem"
+	"sync"
 	"time"
 
 	"github.com/golang/protobuf/proto"
@@ -36,6 +37,18 @@ type identity struct {
 
 	// reference to the MSP that "owns" this identity
 	msp *bccspmsp
+
+	// validationMutex is used to synchronise memory operation
+	// over validated and validationErr
+	validationMutex sync.Mutex
+
+	// validated is true when the validateIdentity function
+	// has been called on this instance
+	validated bool
+
+	// validationErr contains the validation error for this
+	// instance. It can be read if validated is true
+	validationErr error
 }
 
 func newIdentity(cert *x509.Certificate, pk bccsp.Key, msp *bccspmsp) (Identity, error) {
@@ -214,7 +227,15 @@ func newSigningIdentity(cert *x509.Certificate, pk bccsp.Key, signer crypto.Sign
 	if err != nil {
 		return nil, err
 	}
-	return &signingidentity{identity: *mspId.(*identity), signer: signer}, nil
+	return &signingidentity{
+		identity: identity{
+			id:   mspId.(*identity).id,
+			cert: mspId.(*identity).cert,
+			msp:  mspId.(*identity).msp,
+			pk:   mspId.(*identity).pk,
+		},
+		signer: signer,
+	}, nil
 }
 
 // Sign produces a signature over msg, signed by this instance

diff --git a/msp/msp_test.go b/msp/msp_test.go
@@ -1027,23 +1027,33 @@ func TestMSPOus(t *testing.T) {
 	// Set the OUIdentifiers
 	backup := localMsp.(*bccspmsp).ouIdentifiers
 	defer func() { localMsp.(*bccspmsp).ouIdentifiers = backup }()
-	id, err := localMsp.GetDefaultSigningIdentity()
+	sid, err := localMsp.GetDefaultSigningIdentity()
+	assert.NoError(t, err)
+	sidBytes, err := sid.Serialize()
+	assert.NoError(t, err)
+	id, err := localMsp.DeserializeIdentity(sidBytes)
 	assert.NoError(t, err)
 
 	localMsp.(*bccspmsp).ouIdentifiers = map[string][][]byte{
 		"COP": {id.GetOrganizationalUnits()[0].CertifiersIdentifier},
 	}
-	assert.NoError(t, localMsp.Validate(id.GetPublicVersion()))
+	assert.NoError(t, localMsp.Validate(id))
+
+	id, err = localMsp.DeserializeIdentity(sidBytes)
+	assert.NoError(t, err)
 
 	localMsp.(*bccspmsp).ouIdentifiers = map[string][][]byte{
 		"COP2": {id.GetOrganizationalUnits()[0].CertifiersIdentifier},
 	}
-	assert.Error(t, localMsp.Validate(id.GetPublicVersion()))
+	assert.Error(t, localMsp.Validate(id))
+
+	id, err = localMsp.DeserializeIdentity(sidBytes)
+	assert.NoError(t, err)
 
 	localMsp.(*bccspmsp).ouIdentifiers = map[string][][]byte{
 		"COP": {{0, 1, 2, 3, 4}},
 	}
-	assert.Error(t, localMsp.Validate(id.GetPublicVersion()))
+	assert.Error(t, localMsp.Validate(id))
 }
 
 const othercert = `-----BEGIN CERTIFICATE-----

diff --git a/msp/mspimplvalidate.go b/msp/mspimplvalidate.go
@@ -19,19 +19,32 @@ import (
 )
 
 func (msp *bccspmsp) validateIdentity(id *identity) error {
+	id.validationMutex.Lock()
+	defer id.validationMutex.Unlock()
+
+	// return cached validation value if already validated
+	if id.validated {
+		return id.validationErr
+	}
+
+	id.validated = true
+
 	validationChain, err := msp.getCertificationChainForBCCSPIdentity(id)
 	if err != nil {
-		return errors.WithMessage(err, "could not obtain certification chain")
+		id.validationErr = errors.WithMessage(err, "could not obtain certification chain")
+		return id.validationErr
 	}
 
 	err = msp.validateIdentityAgainstChain(id, validationChain)
 	if err != nil {
-		return errors.WithMessage(err, "could not validate identity against certification chain")
+		id.validationErr = errors.WithMessage(err, "could not validate identity against certification chain")
+		return id.validationErr
 	}
 
 	err = msp.internalValidateIdentityOusFunc(id)
 	if err != nil {
-		return errors.WithMessage(err, "could not validate identity's OUs")
+		id.validationErr = errors.WithMessage(err, "could not validate identity's OUs")
+		return id.validationErr
 	}
 
 	return nil

diff --git a/msp/mspwithintermediatecas_test.go b/msp/mspwithintermediatecas_test.go
@@ -33,23 +33,33 @@ func TestMSPWithIntermediateCAs(t *testing.T) {
 
 	// This MSP will trust any cert signed by the CA directly OR by the intermediate
 
-	id, err := thisMSP.GetDefaultSigningIdentity()
+	sid, err := thisMSP.GetDefaultSigningIdentity()
+	assert.NoError(t, err)
+	sidBytes, err := sid.Serialize()
+	assert.NoError(t, err)
+	id, err := thisMSP.DeserializeIdentity(sidBytes)
 	assert.NoError(t, err)
 
 	// ensure that we validate correctly the identity
-	err = thisMSP.Validate(id.GetPublicVersion())
+	err = thisMSP.Validate(id)
+	assert.NoError(t, err)
+
+	id, err = thisMSP.DeserializeIdentity(sidBytes)
 	assert.NoError(t, err)
 
 	// ensure that validation of an identity of the MSP with intermediate CAs
 	// fails with the local MSP
-	err = localMsp.Validate(id.GetPublicVersion())
+	err = localMsp.Validate(id)
 	assert.Error(t, err)
 
+	id, err = thisMSP.DeserializeIdentity(sidBytes)
+	assert.NoError(t, err)
+
 	// ensure that validation of an identity of the local MSP
 	// fails with the MSP with intermediate CAs
 	localMSPID, err := localMsp.GetDefaultSigningIdentity()
 	assert.NoError(t, err)
-	err = thisMSP.Validate(localMSPID.GetPublicVersion())
+	err = thisMSP.Validate(localMSPID)
 	assert.Error(t, err)
 }