[FAB-5753] Deduplicate identities in cauthdsl

Backport from v1.1 for v1.0.2.

The cauthdsl policy evaluation by spec deduplicates the identities prior
to evaluating the policy.  However, the deduplication portion of the
spec was never implemented.

Some hacky deduplication was added in the VSCC path for some reason, but
this is the right place to fix it.

Although this CR does mean that it is possible the results of policy
evaluation between versions will be different, the actual exposure to
non-determinism is quite low, as the only typical place where multiple
signatures are allowed is in endorsement which is already covered by the
VSCC hack.

There is some minor exposure because the config processing allows
multiple signatures, however, none of the default policies repeat the
same principal, so unless the config has been customized with odd
policies, this should also be a non-issue.

Change-Id: Id2026475767549b4eaa4bcd1c5cc39f70bcf5b6b
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>

common/cauthdsl/cauthdsl.go

@@ -1,17 +1,7 @@
 /*
-Copyright IBM Corp. 2016 All Rights Reserved.
+Copyright IBM Corp. All Rights Reserved.
 
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-                 http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
+SPDX-License-Identifier: Apache-2.0
 */
 
 package cauthdsl
@@ -30,7 +20,23 @@ import (
 
 var cauthdslLogger = flogging.MustGetLogger("cauthdsl")
 
-// compile recursively builds a go evaluatable function corresponding to the policy specified
+// deduplicate removes any duplicated identities while otherwise preserving identity order
+func deduplicate(sds []*cb.SignedData) []*cb.SignedData {
+	ids := make(map[string]struct{})
+	result := make([]*cb.SignedData, 0, len(sds))
+	for i, sd := range sds {
+		if _, ok := ids[string(sd.Identity)]; ok {
+			cauthdslLogger.Warningf("De-duplicating identity %x at index %d in signature set", sd.Identity, i)
+		} else {
+			result = append(result, sd)
+			ids[string(sd.Identity)] = struct{}{}
+		}
+	}
+	return result
+}
+
+// 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) {
 	if policy == nil {
 		return nil, fmt.Errorf("Empty policy element")

common/cauthdsl/cauthdsl_test.go

@@ -1,17 +1,7 @@
 /*
-Copyright IBM Corp. 2016 All Rights Reserved.
+Copyright IBM Corp. All Rights Reserved.
 
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-                 http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
+SPDX-License-Identifier: Apache-2.0
 */
 
 package cauthdsl
@@ -179,3 +169,42 @@ func TestNilSignaturePolicyEnvelope(t *testing.T) {
 	_, err := compile(nil, nil, &mockDeserializer{})
 	assert.Error(t, err, "Fail to compile")
 }
+
+func TestDeduplicate(t *testing.T) {
+	ids := []*cb.SignedData{
+		&cb.SignedData{
+			Identity: []byte("id1"),
+		},
+		&cb.SignedData{
+			Identity: []byte("id2"),
+		},
+		&cb.SignedData{
+			Identity: []byte("id3"),
+		},
+	}
+
+	t.Run("Empty", func(t *testing.T) {
+		result := deduplicate([]*cb.SignedData{})
+		assert.Equal(t, []*cb.SignedData{}, result, "Should have no identities")
+	})
+
+	t.Run("NoDuplication", func(t *testing.T) {
+		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]})
+		assert.Equal(t, []*cb.SignedData{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]})
+		assert.Equal(t, []*cb.SignedData{ids[1], ids[0]}, result, "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]})
+		assert.Equal(t, []*cb.SignedData{ids[1], ids[0], ids[2]}, result, "Expected only three non-duplicate identities")
+	})
+}

common/cauthdsl/policy.go

@@ -1,17 +1,7 @@
 /*
-Copyright IBM Corp. 2016 All Rights Reserved.
+Copyright IBM Corp. All Rights Reserved.
 
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-                 http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
+SPDX-License-Identifier: Apache-2.0
 */
 
 package cauthdsl
@@ -70,7 +60,7 @@ func (p *policy) Evaluate(signatureSet []*cb.SignedData) error {
 		return fmt.Errorf("No such policy")
 	}
 
-	ok := p.evaluator(signatureSet, make([]bool, len(signatureSet)))
+	ok := p.evaluator(deduplicate(signatureSet), make([]bool, len(signatureSet)))
 	if !ok {
 		return errors.New("Failed to authenticate policy")
 	}