Skip to content

Commit 90e09ea

Browse files
committed
[FAB-4867]: Add endpoint validation for gossip.
While initializing gossip component for gossip service there is endpoint parameter provided which later split into hostname and port number, while there is no check whenever endpoint configured properly, hence misconfiguration leads to panic with no meaninggful information about the configuration error. This commit takes care of it by adding validation. Change-Id: I50fd728e715b40aa13b449e5867e3542cee67b1e Signed-off-by: Artem Barger <[email protected]>
1 parent 3bc937e commit 90e09ea

File tree

7 files changed

+54
-32
lines changed

7 files changed

+54
-32
lines changed

core/peer/peer_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,13 @@ func TestCreateChainFromBlock(t *testing.T) {
157157
dialOpts = append(dialOpts, grpc.WithInsecure())
158158
return dialOpts
159159
}
160-
service.InitGossipServiceCustomDeliveryFactory(
160+
err = service.InitGossipServiceCustomDeliveryFactory(
161161
identity, "localhost:13611", grpcServer,
162162
&mockDeliveryClientFactory{},
163163
messageCryptoService, secAdv, defaultSecureDialOpts)
164164

165+
assert.NoError(t, err)
166+
165167
err = CreateChainFromBlock(block)
166168
if err != nil {
167169
t.Fatalf("failed to create chain %s", err)

core/scc/cscc/configure_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,8 @@ func TestConfigerInvokeJoinChainCorrectParams(t *testing.T) {
200200
identity, _ := mgmt.GetLocalSigningIdentityOrPanic().Serialize()
201201
messageCryptoService := peergossip.NewMCS(&mocks.ChannelPolicyManagerGetter{}, localmsp.NewSigner(), mgmt.NewDeserializersManager())
202202
secAdv := peergossip.NewSecurityAdvisor(mgmt.NewDeserializersManager())
203-
service.InitGossipServiceCustomDeliveryFactory(identity, peerEndpoint, nil, &mockDeliveryClientFactory{}, messageCryptoService, secAdv, nil)
203+
err := service.InitGossipServiceCustomDeliveryFactory(identity, peerEndpoint, nil, &mockDeliveryClientFactory{}, messageCryptoService, secAdv, nil)
204+
assert.NoError(t, err)
204205

205206
// Successful path for JoinChain
206207
blockBytes := mockConfigBlock()

gossip/integration/integration.go

+19-9
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ package integration
1818

1919
import (
2020
"crypto/tls"
21+
"fmt"
22+
"net"
2123
"strconv"
22-
"strings"
2324
"time"
2425

2526
"github.com/hyperledger/fabric/core/config"
@@ -33,17 +34,23 @@ import (
3334

3435
// This file is used to bootstrap a gossip instance and/or leader election service instance
3536

36-
func newConfig(selfEndpoint string, externalEndpoint string, bootPeers ...string) *gossip.Config {
37-
port, err := strconv.ParseInt(strings.Split(selfEndpoint, ":")[1], 10, 64)
37+
func newConfig(selfEndpoint string, externalEndpoint string, bootPeers ...string) (*gossip.Config, error) {
38+
_, p, err := net.SplitHostPort(selfEndpoint)
39+
40+
if err != nil {
41+
return nil, fmt.Errorf("misconfigured endpoint %s, the error is %s", selfEndpoint, err)
42+
}
43+
44+
port, err := strconv.ParseInt(p, 10, 64)
3845
if err != nil {
39-
panic(err)
46+
return nil, fmt.Errorf("misconfigured endpoint %s, failed to parse port number due to %s", selfEndpoint, err)
4047
}
4148

4249
var cert *tls.Certificate
4350
if viper.GetBool("peer.tls.enabled") {
4451
certTmp, err := tls.LoadX509KeyPair(config.GetPath("peer.tls.cert.file"), config.GetPath("peer.tls.key.file"))
4552
if err != nil {
46-
panic(err)
53+
return nil, fmt.Errorf("failed to load certificates because of %s", err)
4754
}
4855
cert = &certTmp
4956
}
@@ -66,19 +73,22 @@ func newConfig(selfEndpoint string, externalEndpoint string, bootPeers ...string
6673
PublishStateInfoInterval: util.GetDurationOrDefault("peer.gossip.publishStateInfoInterval", 4*time.Second),
6774
SkipBlockVerification: viper.GetBool("peer.gossip.skipBlockVerification"),
6875
TLSServerCert: cert,
69-
}
76+
}, nil
7077
}
7178

7279
// NewGossipComponent creates a gossip component that attaches itself to the given gRPC server
7380
func NewGossipComponent(peerIdentity []byte, endpoint string, s *grpc.Server,
7481
secAdv api.SecurityAdvisor, cryptSvc api.MessageCryptoService, idMapper identity.Mapper,
75-
secureDialOpts api.PeerSecureDialOpts, bootPeers ...string) gossip.Gossip {
82+
secureDialOpts api.PeerSecureDialOpts, bootPeers ...string) (gossip.Gossip, error) {
7683

7784
externalEndpoint := viper.GetString("peer.gossip.externalEndpoint")
7885

79-
conf := newConfig(endpoint, externalEndpoint, bootPeers...)
86+
conf, err := newConfig(endpoint, externalEndpoint, bootPeers...)
87+
if err != nil {
88+
return nil, err
89+
}
8090
gossipInstance := gossip.NewGossipService(conf, s, secAdv, cryptSvc, idMapper,
8191
peerIdentity, secureDialOpts)
8292

83-
return gossipInstance
93+
return gossipInstance, nil
8494
}

gossip/integration/integration_test.go

+12-11
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,15 @@ func TestNewGossipCryptoService(t *testing.T) {
6464
peerIdentity, _ := mgmt.GetLocalSigningIdentityOrPanic().Serialize()
6565
idMapper := identity.NewIdentityMapper(cryptSvc, peerIdentity)
6666

67-
g1 := NewGossipComponent(peerIdentity, endpoint1, s1, secAdv, cryptSvc, idMapper,
67+
g1, err := NewGossipComponent(peerIdentity, endpoint1, s1, secAdv, cryptSvc, idMapper,
6868
defaultSecureDialOpts)
69-
g2 := NewGossipComponent(peerIdentity, endpoint2, s2, secAdv, cryptSvc, idMapper,
69+
assert.NoError(t, err)
70+
g2, err := NewGossipComponent(peerIdentity, endpoint2, s2, secAdv, cryptSvc, idMapper,
7071
defaultSecureDialOpts, endpoint1)
71-
g3 := NewGossipComponent(peerIdentity, endpoint3, s3, secAdv, cryptSvc, idMapper,
72+
assert.NoError(t, err)
73+
g3, err := NewGossipComponent(peerIdentity, endpoint3, s3, secAdv, cryptSvc, idMapper,
7274
defaultSecureDialOpts, endpoint1)
75+
assert.NoError(t, err)
7376
defer g1.Stop()
7477
defer g2.Stop()
7578
defer g3.Stop()
@@ -83,14 +86,12 @@ func TestBadInitialization(t *testing.T) {
8386
peerIdentity, _ := mgmt.GetLocalSigningIdentityOrPanic().Serialize()
8487
s1 := grpc.NewServer()
8588
idMapper := identity.NewIdentityMapper(cryptSvc, peerIdentity)
86-
assert.Panics(t, func() {
87-
newConfig("anEndpointWithoutAPort", "anEndpointWithoutAPort")
88-
})
89-
assert.Panics(t, func() {
90-
viper.Set("peer.tls.enabled", true)
91-
NewGossipComponent(peerIdentity, "localhost:5000", s1, secAdv, cryptSvc, idMapper,
92-
defaultSecureDialOpts)
93-
})
89+
_, err := newConfig("anEndpointWithoutAPort", "anEndpointWithoutAPort")
90+
91+
viper.Set("peer.tls.enabled", true)
92+
_, err = NewGossipComponent(peerIdentity, "localhost:5000", s1, secAdv, cryptSvc, idMapper,
93+
defaultSecureDialOpts)
94+
assert.Error(t, err)
9495
}
9596

9697
func setupTestEnv() {

gossip/service/gossip_service.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -120,20 +120,22 @@ var logger = util.GetLogger(util.LoggingServiceModule, "")
120120

121121
// InitGossipService initialize gossip service
122122
func InitGossipService(peerIdentity []byte, endpoint string, s *grpc.Server, mcs api.MessageCryptoService,
123-
secAdv api.SecurityAdvisor, secureDialOpts api.PeerSecureDialOpts, bootPeers ...string) {
123+
secAdv api.SecurityAdvisor, secureDialOpts api.PeerSecureDialOpts, bootPeers ...string) error {
124124
// TODO: Remove this.
125125
// TODO: This is a temporary work-around to make the gossip leader election module load its logger at startup
126126
// TODO: in order for the flogging package to register this logger in time so it can set the log levels as requested in the config
127127
util.GetLogger(util.LoggingElectionModule, "")
128-
InitGossipServiceCustomDeliveryFactory(peerIdentity, endpoint, s, &deliveryFactoryImpl{},
128+
return InitGossipServiceCustomDeliveryFactory(peerIdentity, endpoint, s, &deliveryFactoryImpl{},
129129
mcs, secAdv, secureDialOpts, bootPeers...)
130130
}
131131

132132
// InitGossipServiceCustomDeliveryFactory initialize gossip service with customize delivery factory
133133
// implementation, might be useful for testing and mocking purposes
134134
func InitGossipServiceCustomDeliveryFactory(peerIdentity []byte, endpoint string, s *grpc.Server,
135135
factory DeliveryServiceFactory, mcs api.MessageCryptoService, secAdv api.SecurityAdvisor,
136-
secureDialOpts api.PeerSecureDialOpts, bootPeers ...string) {
136+
secureDialOpts api.PeerSecureDialOpts, bootPeers ...string) error {
137+
var err error
138+
var gossip gossip.Gossip
137139
once.Do(func() {
138140
if overrideEndpoint := viper.GetString("peer.gossip.endpoint"); overrideEndpoint != "" {
139141
endpoint = overrideEndpoint
@@ -142,7 +144,7 @@ func InitGossipServiceCustomDeliveryFactory(peerIdentity []byte, endpoint string
142144
logger.Info("Initialize gossip with endpoint", endpoint, "and bootstrap set", bootPeers)
143145

144146
idMapper := identity.NewIdentityMapper(mcs, peerIdentity)
145-
gossip := integration.NewGossipComponent(peerIdentity, endpoint, s, secAdv,
147+
gossip, err = integration.NewGossipComponent(peerIdentity, endpoint, s, secAdv,
146148
mcs, idMapper, secureDialOpts, bootPeers...)
147149
gossipServiceInstance = &gossipServiceImpl{
148150
mcs: mcs,
@@ -155,6 +157,7 @@ func InitGossipServiceCustomDeliveryFactory(peerIdentity []byte, endpoint string
155157
secAdv: secAdv,
156158
}
157159
})
160+
return err
158161
}
159162

160163
// GetGossipService returns an instance of gossip service

gossip/service/gossip_service_test.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@ func TestInitGossipService(t *testing.T) {
6666
wg.Add(10)
6767
for i := 0; i < 10; i++ {
6868
go func() {
69+
defer wg.Done()
6970
messageCryptoService := peergossip.NewMCS(&mocks.ChannelPolicyManagerGetter{}, localmsp.NewSigner(), mgmt.NewDeserializersManager())
7071
secAdv := peergossip.NewSecurityAdvisor(mgmt.NewDeserializersManager())
71-
InitGossipService(identity, "localhost:5611", grpcServer, messageCryptoService,
72+
err := InitGossipService(identity, "localhost:5611", grpcServer, messageCryptoService,
7273
secAdv, nil)
73-
74-
wg.Done()
74+
assert.NoError(t, err)
7575
}()
7676
}
7777
wg.Wait()
@@ -703,8 +703,9 @@ func TestInvalidInitialization(t *testing.T) {
703703
defer grpcServer.Stop()
704704

705705
secAdv := peergossip.NewSecurityAdvisor(mgmt.NewDeserializersManager())
706-
InitGossipService(api.PeerIdentityType("IDENTITY"), "localhost:7611", grpcServer,
706+
err := InitGossipService(api.PeerIdentityType("IDENTITY"), "localhost:7611", grpcServer,
707707
&naiveCryptoService{}, secAdv, nil)
708+
assert.NoError(t, err)
708709
gService := GetGossipService().(*gossipServiceImpl)
709710
defer gService.Stop()
710711

@@ -727,8 +728,9 @@ func TestChannelConfig(t *testing.T) {
727728
defer grpcServer.Stop()
728729

729730
secAdv := peergossip.NewSecurityAdvisor(mgmt.NewDeserializersManager())
730-
InitGossipService(api.PeerIdentityType("IDENTITY"), "localhost:6611", grpcServer,
731+
error = InitGossipService(api.PeerIdentityType("IDENTITY"), "localhost:6611", grpcServer,
731732
&naiveCryptoService{}, secAdv, nil)
733+
assert.NoError(t, error)
732734
gService := GetGossipService().(*gossipServiceImpl)
733735
defer gService.Stop()
734736

peer/node/start.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,11 @@ func serve(args []string) error {
175175
}
176176
return dialOpts
177177
}
178-
service.InitGossipService(serializedIdentity, peerEndpoint.Address, peerServer.Server(),
178+
err = service.InitGossipService(serializedIdentity, peerEndpoint.Address, peerServer.Server(),
179179
messageCryptoService, secAdv, secureDialOpts, bootstrap...)
180+
if err != nil {
181+
return err
182+
}
180183
defer service.GetGossipService().Stop()
181184

182185
//initialize system chaincodes

0 commit comments

Comments
 (0)