Change signature of internal evaluator function

Towards a refactoring of the validation/committer, we need to decouple
signature/identity validation and policy checking. This change-set
enables this by changing the signature of the function that internally
evaluates the policy: instead of directly invoking functions to
deserialize the supplied identity, checking compliance of the identity
with an MSP principal and verifying a signature, it calls functions
of a suitable interface. Implementers of the interface are then free
to perform any appropriate precomputation.

FAB-12774 #done

Change-Id: I9df76f801a6b26f303070899ee50f0b699f06e36
Signed-off-by: Alessandro Sorniotti <ale.linux@sopit.net>

Author: ale-linux

common/cauthdsl/cauthdsl.go

@@ -20,19 +20,19 @@ import (
 var cauthdslLogger = flogging.MustGetLogger("cauthdsl")
 
 // deduplicate removes any duplicated identities while otherwise preserving identity order
-func deduplicate(sds []*cb.SignedData, deserializer msp.IdentityDeserializer) []*cb.SignedData {
+func deduplicate(sds []IdentityAndSignature) []IdentityAndSignature {
 	ids := make(map[string]struct{})
-	result := make([]*cb.SignedData, 0, len(sds))
+	result := make([]IdentityAndSignature, 0, len(sds))
 	for i, sd := range sds {
-		identity, err := deserializer.DeserializeIdentity(sd.Identity)
+		identity, err := sd.Identity()
 		if err != nil {
-			cauthdslLogger.Errorf("Principal deserialization failure (%s) for identity %x", err, sd.Identity)
+			cauthdslLogger.Errorf("Principal deserialization failure (%s) for identity %d", err, i)
 			continue
 		}
 		key := identity.GetIdentifier().Mspid + identity.GetIdentifier().Id
 
 		if _, ok := ids[key]; ok {
-			cauthdslLogger.Warningf("De-duplicating identity %x at index %d in signature set", sd.Identity, i)
+			cauthdslLogger.Warningf("De-duplicating identity [%s] at index %d in signature set", key, i)
 		} else {
 			result = append(result, sd)
 			ids[key] = struct{}{}
@@ -43,14 +43,14 @@ func deduplicate(sds []*cb.SignedData, deserializer msp.IdentityDeserializer) []
 
 // compile recursively builds a go evaluatable function corresponding to the policy specified, remember to call deduplicate on identities before
 // passing them to this function for evaluation
-func compile(policy *cb.SignaturePolicy, identities []*mb.MSPPrincipal, deserializer msp.IdentityDeserializer) (func([]*cb.SignedData, []bool) bool, error) {
+func compile(policy *cb.SignaturePolicy, identities []*mb.MSPPrincipal, deserializer msp.IdentityDeserializer) (func([]IdentityAndSignature, []bool) bool, error) {
 	if policy == nil {
 		return nil, fmt.Errorf("Empty policy element")
 	}
 
 	switch t := policy.Type.(type) {
 	case *cb.SignaturePolicy_NOutOf_:
-		policies := make([]func([]*cb.SignedData, []bool) bool, len(t.NOutOf.Rules))
+		policies := make([]func([]IdentityAndSignature, []bool) bool, len(t.NOutOf.Rules))
 		for i, policy := range t.NOutOf.Rules {
 			compiledPolicy, err := compile(policy, identities, deserializer)
 			if err != nil {
@@ -59,7 +59,7 @@ func compile(policy *cb.SignaturePolicy, identities []*mb.MSPPrincipal, deserial
 			policies[i] = compiledPolicy
 
 		}
-		return func(signedData []*cb.SignedData, used []bool) bool {
+		return func(signedData []IdentityAndSignature, used []bool) bool {
 			grepKey := time.Now().UnixNano()
 			cauthdslLogger.Debugf("%p gate %d evaluation starts", signedData, grepKey)
 			verified := int32(0)
@@ -85,7 +85,7 @@ func compile(policy *cb.SignaturePolicy, identities []*mb.MSPPrincipal, deserial
 			return nil, fmt.Errorf("identity index out of range, requested %v, but identies length is %d", t.SignedBy, len(identities))
 		}
 		signedByID := identities[t.SignedBy]
-		return func(signedData []*cb.SignedData, used []bool) bool {
+		return func(signedData []IdentityAndSignature, used []bool) bool {
 			cauthdslLogger.Debugf("%p signed by %d principal evaluation starts (used %v)", signedData, t.SignedBy, used)
 			for i, sd := range signedData {
 				if used[i] {
@@ -96,9 +96,9 @@ func compile(policy *cb.SignaturePolicy, identities []*mb.MSPPrincipal, deserial
 					// Unlike most places, this is a huge print statement, and worth checking log level before create garbage
 					cauthdslLogger.Debugf("%p processing identity %d with bytes of %x", signedData, i, sd.Identity)
 				}
-				identity, err := deserializer.DeserializeIdentity(sd.Identity)
+				identity, err := sd.Identity()
 				if err != nil {
-					cauthdslLogger.Errorf("Principal deserialization failure (%s) for identity %x", err, sd.Identity)
+					cauthdslLogger.Errorf("Principal deserialization failure (%s) for identity %d", err, i)
 					continue
 				}
 				err = identity.SatisfiesPrincipal(signedByID)
@@ -107,7 +107,7 @@ func compile(policy *cb.SignaturePolicy, identities []*mb.MSPPrincipal, deserial
 					continue
 				}
 				cauthdslLogger.Debugf("%p principal matched by identity %d", signedData, i)
-				err = identity.Verify(sd.Data, sd.Signature)
+				err = sd.Verify()
 				if err != nil {
 					cauthdslLogger.Debugf("%p signature for identity %d is invalid: %s", signedData, i, err)
 					continue

common/cauthdsl/cauthdsl_test.go

@@ -71,13 +71,16 @@ func (id *mockIdentity) Serialize() ([]byte, error) {
 	return id.idBytes, nil
 }
 
-func toSignedData(data [][]byte, identities [][]byte, signatures [][]byte) ([]*cb.SignedData, []bool) {
-	signedData := make([]*cb.SignedData, len(data))
+func toSignedData(data [][]byte, identities [][]byte, signatures [][]byte, deserializer msp.IdentityDeserializer) ([]IdentityAndSignature, []bool) {
+	signedData := make([]IdentityAndSignature, len(data))
 	for i := range signedData {
-		signedData[i] = &cb.SignedData{
-			Data:      data[i],
-			Identity:  identities[i],
-			Signature: signatures[i],
+		signedData[i] = &deserializeAndVerify{
+			signedData: &cb.SignedData{
+				Data:      data[i],
+				Identity:  identities[i],
+				Signature: signatures[i],
+			},
+			deserializer: deserializer,
 		}
 	}
 	return signedData, make([]bool, len(signedData))
@@ -111,13 +114,13 @@ func TestSimpleSignature(t *testing.T) {
 		t.Fatalf("Could not create a new SignaturePolicyEvaluator using the given policy, crypto-helper: %s", err)
 	}
 
-	if !spe(toSignedData([][]byte{nil}, [][]byte{signers[0]}, [][]byte{validSignature})) {
+	if !spe(toSignedData([][]byte{nil}, [][]byte{signers[0]}, [][]byte{validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to succeed with valid signatures")
 	}
-	if spe(toSignedData([][]byte{nil}, [][]byte{signers[0]}, [][]byte{invalidSignature})) {
+	if spe(toSignedData([][]byte{nil}, [][]byte{signers[0]}, [][]byte{invalidSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to fail given the invalid signature")
 	}
-	if spe(toSignedData([][]byte{nil}, [][]byte{signers[1]}, [][]byte{validSignature})) {
+	if spe(toSignedData([][]byte{nil}, [][]byte{signers[1]}, [][]byte{validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to fail because signers[1] is not authorized in the policy, despite his valid signature")
 	}
 }
@@ -130,13 +133,13 @@ func TestMultipleSignature(t *testing.T) {
 		t.Fatalf("Could not create a new SignaturePolicyEvaluator using the given policy, crypto-helper: %s", err)
 	}
 
-	if !spe(toSignedData(msgs, signers, [][]byte{validSignature, validSignature})) {
+	if !spe(toSignedData(msgs, signers, [][]byte{validSignature, validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to succeed with  valid signatures")
 	}
-	if spe(toSignedData(msgs, signers, [][]byte{validSignature, invalidSignature})) {
+	if spe(toSignedData(msgs, signers, [][]byte{validSignature, invalidSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to fail given one of two invalid signatures")
 	}
-	if spe(toSignedData(msgs, [][]byte{signers[0], signers[0]}, [][]byte{validSignature, validSignature})) {
+	if spe(toSignedData(msgs, [][]byte{signers[0], signers[0]}, [][]byte{validSignature, validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to fail because although there were two valid signatures, one was duplicated")
 	}
 }
@@ -149,19 +152,19 @@ func TestComplexNestedSignature(t *testing.T) {
 		t.Fatalf("Could not create a new SignaturePolicyEvaluator using the given policy, crypto-helper: %s", err)
 	}
 
-	if !spe(toSignedData(moreMsgs, append(signers, [][]byte{[]byte("signer0")}...), [][]byte{validSignature, validSignature, validSignature})) {
+	if !spe(toSignedData(moreMsgs, append(signers, [][]byte{[]byte("signer0")}...), [][]byte{validSignature, validSignature, validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to succeed with valid signatures")
 	}
-	if !spe(toSignedData(moreMsgs, [][]byte{[]byte("signer0"), []byte("signer0"), []byte("signer0")}, [][]byte{validSignature, validSignature, validSignature})) {
+	if !spe(toSignedData(moreMsgs, [][]byte{[]byte("signer0"), []byte("signer0"), []byte("signer0")}, [][]byte{validSignature, validSignature, validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to succeed with valid signatures")
 	}
-	if spe(toSignedData(msgs, signers, [][]byte{validSignature, validSignature})) {
+	if spe(toSignedData(msgs, signers, [][]byte{validSignature, validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication to fail with too few signatures")
 	}
-	if spe(toSignedData(moreMsgs, append(signers, [][]byte{[]byte("signer0")}...), [][]byte{validSignature, invalidSignature, validSignature})) {
+	if spe(toSignedData(moreMsgs, append(signers, [][]byte{[]byte("signer0")}...), [][]byte{validSignature, invalidSignature, validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication failure as the signature of signer[1] was invalid")
 	}
-	if spe(toSignedData(moreMsgs, append(signers, [][]byte{[]byte("signer1")}...), [][]byte{validSignature, validSignature, validSignature})) {
+	if spe(toSignedData(moreMsgs, append(signers, [][]byte{[]byte("signer1")}...), [][]byte{validSignature, validSignature, validSignature}, &mockDeserializer{})) {
 		t.Errorf("Expected authentication failure as there was a signature from signer[0] missing")
 	}
 }
@@ -184,46 +187,109 @@ func TestNilSignaturePolicyEnvelope(t *testing.T) {
 }
 
 func TestDeduplicate(t *testing.T) {
-	ids := []*cb.SignedData{
-		{
-			Identity: []byte("id1"),
-		},
-		{
-			Identity: []byte("id2"),
-		},
-		{
-			Identity: []byte("id3"),
-		},
-	}
-
 	t.Run("Empty", func(t *testing.T) {
-		result := deduplicate([]*cb.SignedData{}, &mockDeserializer{})
-		assert.Equal(t, []*cb.SignedData{}, result, "Should have no identities")
+		result := deduplicate([]IdentityAndSignature{})
+		assert.Equal(t, []IdentityAndSignature{}, result, "Should have no identities")
 	})
 
 	t.Run("NoDuplication", func(t *testing.T) {
-		result := deduplicate(ids, &mockDeserializer{})
+		md := &mockDeserializer{}
+		ids := []IdentityAndSignature{
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id1"),
+				},
+				deserializer: md,
+			},
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id2"),
+				},
+				deserializer: md,
+			},
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id3"),
+				},
+				deserializer: md,
+			},
+		}
+		result := deduplicate(ids)
 		assert.Equal(t, ids, result, "No identities should have been removed")
 	})
 
 	t.Run("AllDuplication", func(t *testing.T) {
-		result := deduplicate([]*cb.SignedData{ids[0], ids[0], ids[0]}, &mockDeserializer{})
-		assert.Equal(t, []*cb.SignedData{ids[0]}, result, "All but the first identity should have been removed")
+		md := &mockDeserializer{}
+		ids := []IdentityAndSignature{
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id1"),
+				},
+				deserializer: md,
+			},
+		}
+		result := deduplicate([]IdentityAndSignature{ids[0], ids[0], ids[0]})
+		assert.Equal(t, []IdentityAndSignature{ids[0]}, result, "All but the first identity should have been removed")
 	})
 
 	t.Run("DuplicationPreservesOrder", func(t *testing.T) {
-		result := deduplicate([]*cb.SignedData{ids[1], ids[0], ids[0]}, &mockDeserializer{})
-		assert.Equal(t, []*cb.SignedData{ids[1], ids[0]}, result, "The third identity should have been dropped")
+		md := &mockDeserializer{}
+		ids := []IdentityAndSignature{
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id1"),
+				},
+				deserializer: md,
+			},
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id2"),
+				},
+				deserializer: md,
+			},
+		}
+		result := deduplicate([]IdentityAndSignature{ids[1], ids[0], ids[0]})
+		assert.Equal(t, result, []IdentityAndSignature{ids[1], ids[0]}, "The third identity should have been dropped")
 	})
 
 	t.Run("ComplexDuplication", func(t *testing.T) {
-		result := deduplicate([]*cb.SignedData{ids[1], ids[0], ids[0], ids[1], ids[2], ids[0], ids[2], ids[1]}, &mockDeserializer{})
-		assert.Equal(t, []*cb.SignedData{ids[1], ids[0], ids[2]}, result, "Expected only three non-duplicate identities")
+		md := &mockDeserializer{}
+		ids := []IdentityAndSignature{
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id1"),
+				},
+				deserializer: md,
+			},
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id2"),
+				},
+				deserializer: md,
+			},
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id3"),
+				},
+				deserializer: md,
+			},
+		}
+		result := deduplicate([]IdentityAndSignature{ids[1], ids[0], ids[0], ids[1], ids[2], ids[0], ids[2], ids[1]})
+		assert.Equal(t, []IdentityAndSignature{ids[1], ids[0], ids[2]}, result, "Expected only three non-duplicate identities")
 	})
 
 	t.Run("BadIdentity", func(t *testing.T) {
-		result := deduplicate([]*cb.SignedData{ids[1]}, &mockDeserializer{fail: errors.New("error")})
-		assert.Equal(t, []*cb.SignedData{}, result, "No valid identities")
+		md := &mockDeserializer{fail: errors.New("error")}
+		ids := []IdentityAndSignature{
+			&deserializeAndVerify{
+				signedData: &cb.SignedData{
+					Identity: []byte("id1"),
+				},
+				deserializer: md,
+			},
+		}
+		result := deduplicate([]IdentityAndSignature{ids[0]})
+		assert.Equal(t, []IdentityAndSignature{}, result, "No valid identities")
 	})
 }
 
@@ -292,7 +358,7 @@ func TestDeserializeIdentityError(t *testing.T) {
 	cauthdslLogger = logger
 
 	// Call
-	signedData, used := toSignedData([][]byte{nil}, [][]byte{nil}, [][]byte{nil})
+	signedData, used := toSignedData([][]byte{nil}, [][]byte{nil}, [][]byte{nil}, &mockDeserializer{fail: errors.New("myError")})
 	ret := spe(signedData, used)
 
 	// Check result (ret and log)

common/cauthdsl/policy.go

@@ -14,8 +14,51 @@ import (
 	"github.com/hyperledger/fabric/common/policies"
 	"github.com/hyperledger/fabric/msp"
 	cb "github.com/hyperledger/fabric/protos/common"
+	mspp "github.com/hyperledger/fabric/protos/msp"
 )
 
+type Identity interface {
+	// SatisfiesPrincipal checks whether this instance matches
+	// the description supplied in MSPPrincipal. The check may
+	// involve a byte-by-byte comparison (if the principal is
+	// a serialized identity) or may require MSP validation
+	SatisfiesPrincipal(principal *mspp.MSPPrincipal) error
+
+	// GetIdentifier returns the identifier of that identity
+	GetIdentifier() *msp.IdentityIdentifier
+}
+
+type IdentityAndSignature interface {
+	// Identity returns the identity associated to this instance
+	Identity() (Identity, error)
+
+	// Verify returns the validity status of this identity's signature over the message
+	Verify() error
+}
+
+type deserializeAndVerify struct {
+	signedData           *cb.SignedData
+	deserializer         msp.IdentityDeserializer
+	deserializedIdentity msp.Identity
+}
+
+func (d *deserializeAndVerify) Identity() (Identity, error) {
+	deserializedIdentity, err := d.deserializer.DeserializeIdentity(d.signedData.Identity)
+	if err != nil {
+		return nil, err
+	}
+
+	d.deserializedIdentity = deserializedIdentity
+	return deserializedIdentity, nil
+}
+
+func (d *deserializeAndVerify) Verify() error {
+	if d.deserializedIdentity == nil {
+		cauthdslLogger.Panicf("programming error, Identity must be called prior to Verify")
+	}
+	return d.deserializedIdentity.Verify(d.signedData.Data, d.signedData.Signature)
+}
+
 type provider struct {
 	deserializer msp.IdentityDeserializer
 }
@@ -51,7 +94,7 @@ func (pr *provider) NewPolicy(data []byte) (policies.Policy, proto.Message, erro
 }
 
 type policy struct {
-	evaluator    func([]*cb.SignedData, []bool) bool
+	evaluator    func([]IdentityAndSignature, []bool) bool
 	deserializer msp.IdentityDeserializer
 }
 
@@ -60,8 +103,15 @@ func (p *policy) Evaluate(signatureSet []*cb.SignedData) error {
 	if p == nil {
 		return fmt.Errorf("No such policy")
 	}
+	idAndS := make([]IdentityAndSignature, len(signatureSet))
+	for i, sd := range signatureSet {
+		idAndS[i] = &deserializeAndVerify{
+			signedData:   sd,
+			deserializer: p.deserializer,
+		}
+	}
 
-	ok := p.evaluator(deduplicate(signatureSet, p.deserializer), make([]bool, len(signatureSet)))
+	ok := p.evaluator(deduplicate(idAndS), make([]bool, len(signatureSet)))
 	if !ok {
 		return errors.New("signature set did not satisfy policy")
 	}