[FAB-6294] Fix stale reference to policy manager

The policy manager used to be stateful, and it was safe to retain a
reference to it.  However, this added considerable complexity and
introduced the possibility for subtle races so it was turned into an
immutable structure.

The orderer signature filter was not updated to pull a fresh reference
on each invocation, but instead still retains only a reference to the
original policy manager.  This CR updates the signature filter to fetch
a fresh reference upon each invocation.

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

Author: Jason Yellick

orderer/common/deliver/deliver.go

@@ -138,7 +138,7 @@ func (ds *deliverServer) deliverBlocks(srv ab.AtomicBroadcast_DeliverServer, env
 
 	lastConfigSequence := chain.Sequence()
 
-	sf := msgprocessor.NewSigFilter(policies.ChannelReaders, chain.PolicyManager())
+	sf := msgprocessor.NewSigFilter(policies.ChannelReaders, chain)
 	if err := sf.Apply(envelope); err != nil {
 		logger.Warningf("[channel: %s] Received unauthorized deliver request from %s: %s", chdr.ChannelId, addr, err)
 		return sendStatusReply(srv, cb.Status_FORBIDDEN)

orderer/common/msgprocessor/sigfilter.go

@@ -15,17 +15,23 @@ import (
 	"github.com/pkg/errors"
 )
 
+// SigFilterSupport provides the resources required for the signature filter
+type SigFilterSupport interface {
+	// PolicyManager returns a reference to the current policy manager
+	PolicyManager() policies.Manager
+}
+
 type sigFilter struct {
-	policyName    string
-	policyManager policies.Manager
+	policyName string
+	support    SigFilterSupport
 }
 
 // NewSigFilter creates a new signature filter, at every evaluation, the policy manager is called
 // to retrieve the latest version of the policy
-func NewSigFilter(policyName string, policyManager policies.Manager) Rule {
+func NewSigFilter(policyName string, support SigFilterSupport) Rule {
 	return &sigFilter{
-		policyName:    policyName,
-		policyManager: policyManager,
+		policyName: policyName,
+		support:    support,
 	}
 }
 
@@ -37,7 +43,7 @@ func (sf *sigFilter) Apply(message *cb.Envelope) error {
 		return fmt.Errorf("could not convert message to signedData: %s", err)
 	}
 
-	policy, ok := sf.policyManager.GetPolicy(sf.policyName)
+	policy, ok := sf.support.PolicyManager().GetPolicy(sf.policyName)
 	if !ok {
 		return fmt.Errorf("could not find policy %s", sf.policyName)
 	}

orderer/common/msgprocessor/sigfilter_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	"github.com/hyperledger/fabric/common/flogging"
+	mockchannelconfig "github.com/hyperledger/fabric/common/mocks/config"
 	mockpolicies "github.com/hyperledger/fabric/common/mocks/policies"
 	cb "github.com/hyperledger/fabric/protos/common"
 	"github.com/hyperledger/fabric/protos/utils"
@@ -34,26 +35,34 @@ func makeEnvelope() *cb.Envelope {
 }
 
 func TestAccept(t *testing.T) {
-	mpm := &mockpolicies.Manager{Policy: &mockpolicies.Policy{}}
+	mpm := &mockchannelconfig.Resources{
+		PolicyManagerVal: &mockpolicies.Manager{Policy: &mockpolicies.Policy{}},
+	}
 	assert.Nil(t, NewSigFilter("foo", mpm).Apply(makeEnvelope()), "Valid envelope and good policy")
 }
 
 func TestMissingPolicy(t *testing.T) {
-	mpm := &mockpolicies.Manager{}
+	mpm := &mockchannelconfig.Resources{
+		PolicyManagerVal: &mockpolicies.Manager{},
+	}
 	err := NewSigFilter("foo", mpm).Apply(makeEnvelope())
 	assert.NotNil(t, err)
 	assert.Regexp(t, "could not find policy", err.Error())
 }
 
 func TestEmptyPayload(t *testing.T) {
-	mpm := &mockpolicies.Manager{Policy: &mockpolicies.Policy{}}
+	mpm := &mockchannelconfig.Resources{
+		PolicyManagerVal: &mockpolicies.Manager{Policy: &mockpolicies.Policy{}},
+	}
 	err := NewSigFilter("foo", mpm).Apply(&cb.Envelope{})
 	assert.NotNil(t, err)
 	assert.Regexp(t, "could not convert message to signedData", err.Error())
 }
 
 func TestErrorOnPolicy(t *testing.T) {
-	mpm := &mockpolicies.Manager{Policy: &mockpolicies.Policy{Err: fmt.Errorf("Error")}}
+	mpm := &mockchannelconfig.Resources{
+		PolicyManagerVal: &mockpolicies.Manager{Policy: &mockpolicies.Policy{Err: fmt.Errorf("Error")}},
+	}
 	err := NewSigFilter("foo", mpm).Apply(makeEnvelope())
 	assert.NotNil(t, err)
 	assert.Equal(t, ErrPermissionDenied, errors.Cause(err))

orderer/common/msgprocessor/standardchannel.go

@@ -53,7 +53,7 @@ func CreateStandardChannelFilters(filterSupport channelconfig.Resources) *RuleSe
 	return NewRuleSet([]Rule{
 		EmptyRejectRule,
 		NewSizeFilter(ordererConfig),
-		NewSigFilter(policies.ChannelWriters, filterSupport.PolicyManager()),
+		NewSigFilter(policies.ChannelWriters, filterSupport),
 	})
 }
 

orderer/common/msgprocessor/systemchannel.go

@@ -50,7 +50,7 @@ func CreateSystemChannelFilters(chainCreator ChainCreator, ledgerResources chann
 	return NewRuleSet([]Rule{
 		EmptyRejectRule,
 		NewSizeFilter(ordererConfig),
-		NewSigFilter(policies.ChannelWriters, ledgerResources.PolicyManager()),
+		NewSigFilter(policies.ChannelWriters, ledgerResources),
 		NewSystemChannelFilter(ledgerResources, chainCreator),
 	})
 }