Skip to content

Commit

Permalink
[FAB-2643] MsgCryptoSvc: NPE when no policyMgr found
Browse files Browse the repository at this point in the history
Scenario:
Peer gets an AliveMessage in gossip from an external organization,
and the messageCryptoService tries to validate it.
The validation first goes to the local MSP (and fails) so it tries
validating using one of the channel policies
(which were recently introduced), but it fails because
the channelPolicyManagerGetter returns a nil policyManager
from some reason.
This results in a null dereference panic and the unexpected and
sorrowful death of the peer.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x756ff5]

goroutine 45 [running]:
panic(0xc1ad00, 0xc420016050)
/opt/go/src/runtime/panic.go:500 +0x1a1
fabric/peer/gossip/mcs.(*mspMessageCryptoService).VerifyByChannel
/opt/gopath/src/github.com/hyperledger/fabric/peer/gossip/mcs/mcs.go:234
github.com/hyperledger/fabric/peer/gossip/mcs.(*mspMessageCryptoService).Verify
...
Full stacktrace available in the JIRA item.

I added a nil check in 2 places and also added tests that check these
corner cases.

Change-Id: I3f45aff6cbf483258fcb4166f059e03647b947c8
Signed-off-by: Yacov Manevich <yacovm@il.ibm.com>
  • Loading branch information
yacovm committed Mar 5, 2017
1 parent ed7ed80 commit a5f2ba0
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
9 changes: 7 additions & 2 deletions peer/gossip/mcs/mcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@ limitations under the License.
package mcs

import (
"bytes"
"errors"
"fmt"

"bytes"

"github.com/hyperledger/fabric/bccsp"
"github.com/hyperledger/fabric/bccsp/factory"
"github.com/hyperledger/fabric/common/crypto"
Expand Down Expand Up @@ -156,6 +155,9 @@ func (s *mspMessageCryptoService) VerifyBlock(chainID common.ChainID, signedBloc

// Get the policy manager for channelID
cpm, ok := s.channelPolicyManagerGetter.Manager(channelID)
if cpm == nil {
return fmt.Errorf("Could not acquire policy manager for channel %s", channelID)
}
// ok is true if it was the manager requested, or false if it is the default manager
logger.Debugf("Got policy manager for channel [%s] with flag [%s]", channelID, ok)

Expand Down Expand Up @@ -228,6 +230,9 @@ func (s *mspMessageCryptoService) VerifyByChannel(chainID common.ChainID, peerId

// Get the policy manager for channel chainID
cpm, flag := s.channelPolicyManagerGetter.Manager(string(chainID))
if cpm == nil {
return fmt.Errorf("Could not acquire policy manager for channel %s", string(chainID))
}
logger.Debugf("Got policy manager for channel [%s] with flag [%s]", string(chainID), flag)

// Get channel reader policy
Expand Down
22 changes: 18 additions & 4 deletions peer/gossip/mcs/mcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,15 @@ limitations under the License.
package mcs

import (
"fmt"
"testing"

"github.com/golang/protobuf/proto"
"github.com/hyperledger/fabric/bccsp"
"github.com/hyperledger/fabric/bccsp/factory"

mockscrypto "github.com/hyperledger/fabric/common/mocks/crypto"

"github.com/golang/protobuf/proto"
"github.com/hyperledger/fabric/common/crypto"
"github.com/hyperledger/fabric/common/localmsp"
mockscrypto "github.com/hyperledger/fabric/common/mocks/crypto"
"github.com/hyperledger/fabric/common/policies"
"github.com/hyperledger/fabric/common/util"
"github.com/hyperledger/fabric/gossip/api"
Expand Down Expand Up @@ -79,6 +78,7 @@ func TestVerify(t *testing.T) {
map[string]policies.Manager{
"A": &mockChannelPolicyManager{&mockPolicy{&mockIdentityDeserializer{[]byte("Bob"), []byte("msg2")}}},
"B": &mockChannelPolicyManager{&mockPolicy{&mockIdentityDeserializer{[]byte("Charlie"), []byte("msg3")}}},
"C": nil,
},
},
&mockscrypto.LocalSigner{Identity: []byte("Alice")},
Expand All @@ -87,6 +87,7 @@ func TestVerify(t *testing.T) {
channelDeserializers: map[string]msp.IdentityDeserializer{
"A": &mockIdentityDeserializer{[]byte("Bob"), []byte("msg2")},
"B": &mockIdentityDeserializer{[]byte("Charlie"), []byte("msg3")},
"C": &mockIdentityDeserializer{[]byte("Yacov"), []byte("msg4")},
},
},
)
Expand All @@ -103,6 +104,12 @@ func TestVerify(t *testing.T) {

err = msgCryptoService.Verify(api.PeerIdentityType("Charlie"), sigma, msg)
assert.Error(t, err, "Charlie should not verify the signature")

sigma, err = msgCryptoService.Sign(msg)
assert.NoError(t, err)
err = msgCryptoService.Verify(api.PeerIdentityType("Yacov"), sigma, msg)
assert.Error(t, err)
assert.Contains(t, fmt.Sprintf("%v", err), "Could not acquire policy manager")
}

func TestVerifyBlock(t *testing.T) {
Expand All @@ -112,6 +119,7 @@ func TestVerifyBlock(t *testing.T) {
"A": &mockChannelPolicyManager{&mockPolicy{&mockIdentityDeserializer{[]byte("Bob"), []byte("msg2")}}},
"B": &mockChannelPolicyManager{&mockPolicy{&mockIdentityDeserializer{[]byte("Charlie"), []byte("msg3")}}},
"C": &mockChannelPolicyManager{&mockPolicy{&mockIdentityDeserializer{[]byte("Alice"), []byte("msg1")}}},
"D": &mockChannelPolicyManager{&mockPolicy{&mockIdentityDeserializer{[]byte("Alice"), []byte("msg1")}}},
},
}

Expand All @@ -130,9 +138,15 @@ func TestVerifyBlock(t *testing.T) {
// - Prepare testing valid block, Alice signs it.
blockRaw, msg := mockBlock(t, "C", aliceSigner, nil)
policyManagerGetter.managers["C"].(*mockChannelPolicyManager).mockPolicy.(*mockPolicy).deserializer.(*mockIdentityDeserializer).msg = msg
blockRaw2, msg2 := mockBlock(t, "D", aliceSigner, nil)
policyManagerGetter.managers["D"].(*mockChannelPolicyManager).mockPolicy.(*mockPolicy).deserializer.(*mockIdentityDeserializer).msg = msg2

// - Verify block
assert.NoError(t, msgCryptoService.VerifyBlock([]byte("C"), blockRaw))
delete(policyManagerGetter.managers, "D")
nilPolMgrErr := msgCryptoService.VerifyBlock([]byte("D"), blockRaw2)
assert.Contains(t, fmt.Sprintf("%v", nilPolMgrErr), "Could not acquire policy manager")
assert.Error(t, nilPolMgrErr)
assert.Error(t, msgCryptoService.VerifyBlock([]byte("A"), blockRaw))
assert.Error(t, msgCryptoService.VerifyBlock([]byte("B"), blockRaw))

Expand Down

0 comments on commit a5f2ba0

Please sign in to comment.