Skip to content

Commit b756dbe

Browse files
fix: prevent self dial (#5306)
Co-authored-by: Janoš Guljaš <[email protected]>
1 parent fef3fb1 commit b756dbe

File tree

11 files changed

+427
-22
lines changed

11 files changed

+427
-22
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
/.idea
33
/.vscode
44
/tmp
5+
/vendor
56

67
# Compiled Object files, Static and Dynamic libs (Shared Objects)
78
*.o

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ require (
1313
github.com/ethereum/go-ethereum v1.15.11
1414
github.com/ethersphere/batch-archive v0.0.4
1515
github.com/ethersphere/go-price-oracle-abi v0.6.9
16-
github.com/ethersphere/go-storage-incentives-abi v0.9.4
16+
github.com/ethersphere/go-storage-incentives-abi v0.9.3-rc4
1717
github.com/ethersphere/go-sw3-abi v0.6.9
1818
github.com/ethersphere/langos v1.0.0
1919
github.com/go-playground/validator/v10 v10.19.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,8 +240,8 @@ github.com/ethersphere/batch-archive v0.0.4 h1:PHmwQfmUEyDJgoX2IqI/R0alQ63+aLPXf
240240
github.com/ethersphere/batch-archive v0.0.4/go.mod h1:41BPb192NoK9CYjNB8BAE1J2MtiI/5aq0Wtas5O7A7Q=
241241
github.com/ethersphere/go-price-oracle-abi v0.6.9 h1:bseen6he3PZv5GHOm+KD6s4awaFmVSD9LFx+HpB6rCU=
242242
github.com/ethersphere/go-price-oracle-abi v0.6.9/go.mod h1:sI/Qj4/zJ23/b1enzwMMv0/hLTpPNVNacEwCWjo6yBk=
243-
github.com/ethersphere/go-storage-incentives-abi v0.9.4 h1:mSIWXQXg5OQmH10QvXMV5w0vbSibFMaRlBL37gPLTM0=
244-
github.com/ethersphere/go-storage-incentives-abi v0.9.4/go.mod h1:SXvJVtM4sEsaSKD0jc1ClpDLw8ErPoROZDme4Wrc/Nc=
243+
github.com/ethersphere/go-storage-incentives-abi v0.9.3-rc4 h1:YK9FpiQz29ctU5V46CuwMt+4X5Xn8FTBwy6E2v/ix8s=
244+
github.com/ethersphere/go-storage-incentives-abi v0.9.3-rc4/go.mod h1:SXvJVtM4sEsaSKD0jc1ClpDLw8ErPoROZDme4Wrc/Nc=
245245
github.com/ethersphere/go-sw3-abi v0.6.9 h1:TnWLnYkWE5UvC17mQBdUmdkzhPhO8GcqvWy4wvd1QJQ=
246246
github.com/ethersphere/go-sw3-abi v0.6.9/go.mod h1:BmpsvJ8idQZdYEtWnvxA8POYQ8Rl/NhyCdF0zLMOOJU=
247247
github.com/ethersphere/langos v1.0.0 h1:NBtNKzXTTRSue95uOlzPN4py7Aofs0xWPzyj4AI1Vcc=

pkg/hive/hive.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,10 @@ type Service struct {
6464
sem *semaphore.Weighted
6565
bootnode bool
6666
allowPrivateCIDRs bool
67+
overlay swarm.Address
6768
}
6869

69-
func New(streamer p2p.Streamer, addressbook addressbook.GetPutter, networkID uint64, bootnode bool, allowPrivateCIDRs bool, logger log.Logger) *Service {
70+
func New(streamer p2p.Streamer, addressbook addressbook.GetPutter, networkID uint64, bootnode bool, allowPrivateCIDRs bool, overlay swarm.Address, logger log.Logger) *Service {
7071
svc := &Service{
7172
streamer: streamer,
7273
logger: logger.WithName(loggerName).Register(),
@@ -80,6 +81,7 @@ func New(streamer p2p.Streamer, addressbook addressbook.GetPutter, networkID uin
8081
sem: semaphore.NewWeighted(int64(swarm.MaxBins)),
8182
bootnode: bootnode,
8283
allowPrivateCIDRs: allowPrivateCIDRs,
84+
overlay: overlay,
8385
}
8486

8587
if !bootnode {
@@ -174,6 +176,11 @@ func (s *Service) sendPeers(ctx context.Context, peer swarm.Address, peers []swa
174176
w, _ := protobuf.NewWriterAndReader(stream)
175177
var peersRequest pb.Peers
176178
for _, p := range peers {
179+
if p.Equal(s.overlay) {
180+
s.logger.Debug("skipping self-address in broadcast", "overlay", p.String())
181+
continue
182+
}
183+
177184
addr, err := s.addressBook.Get(p)
178185
if err != nil {
179186
if errors.Is(err, addressbook.ErrNotFound) {
@@ -290,8 +297,15 @@ func (s *Service) checkAndAddPeers(peers pb.Peers) {
290297
continue
291298
}
292299

300+
overlayAddr := swarm.NewAddress(p.Overlay)
301+
302+
if overlayAddr.Equal(s.overlay) {
303+
s.logger.Debug("skipping self-address in peer list", "overlay", overlayAddr.String())
304+
continue
305+
}
306+
293307
bzzAddress := bzz.Address{
294-
Overlay: swarm.NewAddress(p.Overlay),
308+
Overlay: overlayAddr,
295309
Underlays: multiUnderlays,
296310
Signature: p.Signature,
297311
Nonce: p.Nonce,

pkg/hive/hive_test.go

Lines changed: 227 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,9 @@ func TestHandlerRateLimit(t *testing.T) {
5050
// new recorder
5151
streamer := streamtest.New()
5252
// create a hive server that handles the incoming stream
53-
server := hive.New(streamer, addressbookclean, networkID, false, true, logger)
54-
testutil.CleanupCloser(t, server)
55-
5653
serverAddress := swarm.RandAddress(t)
54+
server := hive.New(streamer, addressbookclean, networkID, false, true, serverAddress, logger)
55+
testutil.CleanupCloser(t, server)
5756

5857
// setup the stream recorder to record stream data
5958
serverRecorder := streamtest.New(
@@ -94,7 +93,8 @@ func TestHandlerRateLimit(t *testing.T) {
9493
}
9594

9695
// create a hive client that will do broadcast
97-
client := hive.New(serverRecorder, addressbook, networkID, false, true, logger)
96+
clientAddress := swarm.RandAddress(t)
97+
client := hive.New(serverRecorder, addressbook, networkID, false, true, clientAddress, logger)
9898
err := client.BroadcastPeers(context.Background(), serverAddress, peers...)
9999
if err != nil {
100100
t.Fatal(err)
@@ -273,7 +273,8 @@ func TestBroadcastPeers(t *testing.T) {
273273

274274
streamer := streamtest.New()
275275
// create a hive server that handles the incoming stream
276-
server := hive.New(streamer, addressbookclean, networkID, false, true, logger)
276+
serverAddress := swarm.RandAddress(t)
277+
server := hive.New(streamer, addressbookclean, networkID, false, true, serverAddress, logger)
277278
testutil.CleanupCloser(t, server)
278279

279280
// setup the stream recorder to record stream data
@@ -282,7 +283,8 @@ func TestBroadcastPeers(t *testing.T) {
282283
)
283284

284285
// create a hive client that will do broadcast
285-
client := hive.New(recorder, addressbook, networkID, false, tc.allowPrivateCIDRs, logger)
286+
clientAddress := swarm.RandAddress(t)
287+
client := hive.New(recorder, addressbook, networkID, false, tc.allowPrivateCIDRs, clientAddress, logger)
286288

287289
if err := client.BroadcastPeers(context.Background(), tc.addresee, tc.peers...); err != nil {
288290
t.Fatal(err)
@@ -444,3 +446,222 @@ func shortHex(b []byte) string {
444446
}
445447
return s
446448
}
449+
450+
// TestBroadcastPeersSkipsSelf verifies that hive does not broadcast self address
451+
// to other peers, preventing self-connection attempts.
452+
func TestBroadcastPeersSkipsSelf(t *testing.T) {
453+
t.Parallel()
454+
455+
logger := log.Noop
456+
statestore := mock.NewStateStore()
457+
addressbook := ab.New(statestore)
458+
networkID := uint64(1)
459+
addressbookclean := ab.New(mock.NewStateStore())
460+
461+
// Create addresses
462+
serverAddress := swarm.RandAddress(t)
463+
clientAddress := swarm.RandAddress(t)
464+
465+
// Create a peer address
466+
peer1 := swarm.RandAddress(t)
467+
underlay1, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/1234")
468+
if err != nil {
469+
t.Fatal(err)
470+
}
471+
pk, err := crypto.GenerateSecp256k1Key()
472+
if err != nil {
473+
t.Fatal(err)
474+
}
475+
signer := crypto.NewDefaultSigner(pk)
476+
overlay1, err := crypto.NewOverlayAddress(pk.PublicKey, networkID, block)
477+
if err != nil {
478+
t.Fatal(err)
479+
}
480+
bzzAddr1, err := bzz.NewAddress(signer, []ma.Multiaddr{underlay1}, overlay1, networkID, nonce)
481+
if err != nil {
482+
t.Fatal(err)
483+
}
484+
if err := addressbook.Put(bzzAddr1.Overlay, *bzzAddr1); err != nil {
485+
t.Fatal(err)
486+
}
487+
488+
// Create self address entry in addressbook
489+
underlayClient, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/9999")
490+
if err != nil {
491+
t.Fatal(err)
492+
}
493+
pkClient, err := crypto.GenerateSecp256k1Key()
494+
if err != nil {
495+
t.Fatal(err)
496+
}
497+
signerClient := crypto.NewDefaultSigner(pkClient)
498+
bzzAddrClient, err := bzz.NewAddress(signerClient, []ma.Multiaddr{underlayClient}, clientAddress, networkID, nonce)
499+
if err != nil {
500+
t.Fatal(err)
501+
}
502+
if err := addressbook.Put(clientAddress, *bzzAddrClient); err != nil {
503+
t.Fatal(err)
504+
}
505+
506+
// Setup server
507+
streamer := streamtest.New()
508+
server := hive.New(streamer, addressbookclean, networkID, false, true, serverAddress, logger)
509+
testutil.CleanupCloser(t, server)
510+
511+
serverRecorder := streamtest.New(
512+
streamtest.WithProtocols(server.Protocol()),
513+
)
514+
515+
// Setup client
516+
client := hive.New(serverRecorder, addressbook, networkID, false, true, clientAddress, logger)
517+
testutil.CleanupCloser(t, client)
518+
519+
// Try to broadcast: peer1, clientAddress (self), and another peer
520+
peersIncludingSelf := []swarm.Address{bzzAddr1.Overlay, clientAddress, peer1}
521+
522+
err = client.BroadcastPeers(context.Background(), serverAddress, peersIncludingSelf...)
523+
if err != nil {
524+
t.Fatal(err)
525+
}
526+
527+
// Get records
528+
records, err := serverRecorder.Records(serverAddress, "hive", "1.1.0", "peers")
529+
if err != nil {
530+
t.Fatal(err)
531+
}
532+
533+
if len(records) == 0 {
534+
t.Fatal("expected at least one record")
535+
}
536+
537+
// Read the messages
538+
messages, err := readAndAssertPeersMsgs(records[0].In(), 1)
539+
if err != nil {
540+
t.Fatal(err)
541+
}
542+
543+
// Verify that clientAddress (self) was NOT included in broadcast
544+
for _, peerMsg := range messages[0].Peers {
545+
receivedOverlay := swarm.NewAddress(peerMsg.Overlay)
546+
if receivedOverlay.Equal(clientAddress) {
547+
t.Fatal("self address should not be broadcast to peers")
548+
}
549+
}
550+
551+
// Verify server addressbook eventually contains only the valid peers, not self
552+
err = spinlock.Wait(spinTimeout, func() bool {
553+
overlays, err := addressbookclean.Overlays()
554+
if err != nil {
555+
t.Fatal(err)
556+
}
557+
// Should only have bzzAddr1, not clientAddress
558+
for _, o := range overlays {
559+
if o.Equal(clientAddress) {
560+
return false // self should not be in addressbook
561+
}
562+
}
563+
return true
564+
})
565+
if err != nil {
566+
t.Fatal("self address found in server addressbook")
567+
}
568+
}
569+
570+
// TestReceivePeersSkipsSelf verifies that hive does not add self address
571+
// when receiving peer lists from other peers.
572+
func TestReceivePeersSkipsSelf(t *testing.T) {
573+
t.Parallel()
574+
575+
logger := log.Noop
576+
statestore := mock.NewStateStore()
577+
addressbook := ab.New(statestore)
578+
networkID := uint64(1)
579+
addressbookclean := ab.New(mock.NewStateStore())
580+
581+
// Create addresses
582+
serverAddress := swarm.RandAddress(t)
583+
clientAddress := swarm.RandAddress(t)
584+
585+
// Create a valid peer
586+
underlay1, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/1234")
587+
if err != nil {
588+
t.Fatal(err)
589+
}
590+
pk1, err := crypto.GenerateSecp256k1Key()
591+
if err != nil {
592+
t.Fatal(err)
593+
}
594+
signer1 := crypto.NewDefaultSigner(pk1)
595+
overlay1, err := crypto.NewOverlayAddress(pk1.PublicKey, networkID, block)
596+
if err != nil {
597+
t.Fatal(err)
598+
}
599+
bzzAddr1, err := bzz.NewAddress(signer1, []ma.Multiaddr{underlay1}, overlay1, networkID, nonce)
600+
if err != nil {
601+
t.Fatal(err)
602+
}
603+
if err := addressbook.Put(bzzAddr1.Overlay, *bzzAddr1); err != nil {
604+
t.Fatal(err)
605+
}
606+
607+
// Create self address entry (serverAddress) that will be sent by client
608+
underlayServer, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/8888")
609+
if err != nil {
610+
t.Fatal(err)
611+
}
612+
pkServer, err := crypto.GenerateSecp256k1Key()
613+
if err != nil {
614+
t.Fatal(err)
615+
}
616+
signerServer := crypto.NewDefaultSigner(pkServer)
617+
bzzAddrServer, err := bzz.NewAddress(signerServer, []ma.Multiaddr{underlayServer}, serverAddress, networkID, nonce)
618+
if err != nil {
619+
t.Fatal(err)
620+
}
621+
// Add server's own address to client's addressbook (so client can send it)
622+
if err := addressbook.Put(serverAddress, *bzzAddrServer); err != nil {
623+
t.Fatal(err)
624+
}
625+
626+
// Setup server that will receive peers including its own address
627+
streamer := streamtest.New()
628+
server := hive.New(streamer, addressbookclean, networkID, false, true, serverAddress, logger)
629+
testutil.CleanupCloser(t, server)
630+
631+
serverRecorder := streamtest.New(
632+
streamtest.WithProtocols(server.Protocol()),
633+
)
634+
635+
// Setup client
636+
client := hive.New(serverRecorder, addressbook, networkID, false, true, clientAddress, logger)
637+
testutil.CleanupCloser(t, client)
638+
639+
// Client broadcasts: valid peer and server's own address
640+
peersIncludingSelf := []swarm.Address{bzzAddr1.Overlay, serverAddress}
641+
642+
err = client.BroadcastPeers(context.Background(), serverAddress, peersIncludingSelf...)
643+
if err != nil {
644+
t.Fatal(err)
645+
}
646+
647+
// Wait a bit for server to process
648+
time.Sleep(100 * time.Millisecond)
649+
650+
// Verify server's addressbook does NOT contain its own address
651+
overlays, err := addressbookclean.Overlays()
652+
if err != nil {
653+
t.Fatal(err)
654+
}
655+
656+
for _, o := range overlays {
657+
if o.Equal(serverAddress) {
658+
t.Fatal("server should not add its own address to addressbook when received from peer")
659+
}
660+
}
661+
662+
// Verify server does have the valid peer
663+
_, err = addressbookclean.Get(bzzAddr1.Overlay)
664+
if err != nil {
665+
t.Fatalf("expected server to have valid peer in addressbook, got error: %v", err)
666+
}
667+
}

pkg/node/bootstrap.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func bootstrapNode(
117117
b.p2pService = p2ps
118118
b.p2pHalter = p2ps
119119

120-
hive := hive.New(p2ps, addressbook, networkID, o.BootnodeMode, o.AllowPrivateCIDRs, logger)
120+
hive := hive.New(p2ps, addressbook, networkID, o.BootnodeMode, o.AllowPrivateCIDRs, swarmAddress, logger)
121121

122122
if err = p2ps.AddProtocol(hive.Protocol()); err != nil {
123123
return nil, fmt.Errorf("hive service: %w", err)

pkg/node/node.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,7 @@ func NewBee(
748748
return nil, fmt.Errorf("pingpong service: %w", err)
749749
}
750750

751-
hive := hive.New(p2ps, addressbook, networkID, o.BootnodeMode, o.AllowPrivateCIDRs, logger)
751+
hive := hive.New(p2ps, addressbook, networkID, o.BootnodeMode, o.AllowPrivateCIDRs, swarmAddress, logger)
752752

753753
if err = p2ps.AddProtocol(hive.Protocol()); err != nil {
754754
return nil, fmt.Errorf("hive service: %w", err)

0 commit comments

Comments
 (0)